One suggestion on ProviderTL - have a constructor overload that allows
separate ref types for the TL and the CLQ references in case there is a
situation where you want a hard reference for the primary context and a
soft reference for the reentrant ones (which may be situationally rare).
The single refType constructor would use the same refType for both
cases...
...jim
On 2/9/16 2:10 PM, Laurent Bourgès wrote:
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
<mailto: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
