RFR (M) Close alignment gaps in InstanceKlass

[coleen.phillimore at oracle.com]

Thanks, Dean!
Coleen

On 4/23/20 4:16 PM, Dean Long wrote:
> OK, thanks, looks good!
>
> dl
>
> On 4/22/20 7:32 PM, coleen.phillimore at oracle.com wrote:
>>
>>
>> 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
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>