DomFragmentBuilder should re-use DocumentBuilder

[GoogleCodeExporter]

DomFragmentBuilder keeps an instance of DocumentBuilderFactory, but creates a 
new DocumentBuilder whenever necessary (e.g. a call to newDocument(URI)).  
Performance may improve if we keep the DocumentBuilder instance, too, for 
re-use.

Original issue reported on code.google.com by joe.bays...@gmail.com on 20 Feb 2013 at 8:46

[GoogleCodeExporter]

Please be very, very careful.

If you cache a DocumentBuilder, you have to synchronize access to it. Assuming 
multiple threads, you'll need to cache multiple builders, or else block them 
from building while someone else has the precious. This is why dbf is cached, 
and db isn't.

FragmentBuilder isn't thread-safe, but that's well-known. If DocumentBuilders 
are shared, you're going to have to make certain when they are released. It's 
not a simple problem, or at least isn't an easy solution.

Original comment by aale...@gmail.com on 20 Feb 2013 at 9:00

• Added labels: Performance

[GoogleCodeExporter]

Agree with Amy, this isn't obviously a clear performance winner.

Original comment by eric%tib...@gtempaccount.com on 20 Feb 2013 at 9:20

[GoogleCodeExporter]

Unfortunately, we took some of the further discussion offline, but here's the 
bit I think is core:

"Looking at the DocumentBuilder javadoc, it provides newDocument(), which the 
FragmentBuilder is going to call, and after that we don't really care about the 
DocumentBuilder. That's because DocumentBuilder doesn't build the content of 
documents, it just produces the nodefactory, which in DOM is the Document node.

Was I mistaken in thinking of DocumentBuilder as the 'building' tool, when in 
fact it is the factory for the DOM node factory? So that DocumentBuilderFactory 
is actually the factory factory factory?"

Summarizing: objection withdrawn. There's no reason to be concerned about the 
DocumentBuilder.newDocument() method, because it's called once in 
FragmentBuilder to create the Document which is then used to create everything 
else. DocumentBuilder doesn't have any useful state. FragmentBuilder (and 
Document) do, so can't be shared. But there's no real reason that a 
DocumentBuilder couldn't be shared by all the FragmentBuilders (there's a 
slight chance that the DB could be called by multiple threads; if that's 
considered a valid concern, then we can synchronize (on DB) before calling 
newDocument()).

Original comment by aale...@gmail.com on 25 Feb 2013 at 3:46

[GoogleCodeExporter]

(r442) To DomProcessingContext, added a static instance of DocumentBuilder; to 
DomFragmentBuilder, cached an instance DocumentBuilder instead of 
DocumentBuilderFactory.

Original comment by joe.bays...@gmail.com on 26 Feb 2013 at 9:10

• Changed state: Fixed