[OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering
james.graham at oracle.com
Wed Feb 10 21:32:02 UTC 2016
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
On 2/9/16 2:10 PM, Laurent Bourgès wrote:
> 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:
> 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.graham at oracle.com
> <mailto:james.graham at 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
> 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.
More information about the 2d-dev