Feature #7390
replace DOM with StAX in XmlFilePlugin
0%
Related issues
History
#1 Updated by Constantin Asofiei 12 months ago
This is from #6813-83 and #6813-84
It takes ~1.3 seconds to load a 90MB .ast file on my machine, 54% of the time spent in
XmlHelper.parse
.XmlFilePlugin.loadTree
relies on DOM, which is slow on its own. Maybe we should consider to use StAX/XmlStreamReader
for reading, andXmlStreamWriter
for writing, inXmlFilePlugin
?A simple code using
XmlStreamReader
to just read the 90MB .ast file takes ~35% less thanXmlHelper.parse
. For the file above, it would be ~16% improvement inXmlFilePlugin.loadTree
. With some napkin math, in an app we have 130GB of .ast files, which get fully processed I think 10 times? With 1.3s per 90MB, this gives 0.5 hours for just .ast loading, and it would be ~5hours just for this processing, over the full conversion. This does not include the DOM creation for writing, which can improve things further.With a ~16% improvement, this would result in ~50minutes improvement for
XmlFilePlugin.loadTree
over this 130GB .ast files, for the entire conversion. It doesn't look much overall, but it may be worth it when we add the heap cost, too.
This task is meant to implement the above idea, and replace DOM with XmlSTreamReader
and XmlStreamWriter
at the conversion level.
#2 Updated by Ovidiu Maxiniuc 12 months ago
What about a binary encoding?
The XML is OK for development and debugging, but when we want performance the binary encoding is the solution. Based on some special flag in cfg file.
#3 Updated by Greg Shah 12 months ago
- Related to Feature #6813: analyze and improve performance of dynamic conversion of temp-tables and queries added
#4 Updated by Constantin Asofiei 12 months ago
Ovidiu Maxiniuc wrote:
What about a binary encoding?
The XML is OK for development and debugging, but when we want performance the binary encoding is the solution. Based on some special flag in cfg file.
Another issue here is to avoid recursivity. Binary can work by just making AnnotatedAst Externalizable and implement writeExternal and readExternal - but this needs to avoid recursion, as this is expensive.
#5 Updated by Ovidiu Maxiniuc 12 months ago
Why recursion is a problem? We are serializing an AST, which is tree. There are no circular references between AST nodes. If they were, the XML serialization would fail as well.
The writeExternal
(and the readExternal
) just need to write the AST node name/type (String/Long), the map of annotations (the mapped values are only of basic types: String
, Long
, Boolean
, Double
). Indeed, there is a putAnnotationObject(String key, Object annotation)
, but I do not think the result of this is XML serializable as well. And then let the children, if any, do the same.
#6 Updated by Constantin Asofiei 12 months ago
I've used a 'sample profiler' when converting a very large application, and the DOM parsing does stand out (both heap and CPU usage).
I've also noticed that RuleContainer.getFunctionContainer
should cache the 'winner', as this is always computed when resolving a function by its name.
AnnotatedAst.duplicate
- this is being called on each rule container, in PatternEngine.apply
. So there is a lot of heap being allocated and discarded for each .ast file, especially in annotations phase:
- the change in
AnnotatedAst.duplicate
may need to be backed out, I'm not sure yet about this, as the conversion used 7156a. My point here is about allocating the dequeu's and the operations on them. - one of the top expensive operations is still the copy of the annotations map.
I understand that we need to duplicate the .ast for the copy
version in PatternEngine.apply
. But this is done a lot in annotations.xml
. I'm not sure how we can optimize this. Even resolver.setAsts(source, copy);
is expensive, as astMap
is cleared after each rule container, and is being re-built
In any case, replacing DOM with StAX will help.