RFR: 8202184: Reduce time blocking the ClassSpecializer cache creating SpeciesData

[Claes Redestad]

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
>