8282547: IGV: add control-flow graph view

[robcasloz]

This change adds a new view to the Ideal Graph Visualizer showing a classic control-flow graph (CFG) where nodes are serialized within basic blocks and arcs across basic blocks represent control flow:

The CFG provides a higher-level, more compact view of the graph than the existing views, where it can be hard to see the overall structure for all but the smallest graphs:

If a schedule is available in the input graph (because it corresponds to the Global code motion phase or later), the CFG, including the serialization of nodes within basic blocks, is derived from it. Otherwise, the CFG is approximated by the C2-specific ServerCompilerScheduler class. Dependencies among nodes are shown textually, by extending the label of each node with a list of its inputs:

The exact information shown about each input can be configured in Tools -> Options -> Tiny Node Text.

The new CFG view is available at the same level as the existing "sea of nodes" (default) and "clustered sea-of-nodes" views. The view can be selected from the main toolbar:

The default view shown when opening a new graph can be configured in Tools -> Options -> Default View.

The same functionality available for the existing views also applies to the new CFG:

• node extraction:

• pop-up information on edges:

• filtering:

• quick node and basic block search:

• and exporting.

The list of custom filters is extended with three new block-hiding filters:

• Hide root block: remove root block and hide all back-edges from exiting blocks that clutter the CFG.

• Hide uncommon trap blocks: remove blocks with uncommon traps to simplify following the "hot paths" in the CFG.

• Hide exception blocks: remove blocks that create and throw exceptions to also simplify following the "hot paths" in the CFG.

When using the CFG view, it is important to keep in mind that, in graphs corresponding to phases earlier than Global code motion, the presented CFG is just one of many possible projections of the sea-of-nodes representation, and that this projection is computed by an approximation of what C2 would do if it scheduled the graph at that stage. The approximation has some known limitations (JDK-8280568, JDK-8282053). Future work should address these and warn the user when the approximated CFG is known to be inaccurate.

Testing

Functionality

• Tested the above functionality manually on a small selection of graphs.

• Tested automatically that scheduling (approximating C2's GCM and LCM algorithms) and viewing thousands of graphs in the three views does not trigger any assertion failure (by instrumenting IGV to schedule and view graphs as they are loaded and running java -Xcomp -XX:-TieredCompilation -XX:PrintIdealGraphLevel=4).

Performance

• Measured the scheduling and view creation time for each of the three views on a selection of 22 large graphs (between 2515 and 7329 nodes). On average:

• The change introduces a slight overhead when creating the existing views (2% and 5% for the clustered and non-clustered sea-of-nodes views). This overhead is due to the introduction of abstractions and data structures required by the CFG view.

• The new CFG view is considerably faster to create than the existing ones (4.2x and 2.5x faster than the clustered and non-clustered sea-of-nodes views), as it can be expected, since only basic blocks have to be laid out. Furthermore, the CFG view scales better as the size of the graph increases.

• The change slows down scheduling by 27%. This overhead corresponds to the additional work of approximating C2's LCM, and it is essential for the CFG view. Fortunately, scheduling takes only a fraction of the time needed to create a view (typically no more than a third), and it only needs to run once for each graph, whereas views have to be re-created on node extraction, filtering, etc.

The performance results are attached (note that each measurement in the sheet corresponds to the median of ten runs).

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8282547: IGV: add control-flow graph view

Reviewers

• Christian Hagedorn (@chhagedorn - Reviewer)
• Xin Liu (@navyxliu - Committer)
• Tobias Hartmann (@TobiHartmann - Reviewer)

Contributors

• Christian Hagedorn <chagedorn@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7817/head:pull/7817
$ git checkout pull/7817

Update a local copy of the PR:
$ git checkout pull/7817
$ git pull https://git.openjdk.java.net/jdk pull/7817/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7817

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7817.diff

[bridgekeeper]

👋 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]

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

• build
• hotspot-compiler

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

[robcasloz]

/label remove build

[openjdk]

@robcasloz
The build label was successfully removed.

[mlbridge]

Webrevs

• 02: Full - Incremental (ae622849)
• 01: Full - Incremental (7b78cd7f)
• 00: Full (481afcf0)

[chhagedorn]

Great work Roberto! This looks very nice. I've tried it out and it worked well. The only thing I've noticed in the compact CFG view: When extracting a node, it shows all the empty blocks of the entire graph. I would have expected that empty blocks that are neither in- nor outputs of the extracted node are hidden. But I think that could also be potentially handled in a separate RFE and should not block this change.

My review comments are mostly just about code styling and the like since I'm not very familiar with the IGV code which made it hard to review - I trust your expertise in that area :-)

Thanks,
Christian

[robcasloz]

/contributor add @chhagedorn

[openjdk]

@robcasloz
Contributor Christian Hagedorn <chagedorn@openjdk.org> successfully added.

[robcasloz]

Thanks Christian for reviewing and providing so many suggestions! I have implemented all of them except a few for which I have provided separate answers.

The only thing I've noticed in the compact CFG view: When extracting a node, it shows all the empty blocks of the entire graph. I would have expected that empty blocks that are neither in- nor outputs of the extracted node are hidden. But I think that could also be potentially handled in a separate RFE and should not block this change.

This is actually deliberate, I thought it would be useful to preserve the control-flow context, so that the user can at all times know e.g. whether a block she is looking at is within a loop. Having said that, I agree that sometimes, particularly for large graphs, it would be useful to hide the empty blocks. This behavior would also be more consistent with the clustered sea-of-nodes view. Would it make sense to add a toolbar button to hide/show empty blocks in the control-flow graph view? If you think so, I will create a RFE to do that separately.

