On Tue, 4 Feb 2025 09:05:35 GMT, Thomas Schatzl <[email protected]> wrote:
>> Albert Mingkun Yang has updated the pull request with a new target base due
>> to a merge or a rebase. The incremental webrev excludes the unrelated
>> changes brought in by the merge/rebase. The pull request contains four
>> additional commits since the last revision:
>>
>> - Merge branch 'master' into gclocker
>> - review
>> - Merge branch 'master' into gclocker
>> - gclocker
>
> src/hotspot/share/gc/shared/gcLocker.hpp line 33:
>
>> 31:
>> 32: class GCLocker: public AllStatic {
>> 33: static Monitor* _lock;
>
> Not sure if having this copy/reference to `Heap_lock` makes the code more
> clear than referencing `Heap_lock` directly. It needs to be `Heap_lock`
> anyway.
`GCLocker` itself doesn't mandates that the lock must be `Heap_lock`; it's the
interaction with rest of VM that shows that `Heap_lock` is a good candidate.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23367#discussion_r1943040719
