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