RFR: 8351655: Optimize ObjectMonitor::unlink_after_acquire()

[David Holmes]

On Mon, 17 Mar 2025 13:11:58 GMT, Fredrik Bredberg <fbredberg at openjdk.org> wrote:

> This PR implements an optimization of `ObjectMonitor::unlink_after_acquire()`. If it's possible it only converts the first part (the singly linked part) of the `entry_list` into a doubly linked list and leave the _entry_list_tail pointer untouched. 
> 
> It has passed tier1-tier8 tests.

Generally okay but a few minor nits - plus Patricio's suggestion.

Thanks

src/hotspot/share/runtime/objectMonitor.cpp line 1256:

> 1254: // pointers and the entry_list_tail pointer. Within the entry_list the
> 1255: // next pointers always form a consistent singly linked list. The
> 1256: // entry_list can be in either of these forms:

Suggestion:

// entry_list can be in any of these forms:

"either" implies two choices

src/hotspot/share/runtime/objectMonitor.cpp line 1264:

> 1262: // Number four is because new threads has pushed themself to the
> 1263: // entry_list head after the entry_list was last converted into a
> 1264: // doubly linked list.

I suggest deleting this as we already know it from the description of how things work.

src/hotspot/share/runtime/objectMonitor.cpp line 1273:

> 1271:   while (w != nullptr) {
> 1272:     assert(w->TState == ObjectWaiter::TS_ENTER, "invariant");
> 1273:     assert(w->prev() == nullptr || w->prev() == prev, "should be");

Suggestion s/should be/invariant/ for all these assertions

src/hotspot/share/runtime/objectMonitor.cpp line 1297:

> 1295:     // and we don't have the tail pointer, something is broken.
> 1296:     assert(_entry_list_tail != nullptr, "should be");
> 1297: #ifdef ASSERT

I would move this `#ifdef ASSERT` to before the comment to make it clear the comment pertains to debug code.

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

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24078#pullrequestreview-2701100435
PR Review Comment: https://git.openjdk.org/jdk/pull/24078#discussion_r2004764666
PR Review Comment: https://git.openjdk.org/jdk/pull/24078#discussion_r2004852415
PR Review Comment: https://git.openjdk.org/jdk/pull/24078#discussion_r2004864581
PR Review Comment: https://git.openjdk.org/jdk/pull/24078#discussion_r2004863120