RFR: 8139457: Array bases are aligned at HeapWord granularity [v50]
Roman Kennke
rkennke at openjdk.org
Tue Aug 15 17:11:29 UTC 2023
On Tue, 15 Aug 2023 16:18:49 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:
> On master, `header_size` and `base_offset_in_bytes` are two public API; they refer to the header and payload of an array and have the invariant, header-size-in-bytes == base-offset-in-bytes, because `base_offset_in_bytes(...)` is impled as `header_size(...) * HeapWordSize`.
>
> With the new change, `header_size` and `base_offset_in_bytes` are still two public API, but the invariant is broken, `header_size(...) * HeapWordSize != base_offset_in_bytes(...)`. I am asking if we can expose `header_size_in_bytes` as the public API so that invariant is kept.
So you want:
static int base_offset_in_bytes() {
return header_size_in_bytes();
}
??
Seems like an unnecessary duplication to me.
> > 1. it would be the same as base_offset_in_bytes()
>
> True; the impl on master is just a word-to-byte translation. (They being the same is an advantage, reinforcing the invariant, IMO.)
Yes, but that distinction is gone. We shall use only the byte-size version, except for the case where we really want the up-aligned word-sized version (this needs to be used with caution, see below).
> > 2. we still need 'just the header' in places
>
> Is that a public API? I think the concept of "header" contains everything from array-start to payload-start, from the perspective of users (of this class).
No. All users are code-generators (except for base_offset_in_bytes() and header_size() which transform the value in their own ways). And we are using the header-size *without* the gap in order to determine if we need an extra store to initialize the gap. E.g.:
if (!is_aligned(arrayOopDesc::header_size_in_bytes(), BytesPerWord)) {
assert(is_aligned(arrayOopDesc::header_size_in_bytes(), BytesPerInt), "must be 4-byte aligned");
strw(zr, Address(obj, arrayOopDesc::header_size_in_bytes()));
}
> > it's kinda ugly, but having to have to align-up the byte-size in all those places has been ugly as well
>
> I am more concerned about the fact that the public API `header_size` covers more than the header. Having the header-size aligned up to word-size (or anything) should be sth the caller decides.
Ok I agree. I've done that, and ended up with a dozen (or so) cases which all did the exact same thing. That's why I added back header_size() (I originally removed it) as a helper method for the callers which decided to do that. I'm open to suggestions for improving the situation.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/11044#discussion_r1294887659
More information about the hotspot-dev
mailing list