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

[Peter Levart]

Hi all,

On 11/06/2015 01:00 AM, Peter Levart wrote:
> Hi Vladimir,
>
> On 11/05/2015 11:06 PM, Vladimir Ivanov wrote:
>> Peter, Michael,
>>
>> Very good work! I really like how CMH-based BMH & SpeciesData caches 
>> shape out in your proposal.
>>
>>>
>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/BMH.race/webrev.04/
>>
>> Small cleanup suggestion:
>> +        static boolean speciesDataCachePopulated() {
>> +            Class<BoundMethodHandle> rootCls = BoundMethodHandle.class;
>>              try {
>>                  for (Class<?> c : rootCls.getDeclaredClasses()) {
>>                      if (rootCls.isAssignableFrom(c)) {
>>                          final Class<? extends BoundMethodHandle> 
>> cbmh = c.asSubclass(BoundMethodHandle.class);
>>
>> Maybe get rid of multiple BMH mentions here and just use Class<?> and 
>> rootCls?
>>
>>> I think this is better then trying to disguise NoClassDefFoundError 
>>> into
>>> a VirtualMachineError just in order to "fool" the test(s). This is how
>>> VM operates and in general, Error(s) thrown by VM should be propagated
>>> up to the top and reported as-is.
>> We already discussed that aspect. It's not only to fool the tests, 
>> but also to provide more meaningful error reporting. I don't think 
>> that NoClassDefFoundError is satisfactory. Remember, we started our 
>> investigations with a similar type of errors.
>>
>> There are different ways to address the problem, but the most 
>> sensible IMO is:
>>   (1) split class initialization and Species instantiation;
>>
>>   (2) on consecutive attempts after a failed one: either (a) repeat 
>> SpeciesData instantiation; or (b) cache first exception and return it.
>>
>> I'd try to instantiate corresponding Species right away and act 
>> accordingly. CLASS_CACHE.computeIfAbsent() in getConcreteBMHClass 
>> guarantees unique call per key, so no need to piggyback on class 
>> initialization anymore. CLASS_CACHE (or CACHE entries?) can be used 
>> to store initialization state.
>>
>> What do you think?
>
> If the exception is from SpeciesData instantiation (yes it is, 
> Michael's last stack traces show that!), then there's no reason to 
> fail concrete BMH subclass initialization because of that.
>
> Here's the attempt:
>
> http://cr.openjdk.java.net/~plevart/jdk9-dev/BMH.race/webrev.05/
>
> I opted for (a) - repeat SpeciesData instantiation. Who knows, maybe 
> code cache overflow is just a transient condition and retrying may 
> actually succeed? Besides, it was simpler. I just leveraged another 
> computeIfAbsent.
>
> I haven't tested this code yet. Will do that tomorrow morning. Now I'm 
> going to sleep.
>
> Regards, Peter

I made a couple of adjustments:

     http://cr.openjdk.java.net/~plevart/jdk9-dev/BMH.race/webrev.06/

- the SPECIES_DATA field in generated class can not be static final when 
it is initialized out of <clinit>. Just static. It was just static 
before when it was initialized in <clinit> and could be static final. We 
can make it @Stable at least.

- there's no need for empty <clinit> method now. Removed.

Basic java/lang/invoke jtreg tests pass.

Regards, Peter