On Mon, 30 Jun 2025 11:57:23 GMT, Anton Artemov <d...@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