On Sat, 24 May 2025 00:17:26 GMT, Alex Menkov <amen...@openjdk.org> wrote:
> This is first (hotspot) part of the update for > `HotSpotDiagnosticMXBean.dumpThreads` and `jcmd Thread.dump_to_file` to > include lock information in thread dumps (JDK-8356870). > The update has been split into parts to simplify reviewing. > The fix contains an implementation of `jdk.internal.vm.ThreadSnapshot` class > to gather required information about a thread. > Second (dependent) part includes changes in > `HotSpotDiagnosticMXBean.dumpThreads`/`jcmd Thread.dump_to_file`, spec > updates and tests for the functionality. > > Testing: new `HotSpotDiagnosticMXBean.dumpThreads`/`jcmd Thread.dump_to_file` > functionality was tested in loom repo; > sanity tier1 (this fix only) Just a few drive-by questions: 1) Did you consider putting this implementation in another file, say threadServices.hpp or even its own file? I'm asking because javaClasses seems to have become a dumping ground for code that calls from Java code into the JVM. It would be nice if we could put features into files that are named after the feature. 2) Did you consider to put these OopHandle storages in something else than Universe::vm_globals()? We have, for example, ThreadService::_thread_service_storage. src/hotspot/share/classfile/javaClasses.cpp line 69: > 67: #include "oops/typeArrayOop.inline.hpp" > 68: #include "prims/jvmtiExport.hpp" > 69: #include "prims/jvmtiThreadState.hpp" // for JvmtiVTMSTransitionDisabler Drive-by comment: these comments tend to become stale so I recommend that they are dropped from this PR Suggestion: #include "prims/jvmtiThreadState.hpp" ------------- PR Review: https://git.openjdk.org/jdk/pull/25425#pullrequestreview-2870373652 PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2108712946
