[OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering
bourges.laurent at gmail.com
Tue Feb 9 22:10:41 UTC 2016
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
- ReentrantContextProviderTL: enhanced Thread Local provider that uses
another ReentrantContextProviderCLQ if reentrance detected
These 2 last implementations supports any reference wrapper as given in
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>:
> 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
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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the 2d-dev