8354520: IGV: dump contextual information #24724

robcasloz · 2025-04-17T13:05:03Z

This changeset extends the IGV graph dumps with additional properties that ease tracing the dumps back to the context in which they were produced. The changeset dumps, for every compilation, the following additional properties:

JVM arguments
platform information
JVM version information
date and time
process ID
(compiler) thread ID

Additionally, the changeset produces and dumps the C2 stack trace from which each graph is dumped:

This should be particularly useful in an interactive context, where the user steps through C2 code using a debugger and dumps graphs at different points. To produce a stack trace in this context, the usual debugger-entry C2 functions (igv_print, igv_append, Node::dump_bfs, ...) are extended with extra arguments to specify the stack handling registers (stack pointer, frame pointer, and program counter):

The inconvenience of manually specifying the stack handling registers can be addressed by hiding them in debugger user-defined commands, e.g.:

define igv
  p igv_print(true, $sp, $fp, $pc)
end

define igv_node
  p find_node($arg0)->dump_bfs(0, 0, "!", $sp, $fp, $pc)
end

Thanks to @TobiHartmann for providing useful feedback!

Testing

tier1 (windows-x64, linux-x64, linux-aarch64, macosx-x64, and macosx-aarch64; release and debug mode).
Tested interactive usage manually via gdb and rr on linux-x64.
Tested automatically that dumping thousands of graphs does not trigger any assertion failure.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8354520: IGV: dump contextual information (Enhancement - P4)

Reviewers

Damon Fenacci (@dafedafe - Committer) Review applies to dd1ad6ad
Emanuel Peter (@eme64 - Reviewer)

Reviewers without OpenJDK IDs

@mhaessig (no known openjdk.org user name / role) Review applies to 4d1e6f86

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24724/head:pull/24724
$ git checkout pull/24724

Update a local copy of the PR:
$ git checkout pull/24724
$ git pull https://git.openjdk.org/jdk.git pull/24724/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24724

View PR using the GUI difftool:
$ git pr show -t 24724

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24724.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-04-17T13:05:34Z

👋 Welcome back rcastanedalo! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-04-17T13:06:57Z

@robcasloz This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8354520: IGV: dump contextual information

Reviewed-by: epeter, dfenacci

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 288 new commits pushed to the master branch:

1501a5e: 8356122: Client build fails after JDK-8350209
a8cd01f: 8355353: File Leak in os::read_image_release_file of os.cpp:1552
0264797: 8352730: RISC-V: Disable tests in qemu-user
... and 285 more: https://git.openjdk.org/jdk/compare/1138a186eb670e2c0662bda69c35680b41f4d66c...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-04-17T13:07:47Z

@robcasloz The following label will be automatically applied to this pull request:

hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-04-17T14:14:30Z

Webrevs

mhaessig

Thank you for working on this and continually improving IGV!

I found two typos, but otherwise it looks good. I also tested printing to IGV from rr and the new features work as advertised.

Perhaps you could update the dump_bfs help with a comment about the new arguments?

mhaessig · 2025-04-23T10:51:27Z

src/utils/IdealGraphVisualizer/README.md

+
+The JVM provides some entry functions to dump graphs from a debugger such as
+`gdb` or `rr`, see the different variants of `igv_print` and `igv_append` in
+`compile.cpp`. In combination with the IGV network interface, these functions


Suggested change

`compile.cpp`. In combination with the IGV network interface, these functions

[`compile.cpp`](src/hotspot/share/opto/compile.cpp). In combination with the

IGV network interface, these functions

Thanks for the suggestion, but what is the value of adding such a link here?

I often find them helpful to find the files a readme is referring to, so I tend to add them. But it's up to you whether you like them as well 🙂

Fair enough, so the relative link will resolve correctly and point to the actual compile.cpp file on GitHub?

I added the link now (commit 8193767d). Turns out GitHub would treat it, by default, as relative to the directory where the README.md file is placed, so I had to prefix it with / to make it relative to the root of the repo.

src/utils/IdealGraphVisualizer/README.md

Co-authored-by: Manuel Hässig <manuel.hassig@oracle.com>

…entation

robcasloz · 2025-04-24T10:05:14Z

I found two typos, but otherwise it looks good. I also tested printing to IGV from rr and the new features work as advertised.

Thanks for trying out and reviewing, Manuel!

Perhaps you could update the dump_bfs help with a comment about the new arguments?

Done (commit d65ecdb), let me know if that works for you.

I also made a minor correction to the IGV documentation (commit 86fdc67).

robcasloz · 2025-04-30T12:11:32Z

Commit dd1ad6a documents how to dump the C2 stack trace when using lldb (default debugger on macOS platforms). Thanks @dafedafe for reporting and helping out!

dafedafe

Thanks a lot for adding this feature @robcasloz.

dafedafe · 2025-04-30T12:38:17Z

src/hotspot/share/opto/node.cpp

      Compile* C = Compile::current();
      C->init_igv();
-      C->igv_print_graph_to_network("PrintBFS", _print_list);
+      C->igv_print_graph_to_network(nullptr, _print_list, _frame);


I suppose you removed "PrintBFS" to make the graph name be "Debug" like the other ones and make it easier to handle name and stack printing right?

That's right.

robcasloz · 2025-04-30T14:30:31Z

Thanks a lot for adding this feature @robcasloz.

Thanks for reviewing, Damon!

eme64

Nice work @robcasloz !

I just left a few suggestions below, but I think they are basically all nits / optional :)

eme64 · 2025-04-30T14:14:53Z