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

[Peter Levart]

On 11/05/2015 08:05 PM, Peter Levart wrote:
> Hi Michael,
>
> The test failure exposed a weakness in the BMH subclass generation and 
> initialization which is fixed by a patch to BMH. OTOH the test will 
> still fail next time with a different exception but that is another 
> issue, I agree.

We can 1st push the fix for 8131129 and then wait for test failure to 
confirm the expectations and then file another issue for fixing the test(s).

Regards, Peter

>
> Regards, Peter
>
> On 11/05/2015 05:34 PM, Michael Haupt wrote:
>> Hi Peter,
>>
>> but this is more like a separate RFE for improving the testing infrastructure, right? Or am I missing something? Unless this is important for the fix to 8131129, I'd like to keep it separate.
>>
>> Best,
>>
>> Michael
>>
>>> Am 05.11.2015 um 17:25 schrieb Peter Levart<peter.levart at gmail.com>:
>>>
>>>
>>>
>>> On 11/05/2015 02:16 PM, Michael Haupt wrote:
>>>> Hi Claes, Peter, Vladimir, all,
>>>>
>>>> I really support Peter's latest proposal: it reduces the amounts of code, synchronisation, and complication, and actually addresses the problem at hand too. What's more to wish for? :-)
>>>>
>>>> I propose to adopt this solution:http://cr.openjdk.java.net/~plevart/jdk9-dev/BMH.race/webrev.03/
>>>>
>>>> Not sure how to proceed. Peter's name should be on the changeset. As he's a Committer, I'm fine if he pushes it once it's signed off by a Reviewer.
>>>>
>>>> Thanks,
>>>>
>>>> Michael
>>> Hi Michael, Vladimir, Claes, all,
>>>
>>> When I try the following example:
>>>
>>> public class Test {
>>>
>>>     static class Failure {
>>>         static final int value;
>>>         static {
>>>             if (true) {
>>>                 throw new InternalError("Failure");
>>>             }
>>>             value = 123;
>>>         }
>>>     }
>>>
>>>     public static void main(String[] args) {
>>>         try {
>>>             Field valueField = Failure.class.getDeclaredField("value");
>>>             int value = valueField.getInt(null);
>>>             System.out.println("Got value: " + value);
>>>         } catch (Throwable e) {
>>>             System.out.println("\n1st exception:\n");
>>>             e.printStackTrace(System.out);
>>>         }
>>>
>>>         try {
>>>             Field valueField = Failure.class.getDeclaredField("value");
>>>             int value = valueField.getInt(null);
>>>             System.out.println("Got value: " + value);
>>>         } catch (Throwable e) {
>>>             System.out.println("\n2nd exception:\n");
>>>             e.printStackTrace(System.out);
>>>         }
>>>     }
>>> }
>>>
>>>
>>> ...I get the following output:
>>>
>>>
>>> 1st exception:
>>>
>>> java.lang.InternalError: Failure
>>>     at Test$Failure.<clinit>(Test.java:12)
>>>     at sun.misc.Unsafe.ensureClassInitialized(Native Method)
>>>     at sun.reflect.UnsafeFieldAccessorFactory.newFieldAccessor(UnsafeFieldAccessorFactory.java:43)
>>>     at sun.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:142)
>>>     at java.lang.reflect.Field.acquireFieldAccessor(Field.java:1088)
>>>     at java.lang.reflect.Field.getFieldAccessor(Field.java:1069)
>>>     at java.lang.reflect.Field.getInt(Field.java:574)
>>>     at Test.main(Test.java:21)
>>>
>>> 2nd exception:
>>>
>>> java.lang.NoClassDefFoundError: Could not initialize class Test$Failure
>>>     at sun.misc.Unsafe.ensureClassInitialized(Native Method)
>>>     at sun.reflect.UnsafeFieldAccessorFactory.newFieldAccessor(UnsafeFieldAccessorFactory.java:43)
>>>     at sun.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:142)
>>>     at java.lang.reflect.Field.acquireFieldAccessor(Field.java:1088)
>>>     at java.lang.reflect.Field.getFieldAccessor(Field.java:1069)
>>>     at java.lang.reflect.Field.getInt(Field.java:574)
>>>     at Test.main(Test.java:30)
>>>
>>>
>>> The 1st attempt that also tries to initialize the class throws InternalError directly from <clinit>, but the 2nd attempt is a kind of LinkageError, which is a different type. So in order for the test(s) to pass, we have to consider this secondary exception too as a non-critical exception:
>>>
>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/BMH.race/webrev.04/  <http://cr.openjdk.java.net/~plevart/jdk9-dev/BMH.race/webrev.04/>
>>>
>>> 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.
>>>
>>> Regards, Peter
>>>
>>>>> Am 05.11.2015 um 12:07 schrieb Claes Redestad<claes.redestad at oracle.com>:
>>>>>
>>>>>
>>>>> On 2015-11-05 08:00, Peter Levart wrote:
>>>>>> Ok, here it is:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/BMH.race/webrev.03/
>>>>>>
>>>>>> I just moved CACHE registration into SpeciesData.initForBootstrap() method, so no new method is needed. Note that patched source now contains the same number of lines as original. How does the jigsaw HelloWorld score now?
>>>>>>
>>>>>> Regards, Peter
>>>>> Thanks, this looks really good to me. I verified that #classes are the same and it even looks like footprint improves ever so slightly.
>>>>>
>>>>> /Claes
>