Bug 382145 - Render Task order of magnitude slower in BIRT 3.7.2 than BIRT 2.6.2 [message #892582] |
Thu, 28 June 2012 16:25 |
David Schlosnagle Messages: 1 Registered: June 2012 |
Junior Member |
|
|
While upgrading from BIRT 2.6.2 to 3.7.2 (and 4.2) we have encountered severe performance degradation with BIRT's render tasks. We have filed bug 382145 to address this issue, and I have provided a patch against BIRT's master branch that brings performance back in line with 2.6.2. I would like to know what the next steps are in having the patch reviewed and applied to BIRT 4.2.x.
I performed a YourKit tracing profiling of the attached testcase with BIRT
3.7.2, and discovered that there are a number of areas for improvement in the
archive file implementation added to BIRT 3.7. The attached profiler
screenshots show the following major hot spots and the related changes in my patch:
org.eclipse.birt.core.archive.compound.ArchiveFileV3.listEntries(String)
- The current implementation of this method does an O(N) iteration over all entries to find the entries that begin with the specified name pattern. This is inefficient for large numbers of entries and repeated uses of this method then become exponentially costly.
- I refactored this to use the Ext2FileSystem methods that provide an Iterable<String> of entries that begin with the specified name pattern. In combination with changes to EntryTable this becomes O(log N).
org.eclipse.birt.core.archive.compound.v3.EntryTable.write()
- The current implemention copies the keySet, sorts it, then iterates over the entries get()ing each from the backing entry map.
- I refactored the entry map to use a TreeMap to provide an always sorted map. This allows us to avoid the array copy and sort. Additionally, I added a new listEntries(String) method that allows for O(log N) sub-tree searches via SortedMap.tailMap(String) and the new PrefixedIterable class. The listEntries() method was renamed to listAllEntries(), and both listAllEntries() and listEntries(String) now return an Iterable<String> to simplify their usage.
org.eclipse.birt.data.engine.impl.DataEngineImpl.addShutdownListener(IShutdownListener)
- The current implementation of this method does an O(N) iteration over all listener entries to determine whether a listener has already been added. When there are large numbers of listeners (e.g. one per result from org.eclipse.birt.data.engine.impl.ResultIterator.addEngineShutdownListener()), adding and removing become very expensive operations.
- I refactored this to use a LinkedHashSet to provide O(1) add and remove operations.
Overall the attached patch changes bring the RenderTask times back in line with
BIRT 2.6.2, and help the RunTask slightly.
Before fix:
Report generated (RunTask): 60,354ms.
Report rendered (RenderTask): 135,915ms.
After fix:
Report generated (RunTask): 44,837ms.
Report rendered (RenderTask): 12,123ms.
Thanks,
Dave
|
|
|
Re: Bug 382145 - Render Task order of magnitude slower in BIRT 3.7.2 than BIRT 2.6.2 [message #892794 is a reply to message #892582] |
Fri, 29 June 2012 17:49 |
|
David
Thanks for all the details and the patch. The dev team is evaluating
the path now I believe.
Jason
On 6/28/2012 12:25 PM, David Schlosnagle wrote:
> While upgrading from BIRT 2.6.2 to 3.7.2 (and 4.2) we have encountered
> severe performance degradation with BIRT's render tasks. We have filed
> bug 382145 to address this issue, and I have provided a patch against
> BIRT's master branch that brings performance back in line with 2.6.2. I
> would like to know what the next steps are in having the patch reviewed
> and applied to BIRT 4.2.x.
>
> I performed a YourKit tracing profiling of the attached testcase with BIRT
> 3.7.2, and discovered that there are a number of areas for improvement
> in the
> archive file implementation added to BIRT 3.7. The attached profiler
> screenshots show the following major hot spots and the related changes
> in my patch:
>
> org.eclipse.birt.core.archive.compound.ArchiveFileV3.listEntries(String)
>
> The current implementation of this method does an O(N) iteration over
> all entries to find the entries that begin with the specified name
> pattern. This is inefficient for large numbers of entries and repeated
> uses of this method then become exponentially costly.
> I refactored this to use the Ext2FileSystem methods that provide an
> Iterable<String> of entries that begin with the specified name pattern.
> In combination with changes to EntryTable this becomes O(log N).
>
>
> org.eclipse.birt.core.archive.compound.v3.EntryTable.write()
>
> The current implemention copies the keySet, sorts it, then iterates over
> the entries get()ing each from the backing entry map.
> I refactored the entry map to use a TreeMap to provide an always sorted
> map. This allows us to avoid the array copy and sort. Additionally, I
> added a new listEntries(String) method that allows for O(log N) sub-tree
> searches via SortedMap.tailMap(String) and the new PrefixedIterable
> class. The listEntries() method was renamed to listAllEntries(), and
> both listAllEntries() and listEntries(String) now return an
> Iterable<String> to simplify their usage.
>
>
> org.eclipse.birt.data.engine.impl.DataEngineImpl.addShutdownListener(IShutdownListener)
>
>
> The current implementation of this method does an O(N) iteration over
> all listener entries to determine whether a listener has already been
> added. When there are large numbers of listeners (e.g. one per result
> from
> org.eclipse.birt.data.engine.impl.ResultIterator.addEngineShutdownListener()),
> adding and removing become very expensive operations.
> I refactored this to use a LinkedHashSet to provide O(1) add and remove
> operations.
>
>
>
> Overall the attached patch changes bring the RenderTask times back in
> line with
> BIRT 2.6.2, and help the RunTask slightly.
>
> Before fix:
> Report generated (RunTask): 60,354ms.
> Report rendered (RenderTask): 135,915ms.
>
> After fix:
> Report generated (RunTask): 44,837ms.
> Report rendered (RenderTask): 12,123ms.
>
> Thanks,
> Dave
>
|
|
|
Powered by
FUDForum. Page generated in 0.02524 seconds