RFR: 8284016: Normalize handshake closure names [v3]

Serguei Spitsyn sspitsyn at openjdk.org
Tue Jul 1 09:02:41 UTC 2025


On Mon, 30 Jun 2025 11:57:23 GMT, Anton Artemov <duke at openjdk.org> wrote:

>> Hi, please consider the following changes:
>> 
>> There are many classes inherited from the `HandshakeClosure` class, but they do not follow the same naming convention. In this PR we address this issue, all names are normalized in the following way:
>> 
>> `XXXDummyClassNameClosure -> XXXDummyClassNameHandshakeClosure`
>> 
>> or
>> 
>> `XXXDummyClassNameHandshake -> XXXDummyClassNameHandshakeClosure`
>> 
>> or
>> 
>> `XXXStrangeClassName -> SomewhatSimilarNameHandshakeClosure`
>> 
>> Tested in GHA and tiers 1 - 3.
>
> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8284016: Reverted closure names in JVMTI

This looks okay in general.
I've posted several nits though.

src/hotspot/share/classfile/javaClasses.cpp line 1980:

> 1978:   ResourceMark rm(THREAD);
> 1979:   HandleMark   hm(THREAD);
> 1980:   GetStackTraceHandshakeClosure gstc(Handle(THREAD, java_thread));

Nit: A suggestion to rename: `gstc` => `gsthc` or `gstc` => `hc` or `gstc` => `cl`.

src/hotspot/share/prims/whitebox.cpp line 2288:

> 2286:   };
> 2287: 
> 2288:   ReadMonitorsHandshakeClosure rmc;

Nit: A suggestion to rename: `rmc` => `rmhc` or `rmc` => `cl`.

src/hotspot/share/prims/whitebox.cpp line 2319:

> 2317:     jint num_threads_completed() const { return _num_threads_completed; }
> 2318:   };
> 2319:   TraceSelfHandshakeClosure tsc(Thread::current());

Nit: A suggestion to rename: `tsc` => `tshc` or `tsc` => `cl`.

src/hotspot/share/prims/whitebox.cpp line 2357:

> 2355:     bool is_alive = tlh.cv_internal_thread_to_JavaThread(thread_handle, &target, nullptr);
> 2356:     if (is_alive) {
> 2357:       TraceSelfHandshakeClosure* tsc = new TraceSelfHandshakeClosure(target);

Nit: A suggestion to rename: `tsc` => `tshc` or `tsc` => `cl`.

src/hotspot/share/runtime/synchronizer.cpp line 1837:

> 1835:     // A JavaThread needs to handshake in order to safely free the
> 1836:     // ObjectMonitors that were deflated in this cycle.
> 1837:     DeflationHandshakeClosure hfd_hc;

Nit: Rename: `hfd_hc` => `dhc` or `hfd_hc` => `cl`.

src/hotspot/share/runtime/vmThread.cpp line 456:

> 454:       if (HandshakeALot) {
> 455:         MutexUnlocker mul(VMOperation_lock);
> 456:         ALotOfHandshakeClosure hal_cl;

Nit: Rename: `hal_cl` => `aohc` or `hfd_hc` => `cl`.

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

PR Review: https://git.openjdk.org/jdk/pull/26014#pullrequestreview-2974276487
PR Review Comment: https://git.openjdk.org/jdk/pull/26014#discussion_r2176875478
PR Review Comment: https://git.openjdk.org/jdk/pull/26014#discussion_r2176885943
PR Review Comment: https://git.openjdk.org/jdk/pull/26014#discussion_r2176889645
PR Review Comment: https://git.openjdk.org/jdk/pull/26014#discussion_r2176891619
PR Review Comment: https://git.openjdk.org/jdk/pull/26014#discussion_r2176908630
PR Review Comment: https://git.openjdk.org/jdk/pull/26014#discussion_r2176911514


More information about the serviceability-dev mailing list