Project

General

Profile

Feature #7390

replace DOM with StAX in XmlFilePlugin

Added by Constantin Asofiei 12 months ago. Updated 12 months ago.

Status:
New
Priority:
Normal
Assignee:
-
Target version:
-
Start date:
Due date:
% Done:

0%

billable:
No
vendor_id:
GCD
version:

Related issues

Related to Database - Feature #6813: analyze and improve performance of dynamic conversion of temp-tables and queries WIP

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, and XmlStreamWriter for writing, in XmlFilePlugin?

A simple code using XmlStreamReader to just read the 90MB .ast file takes ~35% less than XmlHelper.parse. For the file above, it would be ~16% improvement in XmlFilePlugin.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.

Beside this, there is still 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.

Also available in: Atom PDF