[RFR] (M) 8143608: Don't 64-bit align start of InstanceKlass vtable, itable, and nonstatic_oopmap on 32-bit systems

[Chris Plummer]

Hello,

I'm doing a bit of a reset on this thread. I've updated the webrev. The 
only change is fixing SA to also no longer do the aligning:

https://bugs.openjdk.java.net/browse/JDK-8143608
http://cr.openjdk.java.net/~cjplummer/8143608/webrev.03/webrev.hotspot/

I've also filed the following CRs for pre-existing bugs instead of 
addressing them with these changes:

https://bugs.openjdk.java.net/browse/JDK-8145627 -
     sun.jvm.hotspot.oops.InstanceKlass::getSize() returns the incorrect 
size and has no test
https://bugs.openjdk.java.net/browse/JDK-8145628
hotspot metadata classes shouldn't use HeapWordSize or heap related 
macros like align_object_size

And just restating here why I think the removal of these 
align_object_field references is safe:

The argument to align_object_offset() is the offset in words, and it 
returns the offset in words after aligning to a HeapWordsPerLong 
boundary. Since the argument is in words, there is no concern with my 
changes removing word alignment of something that is not already word 
aligned. It must already be word aligned or the current implementation 
would be broken.

Since HeapWordsPerLong is 1 on 64-bit, the end result is that 
align_object_offset() is a no-op, so the removal is having no impact on 
64-bit.

On 32-bit HeapWordsPerLong is 2, so the end result is that the offset 
will remain 32-bit word aligned, and not be 64-bit aligned. This is in 
fact the goal of this change, so what remains is to show that 64-bit 
alignment of the fields is not needed on 32-bit. Since all of the fields 
for which alignment has been removed are 32-bit types, it is safe to 
keep them 32-bit word aligned instead of making them 64-bit aligned.

thanks,

Chris



On 12/4/15 2:02 PM, Chris Plummer wrote:
> Hello,
>
> Please review the following:
>
> https://bugs.openjdk.java.net/browse/JDK-8143608
> http://cr.openjdk.java.net/~cjplummer/8143608/webrev.00/webrev.hotspot/
>
> A bit of background would help. The InstanceKlass object has a number 
> of variable length fields that are laid out after the declared fields. 
> When an InstanceKlass object is allocated, extra memory is allocated 
> for it to leave room for these fields. The first three of these fields 
> are vtable, itable, and nonstatic_oopmap. They are all arrays of 
> HeapWord sized values, which means void* size, which means they only 
> need 32-bit alignment on 32-bit systems. However, they have always 
> been 64-bit aligned. This webrev removes the forced 64-bit alignment 
> on 32-bit systems, saving footprint.
>
> This change affects all 32-bit platforms. It should have no net impact 
> on 64-bit platforms since the fields remain (naturally) 64-bit aligned 
> (unless of course I've introduced a bug). The intent is to not change 
> what is done for 64-bit platforms.
>
> BTW, there is a change to AARCH64, which may seem odd at first. It 
> just removes an "if" block where the condition should always have 
> evaluated to false, so it should have no net affect.
>
> Tested with JPRT "-testset hotspot". Please let me know if you think 
> there are any additional tests that should be run.
>
> thanks,
>
> Chris
>