RFR: 8305895: Implement JEP 450: Compact Object Headers (Experimental) [v26]

Roman Kennke rkennke at openjdk.org
Fri Sep 27 08:27:57 UTC 2024


On Thu, 26 Sep 2024 17:25:06 GMT, Scott Gibbons <sgibbons at openjdk.org> wrote:

>> @sviswa7 or @asgibbons WDYT about including https://github.com/rkennke/jdk/commit/097c2afa04397773e514552dfb942aa889bfa2c1 as part of compact object headers implementation? Otherwise we would have to disable indexOf intrinsic when running with compact headers, because of the assumption that array headers are >= 16 bytes, which is no longer true with compact headers.
>
> @rkennke I reviewed [rkennke@ 097c2af](https://github.com/rkennke/jdk/commit/097c2afa04397773e514552dfb942aa889bfa2c1) and the code looks good to me.  I would prefer this approach instead of not generating the IndexOf intrinsic.
> 
> Should the controlling `if` be conditioned on `UseCompactObjectHeaders` instead of `arrayOopDesc::base_offset_in_bytes`?  I can see benefits to either - which provides more clarity?  I like the assert as it makes the intention clear (thanks!).

I like to have the functional connection: if - for whatever reason - the array base offset is smaller than 16, we need to deal with that. The reason for this happens to be `UseCompactObjectHeaders`, but that may not be clear to the reader of the code. I could add an `assert(UseCompactObjectHeaders` in that branch to make that connection clear. Also consider that `UseCompactObjectHeaders` is intended to go away at some point.

I wonder if having 2 or 3 branches ahead of the main-loop (which probably doesn't do much, because haystack is <=32 bytes) is a useful approach, or if there may be a better way to get the bytes on the stack? I don't know enough about the implementation to make that judgement.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1778230714


More information about the build-dev mailing list