RFR 8145628: hotspot metadata classes shouldn't use HeapWordSize or heap related macros like align_object_size

[Coleen Phillimore]

Thank you, Chris for looking at this change.

On 1/28/16 4:24 PM, Chris Plummer wrote:
> Hi Coleen,
>
> Can you do some testing with ObjectAlignmentInBytes set to something 
> other than 8?

Okay, I can run one of the testsets with that.  I verified it in the 
debugger mostly.
>
> Someone from GC team should apply your patch, grep for 
> align_object_size(), and confirm that the ones you didn't change are 
> correct. I gave a quick look and they look right to me, but I wasn't 
> always certain if object alignment was appropriate in all cases.

thanks - this is why I'd changed the align_object_size to 
align_heap_object_size before testing and changed it back, to verify 
that I didn't miss any.
>
> I see some remaining HeapWordSize references that are suspect, like in 
> Array.java and bytecodeTracer.cpp. I didn't go through all of them 
> since there are about 428. Do they need closer inspection?
>
> align_metadata_offset() is not used. It can be removed.

Okay, I'll remove it.  That's a good idea.

>
> Shouldn't align_metadata_size()  align to 64-bit like 
> align_object_size() did, and not align to word size? Isn't that what 
> we agreed to? Have you tested CDS? David had concerns about the 
> InstanceKlass::size() not returning the same aligned size as 
> Metachunk::object_alignment().

I ran the CDS tests but I could test some more with CDS.     We don't 
want to force the size of objects to be 64 bit (especially Symbol) 
because Metachunk::object_alignment() is 64 bits. Unfortunately, with 
the latter, metadata is never aligned on 32 bit boundaries for 32 bit 
platforms, but to fix this, we have to pass a minimum_alignment 
parameter to Metaspace::allocate() because the alignment is not a 
function of the size of the object but what is required from its 
nonstatic data members.  I found MethodCounters, Klass (and subclasses) 
and ConstantPool has such alignment constraints.   Not sizing metadata 
to 64 bit sizes is a start for making this change.
>
> instanceKlass.hpp: Need to fix the following comment:
>
>   97   // sizeof(OopMapBlock) in HeapWords.
Fixed, Thanks!

Coleen

>
> thanks,
>
> Chris
>
>
> On 1/27/16 10:27 AM, Coleen Phillimore wrote:
>> Summary: Use align_metadata_size, align_metadata_offset and 
>> is_metadata_aligned for metadata rather
>> than align_object_size, etc.  Use wordSize rather than HeapWordSize 
>> for metadata.  Use align_ptr_up
>> rather than align_pointer_up (all the related functions are ptr).
>>
>> Ran RBT quick tests on all platforms along with Chris's Plummers 
>> change for 8143608, ran jtreg hotspot tests and nsk.sajdi.testlist 
>> co-located tests because there are SA changes.   Reran subset of this 
>> after merging.
>>
>> I have a script to update copyrights on commit. It's not a big 
>> change, just mostly boring.  See the bug comments for more details 
>> about the change.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8145628.01/
>> bug link https://bugs.openjdk.java.net/browse/JDK-8145628
>>
>> thanks,
>> Coleen
>>
>>
>