Jim, Thanks to your comments, I generalized the TL / CLQ approach in sun.java2d package with new classes: - ReentrantContext: base class for thread contexts with few package-visible fields: usage and reference - ReentrantContextProvider: base provider class that define the abstract methods to be implemented + several reference wrappers (hard, soft, weak) - ReentrantContextProviderCLQ: very simple provider that only use a CLQ for storage - ReentrantContextProviderTL: enhanced Thread Local provider that uses another ReentrantContextProviderCLQ if reentrance detected
These 2 last implementations supports any reference wrapper as given in their constructor. Here is the updated webrev: http://cr.openjdk.java.net/~lbourges/marlin/marlin-8148886.3/ Please give me your point of view; is it the good direction ? (javadoc / comments remain to be fixed) Of course, the new classes are used by both AAShapePipe and MarlinRenderingEngine and it works well: performance is still the same but all tests are passing. Few anwsers below: 2016-02-08 23:33 GMT+01:00 Jim Graham <james.gra...@oracle.com>: > You might want to separate the new ReentrantTL class into a separate file > in sun.java2d so it can be used in other places. What are the hurdles for > using it in Marlin instead of rolling its own? If it is going to be reused > it might be better to represent the ReentrantContext class as an interface > so that objects with pre-determined super-class hierarchies can still > participate. > Done as proposed; the 'hurdles' were that I want to keep high performance and may prefer sometimes code duplication ... but my first tests seem indicating that the new approach is equivalent as before. > MarlinRenderingEngine line 182 - testing useThreadLocal is > redundant/irrelevant here, checking the queue variable should be enough. > (it was a trick as hotspot optimizes dead code: the condition !useThreadLocal may let hotspot remove a useless block) > ReentrantThreadLocal - depth is a poor name for that field. Perhaps > usage? With "*_TL_INACTIVE", "*_TL_IN_USE", and "*_CLQ" being the variants? > Agreed: I adopted your proposal. > > AAShapePipe line 297 - should it double check that the level is > DEPTH_CLQ? Otherwise someone can submit an undefined TL object for > restore() and it will end up in the queue... > I added the double-check as recommended. Cheers, Laurent
