RFR(S): 8131129: Attempt to define a duplicate BMH$Species class

[Claes Redestad]

Hi,

On 2015-11-04 13:18, Peter Levart wrote:
> Here's what I am thinking, in code:
>
> http://cr.openjdk.java.net/~plevart/jdk9-dev/BMH.race/webrev.02/
>
> Now that definition of BMH subclass is atomic, caching of SpeciesData 
> can be simplified. We don't need special placeholder instances as 
> locks and synchronized static methods. To make BMH subclass definition 
> atomic, we can leverage CHM.computeIfAbsent that does the similar 
> "placeholder" dance, but in much more sophisticated way. BMH logic is 
> much more straightforward and easier to grasp that way.
>
> So what do you think of this version. Your version is logically 
> correct too, so you decide which one is better.

I've been tracking this patch for a bit since it's startup sensitive, 
and suggested some improvements offline to avoid the reflection lookups 
on non-asserting code that's been rolled into this patch.

I gave both patches here a spin and noticed that Peter's variant pulls 
in some 6 extra classes on a jigsaw Hello World test I'm playing with 
(such as ConcurrentHashMap$BaseIterator). Not a strong argument in 
itself, but if there's no stronger reason for your version than to clean 
this up a bit I'd vote in favor of Michael's approach...

> Regards, Peter
>
> On 10/29/2015 04:20 PM, Michael Haupt wrote:
>> Hi Vladimir, Peter,
>>
>> once more, thanks for all your comments. The revised webrev is at 
>> http://cr.openjdk.java.net/~mhaupt/8131129/webrev.01/.

  however, the access to FAILED_SPECIES_CACHE doesn't seem to be 
thread-safe and needs to be synchronized with a static lock object in 
BoundMethodHandle (initiating different SpeciesData concurrently might 
lead to ConcurrentModificationException when accessing or putting values 
into FAILED_SPECIES_CACHE.

I'd suggest cleaning up the synchronized methods to lock on specific 
objects while we're at it, and maybe should initialize 
FAILED_SPECIES_CACHE as Collections.emptyList(), since it'll typically 
never be used anyhow:

http://cr.openjdk.java.net/~redestad/scratch/bmh.race.01/

Perhaps this clunky implementation is an argument in favor of Peter's 
approach, but it keeps class count in check.

Thanks!

/Claes

>>
>> Best,
>>
>> Michael