RFR (M) Close alignment gaps in InstanceKlass

[coleen.phillimore at oracle.com]

On 4/22/20 9:00 PM, Dean Long wrote:
> It looks like calling the JVMCI getSourceFileName() on an array would 
> have accessed random memory because it was expecting an 
> InstanceKlass.  Instead of returning null we might want to throw an 
> exception like in HotSpotResolvedPrimitiveType.
It was never called, except when I tried to call it on an array in the 
test.  It caused an assert in the hotspot code. How about this? 
Something else in that file throws JVMCIError with a similar message.

     public String getSourceFileName() {
         if (isArray()) {
             throw new JVMCIError("Cannot call getSourceFileName() on an 
array klass type: %s", this);
         }
         return getConstantPool().getSourceFileName();
     }

>
> dl
>
> On 4/22/20 5:39 PM, Dean Long wrote:
>> Can you compare the result to some string, like "Object.java"?  That 
>> seems to be what HotSpotResolvedObjectTypeTest.java is doing.
>> Also, did getSourceFileName() return null for arrays before your change?

Fixed:
     public void getSourceFileNameTest() {
         Class<?> c = Object.class;
         ResolvedJavaType type = metaAccess.lookupJavaType(c);
         assertEquals(type.getSourceFileName(), "Object.java");
     }

thanks,
Coleen

>>
>> dl
>>
>> On 4/22/20 8:18 AM, coleen.phillimore at oracle.com wrote:
>>>
>>> Hi Dean, Thank you for looking at the JVMCI changes and the 
>>> suggestion to add the test.  I did this and found a bug.  The new 
>>> test is quite limited because there's no good test to see if a 
>>> source file name can assertNotNull(type.getSourceFileName()), so I 
>>> couldn't iterate through the list of loaded classes like the other 
>>> tests in that file.
>>>
>>> http://cr.openjdk.java.net/~coleenp/2020/8238048.02.incr/webrev/index.html 
>>>
>>>
>>> Thanks,
>>> Coleen
>>>
>>>
>>> On 4/21/20 9:51 PM, Dean Long wrote:
>>>> Hi Coleen.  The JVMCI changes look OK. It looks like there is a 
>>>> Graal unittest that covers getSourceFileName, but those tests don't 
>>>> always get run.  If it's not too much trouble, could you look into 
>>>> enabling getSourceFileName() testing in
>>>>
>>>> test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java 
>>>>
>>>>
>>>> ?  It's currently on the "untested" list.
>>>>
>>>> thanks,
>>>>
>>>> dl
>>>>
>>>> On 4/21/20 1:12 PM, coleen.phillimore at oracle.com wrote:
>>>>> Summary: moved fields around and some constant fields into 
>>>>> ConstantPool
>>>>>
>>>>> This is a simple change except that I moved some constant fields 
>>>>> from InstanceKlass into the constant pool so they can be shared 
>>>>> read-only in the CDS archive.  There are associated repercussions 
>>>>> in SA and JVMCI, so please look at these changes. Also moved 
>>>>> similarly sized fields together in the class so there's less 
>>>>> likelihood of introducing gaps in future InstanceKlass changes.
>>>>>
>>>>> InstanceKlass is reduced from 544 to 520 bytes in a simple Hello 
>>>>> World class.
>>>>>
>>>>> open webrev at 
>>>>> http://cr.openjdk.java.net/~coleenp/2020/8238048.01/webrev
>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8238048
>>>>>
>>>>> Tested with tier1-6.
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>>
>>>>>
>>>>
>>>
>>
>