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
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
More information about the core-libs-dev
mailing list