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

Claes Redestad claes.redestad at oracle.com
Wed Nov 4 20:12:59 UTC 2015


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 

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:


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



>> Best,
>> Michael 

More information about the core-libs-dev mailing list