RFR: 8139457: Array bases are aligned at HeapWord granularity [v52]

[Roman Kennke]

On Tue, 22 Aug 2023 11:52:33 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> See [JDK-8139457](https://bugs.openjdk.org/browse/JDK-8139457) for details.
>> 
>> Basically, when running with -XX:-UseCompressedClassPointers, arrays will have a gap between the length field and the first array element, because array elements will only start at word-aligned offsets. This is not necessary for smaller-than-word elements.
>> 
>> Also, while it is not very important now, it will become very important with Lilliput, which eliminates the Klass field and would always put the length field at offset 8, and leave a gap between offset 12 and 16.
>> 
>> Testing:
>>  - [x] runtime/FieldLayout/ArrayBaseOffsets.java (x86_64, x86_32, aarch64, arm, riscv, s390)
>>  - [x] bootcycle (x86_64, x86_32, aarch64, arm, riscv, s390)
>>  - [x] tier1 (x86_64, x86_32, aarch64, riscv)
>>  - [x] tier2 (x86_64, aarch64, riscv)
>>  - [x] tier3 (x86_64, riscv)
>
> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Move away arrayOopDesc::header_size()

> > Thanks, Stefan for checking this. Yes, arrayOop::header_size() is confusing. The only purpose of this method is to help a few code paths which need the header_size_in_bytes() to be aligned-up and converted to word-size. Not quite sure how to deal with it. Move it out to some different place? Name it differently?
> 
> I want to make sure that we are on the same page about this: Do you agree with my assessment that that arrayOopDesc::header_size() does _not_ align-up arrayOopDesc::header_size_in_bytes() in _all_ configurations, and that that needs to be fixed? That was the main point of my post above.

Ohh, I think I misunderstood. Sorry. Indeed, you are right, that is a bug. Arguably, it's even a pre-existing bug, except that it never materialized because header_size_in_bytes() would previously already answer with an up-aligned value. After my last refactoring, the only remaining place where this was used was arrayOopDesc::max_array_length() and I fixed it there. (On a side-note: that method is in dire need of a rewrite, it is too conservative now and too messy, and, AFAICT, only exists because of 32-bit builds.)

> Personally, I think we could keep the function in arrayOopDesc as long as we give descriptive name that shows the readers that the function returns a something larger than the actual header size. Maybe `aligned_header_size[_in_words]` or `align_up_header_size[_in_words]?

My last refactoring eliminated 1 use (in unsafe.cpp) which I find better. It also eliminated the use in collectedHeap.cpp by reducing it to an inlined version that only deals with T_INT. I also inlined it into max_array_length(), which is now also reduced to a simpler case. I believe it's better that we don't have this method to begin with, it's only confusing.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/11044#issuecomment-1690045932