Community
Participate
Working Groups
Build Identifier: Currently MetaDataRepository uses StringPool to intern shared strings. However, the entry point is on XMLParser and results in each child repository of a CompositeRepository having its own copy of duplicate strings. Additional savings can be gained by using the same StringPool to intern the strings for all child repositories. Reproducible: Always
Created attachment 183481 [details] Replace StringPool implementation with String.intern() Through empirical testing and discussions with GC team members it is appropriate to use String.intern() instead of our own StringPool implementation. This has the effect of sharing the pooled strings amongst all repositories in a composite repository. Although the Java Language Specification is silent on whether interned Strings are garbage collected all modern VMs do, in fact, garbage collect interned Strings. The IBM 1.4.2 VM started this practice in 2005. The Sun vms where doing it prior to this. Interned Strings are placed immediately in OLD space requiring a major collection to clean them up. However, given the long lived (minutes) nature of repository strings even our StringPool implementation would have ended up in OLD space, especially considering the heap churn evident during repository loads. As a final check I analyzed pre and post patch test runs using YourKit on 32 and 64 bit flavours of Java 5, and Java 6 Sun and IBM VMs. (YOurKit did not like the 1.4.2). There was no unwanted GC behaviour during the runs or significant changes in execution time.
I would suggest that this fix is a good candidate for back porting to the 3.6 stream. It is a trivial fix and will provide significant memory savings. Especially in 3.6 where we are unlikely to back port the SharedIU change. In loading the 3.7 I-Build repository it saves 11 Meg, 25% of retained memory. But perhaps more importantly it saves 50% or 90 Meg of heap expansion during the load.
This is a very interesting change since it allows for sharing across all the repos (and more), therefore the gain is potentially huge! Back in 2003/2004 we had attempted to use this and the performance at the time were rather deceiving both from a point of view of time to add to intern and GC (not GC'ed at the time). Obviously things have changed since then, so if we are confident about the speed and the GC characteristics, then I don't why we should refrain from using this and also backport this in 3.6.x.
I looked a the patch quickly (didn't apply it, just tried to do the patch math in my head), and it appears that we are doing the intern() on all strings (versions, IDs, names, etc...). I'm not complaining about that approach, but I wonder if there is any reason to only intern() strings that we know are likely to be duplicated?
The patch just change the invocation from canonicalize to intern, to that end the logic of what is 'shared' is unmodified before / after this patch. We could likely try to be smarter about what we end up sharing or not by making the decision of interning higher up in the call hierarchy but this is the scope of another bug.
Released patch (with updated copyright stmts) to HEAD.
