Hi all
Been reading up a bit on concurrency and synchronization and the likes, and I'm in the meantime quite convinced that the strategy should indeed be altered to move away from static final Maps. Don't know if Richard is still listening in and has any opinions/ideas?
Without doing any measurements, one can't say for certain, but I'm guessing the risk of thread contention is reasonably high. OTOH, I also haven't measured how much time each thread would actually spend parsing properties. If that percentage is only marginal, then the risk would be not be immediately catastrophic, unless in case of a very large amount of concurrent threads where chances increase that two threads need the same Map at the exact same instance.
In the meantime, I'll see if I can think up some tests/timings and make them part of the build. Jeremias wants to measure this for the past evolution, and I'd like to make it easier for us to track it in the future. In the longer term, we could maybe set up a page with statistics for this on the website (?)
That said, Java 1.5 has added a neat util.concurrent package with, besides the famous Semaphore, for example, a ConcurrentHashMap implementation. Supposed to perform far better in case of heavy contention, since it never locks the entire underlying table. Tricky implementation to simulate under 1.4 though, AFAIU. :-(
The benefit remains, though, since in the above example, given the results Jeremias provided, the save on those different Property instances together can easily mean a couple MB of extra heap per thread.
I've been playing with trying to make the cache thread-local, but I did not immediately find a solution for situations like: - NumberProperty.getLength() has no context at all, but requires access to the cache of FixedLengths. Seems like NumberProperty should get a reference to the document it belongs to --a direct reference to the Root?-- from where it could gain access to a document-global cache of properties... The increase due to the reference is still insignificant compared to the save on distinct instances, and it would make the caches guaranteed thread-safe (since FOP is sequential internally) without the need for synchronization.
On the other hand, as mentioned in Bugzilla 41044, I've been thinking about using the PropertyMakers to our advantage here. Make all Property constructors protected, so only the Makers can access them in their make() methods. Nowhere outside the properties package should any class ever instantiate a NumberProperty (note: not even the PropertyParser, that should use a NumberPropertyMaker to get to an instance)
FlyWeightFactories? Good idea! I think we have them already, only, they are not exactly configured yet to flyweight anything.
I'm going to make this a priority, together with doing something similar for CommonHyphenation and CommonFont (which could become a candidate if it were only decomposed in font-size + rest of the properties...).
Just so you know :) Cheers Andreas