[chhagedorn]

Thanks Christian for reviewing and providing so many suggestions! I have implemented all of them except a few for which I have provided separate answers.

Thanks for coming back with the updates - they look good!

This is actually deliberate, I thought it would be useful to preserve the control-flow context, so that the user can at all times know e.g. whether a block she is looking at is within a loop.

I see that this can be valuable.

Having said that, I agree that sometimes, particularly for large graphs, it would be useful to hide the empty blocks. This behavior would also be more consistent with the clustered sea-of-nodes view.

Yes, that was the problem I was experiencing when I was testing your new change with such a large graph. Among hundreds of empty blocks, the actually selected nodes were too widely spread such that the view became hard to use.

Would it make sense to add a toolbar button to hide/show empty blocks in the control-flow graph view? If you think so, I will create a RFE to do that separately.

I think that would be great, thanks for considering it :-) And as you've mentioned above, for small graphs it helps to show the empty blocks as well. So, a toggle button would be a good choice.

Thanks,
Christian

[openjdk]

@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:

8282547: IGV: add control-flow graph view

Co-authored-by: Christian Hagedorn <chagedorn@openjdk.org>
Reviewed-by: chagedorn, xliu, thartmann

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 282 new commits pushed to the master branch:

• f074775: 8281284: Write JSlider accessibility test
• fe670ff: 8283692: Add PrintIdealPhase that includes block scheduling
• ab17f88: 8283626: AArch64: Set relocInfo::offset_unit to 4
• c3d903a: 8282936: Write a regression test for JDK-4615365
• cc598e0: 8283774: TestZoneOffset::test_immutable should ignore ZoneOffset::rules
• 0e788e0: 8283781: Avoid allocating unused lastRulesCaches
• 043b0a7: 8273355: Flickering on tooltip appearance IntelliJ IDEA 2021.2.1
• 2367228: 8283834: Unmappable character for US-ASCII encoding in TestPredicateInputBelowLoopPredicate
• 2e9fd56: 8283670: gtest os.release_multi_mappings_vm is still racy
• f01cce2: 8264160: Regex \b is not consistent with \w without UNICODE_CHARACTER_CLASS
• ... and 272 more: https://git.openjdk.java.net/jdk/compare/fc918a73d0dcc28146e60f15e978209424a32576...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.

[robcasloz]

Thanks Christian for reviewing and providing so many suggestions! I have implemented all of them except a few for which I have provided separate answers.

Thanks for coming back with the updates - they look good!

This is actually deliberate, I thought it would be useful to preserve the control-flow context, so that the user can at all times know e.g. whether a block she is looking at is within a loop.

I see that this can be valuable.

Having said that, I agree that sometimes, particularly for large graphs, it would be useful to hide the empty blocks. This behavior would also be more consistent with the clustered sea-of-nodes view.

Yes, that was the problem I was experiencing when I was testing your new change with such a large graph. Among hundreds of empty blocks, the actually selected nodes were too widely spread such that the view became hard to use.

Would it make sense to add a toolbar button to hide/show empty blocks in the control-flow graph view? If you think so, I will create a RFE to do that separately.

I think that would be great, thanks for considering it :-) And as you've mentioned above, for small graphs it helps to show the empty blocks as well. So, a toggle button would be a good choice.

Thanks, Christian

Thanks again for reviewing! I just addressed your latest comment, will file a RFE for hiding/showing empty blocks as soon as this PR gets a second approval.

[chhagedorn]

will file a RFE for hiding/showing empty blocks as soon as this PR gets a second approval.

Thanks!

[navyxliu]

This looks good to me. I am not a reviewer. I even don't think I understand IGV completely.
I just read the patch and also try it out. I think this compact view of sea-of-node is really useful.

[robcasloz]

This looks good to me. I am not a reviewer. I even don't think I understand IGV completely. I just read the patch and also try it out. I think this compact view of sea-of-node is really useful.

Thanks for having a look at the changes and trying out the new view, Xin, happy that you found it useful!

[TobiHartmann]

Works like a charm for me and changes look reasonable. Approved!

[robcasloz]

Works like a charm for me and changes look reasonable. Approved!

Thanks for reviewing, Tobias!

[robcasloz]

/integrate

[openjdk]

Going to push as commit edb42d7.
Since your change was applied there have been 299 commits pushed to the master branch:

• 7418373: 8283788: Remove unused VM_DeoptimizeAll::_dependee
• aa33525: 8283787: C1: Remove unused ArrayStoreExceptionStub::_info
• 8b65611: 8283789: CompilerPhaseTypeHelper::to_bitmask should operate on uint64_t
• 9bb916d: 8283800: Simplify String.indexOf/lastIndexOf calls
• b323f54: 8283846: Remove unused jdk.internal.reflect.SignatureIterator
• eb5b712: 8283701: Add final or sealed modifier to appropriate java.awt.color ICC_Profile API classes
• d066856: 8282162: [vector] Optimize integral vector negation API
• bfd9c2b: 8283015: Create a test for JDK-4715496
• 8cdabea: 8207025: JvmtiEnv::SetSystemProperty() does not handle OOM
• 272d653: 8283889: Fix Typo in open/src/java.sql/share/classes/java/sql/package-info.java
• ... and 289 more: https://git.openjdk.java.net/jdk/compare/fc918a73d0dcc28146e60f15e978209424a32576...master

Your commit was automatically rebased without conflicts.

[openjdk]

@robcasloz Pushed as commit edb42d7.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.