DO NOT REPLY [Bug 46962] Deadlock in PropertyCache class
https://issues.apache.org/bugzilla/show_bug.cgi?id=46962 --- Comment #6 from Alexis Giotis alex.gio...@gmail.com 2011-08-05 20:21:52 UTC --- Created attachment 27357 -- https://issues.apache.org/bugzilla/attachment.cgi?id=27357 Concurrent map based implementation of property cache supporting hashCode collisions -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
DO NOT REPLY [Bug 46962] Deadlock in PropertyCache class
https://issues.apache.org/bugzilla/show_bug.cgi?id=46962 Alexis Giotis alex.gio...@gmail.com changed: What|Removed |Added Attachment #27342|0 |1 is obsolete|| Attachment #27343|0 |1 is obsolete|| Attachment #27357|0 |1 is obsolete|| --- Comment #7 from Alexis Giotis alex.gio...@gmail.com 2011-08-05 20:31:09 UTC --- Created attachment 27358 -- https://issues.apache.org/bugzilla/attachment.cgi?id=27358 Patch with rewritten PropertyCache (trunk revision 1154337) In short, the new PropertyCache implementation attached is: - Up to 3 times faster (depending on the tests, the -Xmx, and the retention or not of strong refs to he cached entries) - 3 times less lines of code - Obviously thread-safe - Written using JDK5 generics - Has similar memory requirements Additionally this patch fixes broken hashCode() and equals() methods of classes extending Property (including the patch in bug 51625). Those caused in tests many hashCode collisions. In more detail, I wrote 2 new implementations of PropertyCache (one in this patch and the one in attachment 27357) and tested both against the original one with the fix contained in attachment 27343. When strong references to the cached entries are kept, then the performance of all is similar. When they are not (more common case), the ones based on the concurrent hash map are up to 3 times faster. The tests were allocating 1M (million) Property instances from which 100K were equal (but different instances). The first implementation based on the concurrent map supports caching not-equal objects with the same hashcode but is fairly complex. The one attached in the final patch does not. After some experimentation and tests with large (1000 page) documents hashcode collisions were caused due to buggy hashCode and equal implementations. Handling this case has a performance penalty. In a test that caches 1M entries from which there are only 100 different hashCodes the time to complete was: 52 seconds for the initial implementation 12 seconds the the concurrent map that can cache not-equal objects with the same hashcode. 1 second for the concurrent map that keeps the more recent one. In other words, in this case (which is due to buggy hashcode()/equals), the cost of creating a new instance and replacing the previously cached one is by far smaller that the one to maintain in memory the different instances. Note that this implementation does not provide any guarantee related to the uniqueness of equal instances when concurrently executed. Such a guarantee is not only complex to code but also it requires additional locking. In practice, it is not very probable that this will happen, finally there will be only one and of course this should be a tolerable situation. After all, the caching can be globally disabled with the same system property as before. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
DO NOT REPLY [Bug 46962] Deadlock in PropertyCache class
https://issues.apache.org/bugzilla/show_bug.cgi?id=46962 --- Comment #4 from Alexis Giotis alex.gio...@gmail.com 2011-08-02 19:45:47 UTC --- Created attachment 27342 -- https://issues.apache.org/bugzilla/attachment.cgi?id=27342 Patch with a unit test to reproduce the deadlock Attached is a patch that includes a unit test (PropertyCacheTest.java) that almost always reproduces the deadlock using just 2 threads. This is not meant to be committed but only as a demonstration of the problem and as a test of next patches that fix it. Except adding the test case, I also needed to add a sleep of 1 second in the rehash() method and implement the equals() and hashcode() of the org.apache.fop.fo.properties.Property class. The Property class was just more easy to instantiate. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
DO NOT REPLY [Bug 46962] Deadlock in PropertyCache class
https://issues.apache.org/bugzilla/show_bug.cgi?id=46962 --- Comment #5 from Alexis Giotis alex.gio...@gmail.com 2011-08-02 22:24:44 UTC --- Created attachment 27343 -- https://issues.apache.org/bugzilla/attachment.cgi?id=27343 Patch fixing PropertyCache issue. Attached is a proposed patch that fixes this issue with minimal changes. The patch is against the revision 1067783 of PropertyCache in FOP trunk. On the other hand, it might better to base the implementation of the PropertyCache on ConcurrentHashMap. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
DO NOT REPLY [Bug 46962] Deadlock in PropertyCache class
https://issues.apache.org/bugzilla/show_bug.cgi?id=46962 --- Comment #1 from Andreas L. Delmelle adelme...@apache.org 2009-04-03 11:53:01 PST --- Thanks for the report. We will investigate this closer ASAP. For now, the only immediate relief would be to switch to FOP Trunk, which allows to disable the PropertyCache via a system property org.apache.fop.fo.properties.use-cache. Set it to false to avoid caching. The drawback is obviously that the processes will all use up more memory (the difference can be quite significant if you have a lot of identical property-specs on a lot of FOs). For the rest, it would also be interesting to know more about the environment. Which Java VM (vendor + version) are you using? If it's GNU Classpath, I'd first try if switching to a Sun VM helps. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
DO NOT REPLY [Bug 46962] Deadlock in PropertyCache class
https://issues.apache.org/bugzilla/show_bug.cgi?id=46962 --- Comment #2 from Andreas L. Delmelle adelme...@apache.org 2009-04-03 12:20:07 PST --- (In reply to comment #1) Just to be complete: For now, the only immediate relief would be to switch to FOP Trunk, ... The addition of the system property to disable caching of the properties apparently never made it into 0.95, but the required modifications are pretty straightforward: - add a 'useCache' member to PropertyCache - in the constructor, initialize it to reflect the value of the system property - in the generic, private fetch() method, if useCache is true, bypass the entire method body, and simply return the parameter instance Apart from that, no easy solution I'm afraid. It comes down to choosing the lesser of two 'evils': * either use the Trunk version, which means, strictly speaking, no guarantees about stability, although there are people who do use it in production environments * or modify the sources in the 0.95 branch, which also leaves you with an unofficial version. Again, we'll be look into it closer soon, but these types of issues are almost always very difficult to reproduce... -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
