On Fri, 1 May 2026 15:37:12 GMT, Kevin Walls <[email protected]> wrote:

> This implements "jcmd on core files" for Linux, and for MiniDumps on Windows 
> (MacOS is "future work").
> jcmd "revives" the VM memory and .so/.dll from the core/minidump, and runs 
> the existing native diagnostic command parser and command implementations.
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

Some general comments. I can see that we do these Thread::is_revived checks in 
some places, and then do something special. How are we supposed to understand 
when we're meant to insert these into the code or why they are inserted at 
those particular places :-)?

src/hotspot/share/classfile/dictionary.cpp line 242:

> 240: 
> 241: void Dictionary::print_table_statistics(outputStream* st, const char* 
> table_name) {
> 242:   NOT_WINDOWS(static) TableStatistics ts; // Avoid atexit for static 
> destructor on Windows

I think that we can just delete this static, I opened a separate PR for it: 
https://github.com/openjdk/jdk/pull/31039

src/hotspot/share/runtime/thread.cpp line 580:

> 578: }
> 579: 
> 580: jboolean Thread::_revived_vm = false;

This can't be a `bool`?

src/hotspot/share/runtime/thread.cpp line 593:

> 591:   const char *runtime_version;
> 592:   const char *runtime_vendor_version;
> 593:   const char *jdk_debug_level;

Style: Please make sure that pointer asterisks hug the type, not the variable 
(`char* runtime_name`).

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

PR Review: https://git.openjdk.org/jdk/pull/31011#pullrequestreview-4229117715
PR Review Comment: https://git.openjdk.org/jdk/pull/31011#discussion_r3189368116
PR Review Comment: https://git.openjdk.org/jdk/pull/31011#discussion_r3189383829
PR Review Comment: https://git.openjdk.org/jdk/pull/31011#discussion_r3189387758

Reply via email to