On Tue, 26 Mar 2024 21:55:38 GMT, Kevin Walls <kev...@openjdk.org> wrote:
>> Introduce the jcmd "VM.inspect" to implement access to detailed JVM object
>> information.
>>
>> Not recommended for live production use. Requires UnlockDiagnosticVMOptions
>> and not included in jcmd help output, to remind us this is not a
>> general-purpose customer-facing tool.
>
> Kevin Walls has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Undo include
It looks like the test only inspects a thread and a java object. Perhaps you
could add tests for additional VM objects. Maybe grab a frame PC from a thread
stack trace.
src/hotspot/share/services/diagnosticCommand.hpp line 590:
> 588: }
> 589: static const char* description() {
> 590: return "Inspect VM object at address.";
Maybe distinguishing between java heap objects and VM objects would be useful
to the reader/user.
src/hotspot/share/utilities/debug.cpp line 679:
> 677: }
> 678:
> 679: // Additional "good oop" checks, separate method to not disturb existing
> asserts.
Suggestion:
// Additional "good oop" checks. Separate method to not disturb existing
asserts.
src/hotspot/share/utilities/debug.cpp line 680:
> 678:
> 679: // Additional "good oop" checks, separate method to not disturb existing
> asserts.
> 680: extern "C" bool dbg_is_good_oop_detailed(oopDesc* o) {
What was the motivation for adding this? Is this copied from other HS code? How
do we know it's doing a good enough job of verification? What happens if
verification is not thorough enough?
test/hotspot/jtreg/serviceability/dcmd/vm/VMInspectTest.java line 83:
> 81: }
> 82:
> 83: // Tests where enabled:
Suggestion:
// Tests run with VM.inspect support enabled:
-------------
PR Review: https://git.openjdk.org/jdk/pull/17655#pullrequestreview-1962319603
PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1540483489
PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1540485389
PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1540486518
PR Review Comment: https://git.openjdk.org/jdk/pull/17655#discussion_r1540492385
