[OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

Jim Graham 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 
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.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
>     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



More information about the 2d-dev mailing list