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