RFR: 8319773: Avoid inflating monitors when installing hash codes for LM_LIGHTWEIGHT [v4]

Daniel D. Daugherty dcubed at openjdk.org
Thu Nov 16 18:44:52 UTC 2023


On Thu, 16 Nov 2023 18:36:09 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> The rename from `locked_mark` to `new_mark` seems superfluous. The old and
>> new version of the variable name are used in the same places so I don't quite see
>> the reason for the rename. I liked `locked_mark` because it was the "locked" version
>> of the original `mark`. I do like the use of the `const`.
>
> Now I think I see why you changed the `cas_set_mark()` call to return its value into
> `mark` instead of `old_mark` and why the original parameter to the CAS was changed
> from `mark` to `old_mark`. It's a bit different style then we typically use for CAS where
> the return value goes into an `old_something` variable.
> 
> Since `mark` is now the loop control and `old_mark` is now `const` you had to juggle
> things with the `cas_set_mark()` call. Okay, I think I get the reason for this bit of juggling.
> (I still don't understand the rename from `locked_mark` to `new_mark` but that's okay.)

Okay so this bit of code changes were done for a couple of reasons (of course this is my guess):
1) add a retry here in case of collision to give this thread another chance at doing a lightweight lock instead of always dropping into inflate-enter loop below on a collision
2) clean up the code a little bit with some use of `const` to make things a little more clear.

I'm going to guess that the type of collision that we're trying to avoid is hash code installation.

You may want to add a comment above `L516 while (mark.is_unlocked()) {` that says:

// Allow for a retry here just in case the cas_set_markI() below collided with a hash code installation:

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16603#discussion_r1396180120


More information about the hotspot-dev mailing list