Hi Claes,

Your patch is OK from logical point of view, but something bothers me a little. For each species data that gets cached, at least 3 objects are created:

- the Function passed to computeIfAbsent
- the unresolved SpeciesData object that serves as a placeholder
- the 2nd resolved SpeciesData object that replaces the 1st

The 1st SpeciesData object serves just as placeholder, so it could be lighter-weight. An java.lang.Object perhaps? Creation of Function object could be eliminated by using putIfAbsent instead of computeIfAbsent then. For the price of some unsafe casts that are contained to a single method that is the sole user of CHM cache.

For example, like this:

http://cr.openjdk.java.net/~plevart/jdk-dev/8202184_ClassSpecializer/webrev.01/

I won't oppose to your version if you find it easier to understand. I just hat to try to do it without redundant creation of placeholder SpeciesData object.

What do you think?

Regards, Peter

On 04/24/2018 06:57 PM, Claes Redestad wrote:
Hi,

the current implementation of ClassSpecializer.findSpecies may cause
excessive blocking due to a potentially expensive computeIfAbsent, and
we have reason to believe this might have been cause for a few very
rare bootstrap issues in tests that attach debuggers to VM in the middle
of this.

Breaking this apart so that code generation and linking is done outside
of the computeIfAbsent helps minimize the risk that a thread gets
interrupted by a debugger at a critical point in time, while also removing a few limitations on what we can do from a findSpecies, e.g., look up other
SpeciesData.

Bug: https://bugs.openjdk.java.net/browse/JDK-8202184
Webrev: http://cr.openjdk.java.net/~redestad/8202184/open.00/

This implementation inserts a placeholder, unresolved SpeciesData,
and then replaces it with another instance that is linked together
fully before publishing it, which ensure safe publication. There might
be alternatives involving volatiles and/or careful fencing, but I've not
been able to measure any added startup cost from this anyway.

Thanks!

/Claes

Reply via email to