RFR(S) 8182473: Optimize value type layout to max alignment of fields

[David Simms]

Hi Frederic,

Thanks for taking a closer look at this...

On 29/06/17 18:45, Frederic Parain wrote:
> Mr Simms,
>
> I’ve run the ValueTypeCreation test with -XX:+PrintValueLayout to see the
> layout of the different value types.
> Here’s the output:
>
> java.lang.__Value: field layout
>    @ 16 --- instance fields start ---
>    @ 16 --- instance fields end ---
>    @ 16 --- instance ends ---
>    @112 --- static fields start ---
>    @112 --- static fields end ---
>
>    OopMapBlocks:   0  /  0
>
> runtime.valhalla.valuetypes.Point: field layout
>    @ 16 --- instance fields start ---
>    @ 16 "x" I
>    @ 20 "y" I
>    @ 24 --- instance fields end ---
>    @ 24 --- instance ends ---
>    @112 --- static fields start ---
>    @112 --- static fields end ---
>
>    OopMapBlocks:   0  /  0
>
> runtime.valhalla.valuetypes.Long8Value: field layout
>    @ 16 --- instance fields start ---
>    @ 16 "longField1" J
>    @ 24 "longField2" J
>    @ 32 "longField3" J
>    @ 40 "longField4" J
>    @ 48 "longField5" J
>    @ 56 "longField6" J
>    @ 64 "longField7" J
>    @ 72 "longField8" J
>    @ 80 --- instance fields end ---
>    @ 80 --- instance ends ---
>    @112 --- static fields start ---
>    @112 --- static fields end ---
>
>    OopMapBlocks:   0  /  0
>
> runtime.valhalla.valuetypes.Person: field layout
>    @ 16 --- instance fields start ---
>    @ 16 "id" I
>    @ 20 "firstName" Ljava.lang.String;
>    @ 24 "lastName" Ljava.lang.String;
>    @ 32 --- instance fields end ---
>    @ 32 --- instance ends ---
>    @112 --- static fields start ---
>    @112 --- static fields end ---
>
>    OopMapBlocks:   1  /  2
>      Offset:  20  - 24 Count:   2
>
> runtime.valhalla.valuetypes.ValueTypeCreation$SmallEmbed: field layout
>    @ 16 --- instance fields start ---
>    @ 16 "a" B
>    @ 17 "b" B
>    @ 20 --- instance fields end ---
>    @ 24 --- instance ends ---
>    @112 --- static fields start ---
>    @112 --- static fields end ---
>
>    OopMapBlocks:   0  /  0
>
> runtime.valhalla.valuetypes.ValueTypeCreation$Composition: field layout
>    @ 16 --- instance fields start ---
>    @ 16 "a" S
>    @ 18 "se" Qruntime.valhalla.valuetypes.ValueTypeCreation$SmallEmbed;
>    @ 24 "long8Value" Qruntime.valhalla.valuetypes.Long8Value;
>    @ 88 "person" Qruntime.valhalla.valuetypes.Person;
>    @104 --- instance fields end ---
>    @104 --- instance ends ---
>    @112 --- static fields start ---
>    @112 --- static fields end ---
>
>    OopMapBlocks:   1  /  1
>      Offset:  92  - 96 Count:   2
>
> runtime.valhalla.valuetypes.ValueTypeCreation$ValueHolder: field layout
>    @ 12 --- instance fields start ---
>    @ 16 "a" S
>    @ 12 "b" I
>    @ 18 "small" Qruntime.valhalla.valuetypes.ValueTypeCreation$SmallEmbed;
>    @ 24 "comp" Qruntime.valhalla.valuetypes.ValueTypeCreation$Composition;
>    @112 --- instance fields end ---
>    @112 --- instance ends ---
>    @112 --- static fields start ---
>    @112 --- static fields end ---
>
>    OopMapBlocks:   1  /  1
>      Offset: 100  -104 Count:   2
>
>
> The layout of value type runtime.valhalla.valuetypes.Person doesn’t look
> optimal: it looks like there’s a gap between the end of the last field, lastName,
> which has a size of 4 bytes, and the end of the instance field area.
>
> I don’t know of this issue comes from your changes or if it was already
> there before, but as you’re working on the layout_fields() method, could
> you look at this issue?
>
> Thank you,
>
> Fred

There is a gap, you are correct, classFileParser.cpp:4360 still 8 byte 
aligns the end of values, larger than "heapOopSize".

This is related to how "ValueKlass::value_store()" attempts to avoid 
shearing fields. "ValueKlass::raw_value_byte_size()" only returns 1, 2, 
4, "align_size_up(X, 8)". In the case of "Person", we could set a new 
valueKlass "load/store alignment" to 4, but given flat value arrays use 
power of 2 addressing we won't save anything there in terms of storage 
space.

However, for size 12 where max required alignment is 4, yes for non-flat 
arrays the end padding is useless and takes up space, and the 
"NONSTATIC_VALUETYPE" alignment will also waste 4 bytes.

What do you think, is it worth adding "storage alignment" to ValueKlass, 
and increase the amount of logic ? Probably not right ? Consider a value 
with a long and an int field..."value_store" would get messy if we want 
to save 4 bytes there. I'm not sure that is worth it.

Let me know what you think
/D