RFR(S): 8131129: Attempt to define a duplicate BMH$Species class
Peter Levart
peter.levart at gmail.com
Fri Nov 6 08:58:31 UTC 2015
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
More information about the core-libs-dev
mailing list