Hi Peter,
too late! :-)
Good observation about us creating a new Function every time we look up
an item. If we go with an Object as a reservation marker we could get
away with a singleton static Function and still use computeIfAbsent
(which I think is clearer and avoids creating a reservation Object on
every lookup). I'd consider this a reasonable follow-up RFE:
http://cr.openjdk.java.net/~redestad/scratch/ClassSpec_followup.00/
Its value as an optimization might be somewhat dubious, but it does help
future-proof the mechanism (say if we wanted to add forms that are even
more heavyweight or if we needed to side-effect something when calling
newSpeciesData...).
/Claes
On 2018-04-25 18:45, Peter Levart wrote:
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