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

[Roman Kennke]

On Fri, 27 Sep 2024 14:44:35 GMT, Scott Gibbons <sgibbons at openjdk.org> wrote:

>> 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.
>
> I believe the code in the patch is good enough as-is, especially if `UseCompactObjectHeaders` is slated to go away.  The existing `if` will prevent the < 16 byte header code from being emitted, which is the desired behavior - i.e., if the header size is >= 16, there will be no code emitted to the intrinsic for that block.  So there will not be an additional branch for the code when it is executed.
> 
> I'm good with a comment tying `UseCompactObjectHeaders` to the condition.  The comment can be removed when the flag is removed.  "Ship it" :-)

Wait a second, I've probably not been clear. `UseCompactObjectHeaders` is slated to become *on by default* and then slated to go away. That means that array base offets <= 16 bytes will become the default. The generated code will be something like:


if (haystack_len <= 8) {
  // Copy 8 bytes onto stack
} else if (haystack_len <= 16) {
  // Copy 16 bytes onto stack
} else {
  // Copy 32 bytes onto stack
}


So that is 2 branches in this prologue code instead of originally 1.

However, I just noticed that what I proposed is not enough. Consider what happens when haystack_len is 17. This would take the last case and copy 32 bytes. But we only have 17+8=25 bytes that we can guarantee to be available for copying. If this happens to be the array at the very beginning of the heap (very rare/unlikely), this would segfault.

I think I need to mull over it some more to come up with a correct fix.

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

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