Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.
Same goal as https://github.com/freemarker/freemarker/pull/2 however it would be nice if this were part of 2.3.x. Please find attached patch.
Reduce locking on the class cache
It imports imports java.util.concurrent.ConcurrentHashMap... that's there only since J2SE 1.5. 2.3.x only requires 1.2.
Johno Crawford's 2nd bacward-compatible patch
I have just attached your patch (sent to me in e-mail) that should be backward compatible...
Moved ConcurrentMapFactory to utility package
I believe it's not safe to call those methods outside synchronization, because the new items are added to the map concurrently. Or is there something in the API that says otherwise?
But anyway, decreasing locking on the class cache is hight priority TODO, so whenever 2..3.20 is released it will be almost surely part of it. (The problem that will addressed there is that the map remains locked during introspection. After all, doing a Map.get inside sync. block is not a big deal, until the instrospection starts blocking gets. Using ConcurrentHashMap on Java 1.5+ was also considered, but I'm nor sure what was the result... maybe that it's overhead killed its advantage if there's was no long locking anyway.)
This is now fixed in the 2.3.x head. However, 2.3.20 is only expected on the end of May. But until that, I think it's already safe to use.
To be clear, actually I haven't used this patch, as I did other refactorings too. But it uses ConcurrentHashMap when available, so it's similar in that respect.
Hey Daniel, thanks for fixing this concurrency issue in 2.3.20.
I was wondering if it was a conscious decision to not include this in freemarker.ext.beans.OverloadedMethodsSubset.selectorCache? I found this to be a bottle neck on one of our pages and fixed it in our fork which just uses a ConcurrentHashMap.
I ask this because I see a TODO
// TODO: make it not concurrent
private final Map selectorCache = new HashMap();
I thought that's not very important as each method name has different lock, and so I haven't yet fixed that... But sure, I should. I add this to the 2.3.21 TODO.