On Fri, 27 Jun 2025 09:10:26 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. I found a couple of places to realign the parameters, but otherwise this looks good. I like the new naming conventions. We have a lot of handshakes now! Were you able to build shenandoah (not built by default, need to add --enable-jvm-feature-shenandoahgc to configure)? src/hotspot/share/prims/jvmtiEnvBase.hpp line 629: > 627: public: > 628: GetCurrentContendedMonitorHandshakeClosure(JvmtiEnv *env, > 629: JavaThread* calling_thread, Can you realign these parameters? src/hotspot/share/prims/jvmtiEnvBase.hpp line 650: > 648: public: > 649: GetStackTraceHandshakeClosure(JvmtiEnv *env, jint start_depth, jint > max_count, > 650: jvmtiFrameInfo* frame_buffer, jint* count_ptr) realign parameters. src/hotspot/share/prims/jvmtiEnvBase.hpp line 760: > 758: public: > 759: GetSingleStackTraceHandshakeClosure(JvmtiEnv *env, JavaThread > *calling_thread, > 760: jthread thread, jint max_frame_count) Also realign parameters. src/hotspot/share/prims/jvmtiEnvBase.hpp line 798: > 796: public: > 797: GetFrameLocationHandshakeClosure(JvmtiEnv *env, jint depth, > 798: jmethodID* method_ptr, jlocation* location_ptr) Also realign parameters. ------------- Marked as reviewed by coleenp (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/26014#pullrequestreview-2967227483 PR Review Comment: https://git.openjdk.org/jdk/pull/26014#discussion_r2172435715 PR Review Comment: https://git.openjdk.org/jdk/pull/26014#discussion_r2172435267 PR Review Comment: https://git.openjdk.org/jdk/pull/26014#discussion_r2172434071 PR Review Comment: https://git.openjdk.org/jdk/pull/26014#discussion_r2172433356