8345041: IGV: Free Placement Mode in IGV Layout #22438

tobiasholenstein · 2024-11-28T13:45:45Z

This PR depends on #22430. To check out this PR locally:

git fetch https://git.openjdk.org/jdk.git pull/22438/head:pull/22438
git checkout pull/22438

Introduce a Free Placement Mode to IGV, allowing users to position nodes freely without being limited to the hierarchical layout constraints.

In this mode, users can manually drag and place nodes anywhere within the space, giving them complete control over the visual arrangement of the graph. Connections between nodes will be rendered as straight (or S curved) lines, without recalculating or enforcing hierarchical constraints.

This feature is ideal for users who need a flexible, non-restrictive way to organize and visualize complex graph structures in a customized and intuitive manner. The free placement of nodes will remain persistent until the layout is reset or another layout mode is selected.

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

Warning

⚠️ Patch contains a binary file (src/utils/IdealGraphVisualizer/View/src/main/resources/com/sun/hotspot/igv/view/images/dynamic.png)

Issue

JDK-8345041: IGV: Free Placement Mode in IGV Layout (Enhancement - P4)

Reviewers

Emanuel Peter (@eme64 - Reviewer) Review applies to 4f7ca8ee
Roberto Castañeda Lozano (@robcasloz - Reviewer) Review applies to 14d20181
Christian Hagedorn (@chhagedorn - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22438

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

Using diff file

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

Using Webrev

Link to Webrev Comment

This reverts commit 2f4e3fd.

fix

bridgekeeper · 2024-11-28T13:46:34Z

👋 Welcome back tholenstein! 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 · 2024-11-28T13:47:37Z

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

8345041: IGV: Free Placement Mode in IGV Layout

Reviewed-by: chagedorn, epeter, rcastanedalo

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

5e6cda4: 8347000: Bug in com/sun/net/httpserver/bugs/B6361557.java test
3f7052e: 8346868: RISC-V: compiler/sharedstubs tests fail after JDK-8332689
f119663: 8346573: Can't use custom default file system provider with custom system class loader
379ac34: 8346838: RISC-V: runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java crash with debug VMs
0285020: 8345676: [ubsan] ProcessImpl_md.c:561:40: runtime error: applying zero offset to null pointer on macOS aarch64
0a81676: 8346881: [ubsan] logSelection.cpp:154:24 / logSelectionList.cpp:72:94 : runtime error: applying non-zero offset 1 to null pointer
08debd3: 8346993: C2 SuperWord: refactor to make more vector nodes available in VectorNode::make
de02503: 8346832: runtime/CompressedOops/CompressedCPUSpecificClassSpaceReservation.java fails on RISC-V
27646e5: 8344148: Add an explicit compiler phase for warning generation
8d388cc: 8346457: AOT cache creation crashes with "assert(pair_at(i).match() < pair_at(i+1).match()) failed: unsorted table entries"
... and 383 more: https://git.openjdk.org/jdk/compare/a80ccf2cd2792c24b51f1143cb0e6c5b036c5b28...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 · 2024-11-28T13:48:26Z

@tobiasholenstein 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 · 2024-11-28T14:18:54Z

Webrevs

eme64

This is really amazing, the feature. Really would make me start using IGV.

I scanned the code changes quickly, and it seems reasonable, at least to a non-IGV developer 😅

...ayout/src/main/java/com/sun/hotspot/igv/hierarchicallayout/FreeInteractiveLayoutManager.java

robcasloz

Hi Toby, I get the following assertion failure when I open the regular sea-of-nodes view of the attached graph free-placement-assertion-failure.zip and enable the "Simplify graph" and "Condense graph" filters:

robcasloz · 2024-11-28T18:08:58Z

Hi Toby, I get the following assertion failure when I open the regular sea-of-nodes view of the attached graph free-placement-assertion-failure.zip and enable the "Simplify graph" and "Condense graph" filters:

I just checked and turns out this is a regression in #22402, please address it in that pull request.

robcasloz

Test and performance results of this changeset (applied on top of 12332f2) look good.

tobiasholenstein · 2024-11-30T00:08:59Z

@chhagedorn reported the following bug: Click dynamic layout -> show all nodes -> Bug: showing only lines

broken.xml.zip

this should be fixed now. The cause was numerical instability of applyForceBasedAdjustment

tobiasholenstein · 2024-11-30T00:11:27Z

Thanks @eme64, @robcasloz and @chhagedorn for testing and reviewing!

/integrate delegate

openjdk · 2024-11-30T00:11:47Z

@tobiasholenstein Integration of this pull request has been delegated and may be completed by any project committer using the /integrate pull request command.

chhagedorn

Awesome enhancement! I've just tested it on Linux and it works very well.

robcasloz

I re-tested and re-ran layout drawing performance tests, all good.
The feature would be even more useful, in my opinion, if the hand-made layouts would persist across graphs in a group and even across sessions (e.g. serializing the node coordinates as properties, similarly to JDK-8345039), but this could be done separately.

robcasloz · 2024-12-02T09:12:20Z

/integrate

openjdk · 2024-12-02T09:13:22Z

@robcasloz Your integration request cannot be fulfilled at this time, as the status check jcheck-openjdk/jdk-22438 did not complete successfully

bridgekeeper · 2024-12-30T12:13:47Z

@tobiasholenstein This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

tobiasholenstein · 2025-01-07T11:59:37Z

/integrate

openjdk · 2025-01-07T11:59:50Z

@tobiasholenstein This pull request has not yet been marked as ready for integration.

tobiasholenstein · 2025-01-07T12:00:35Z

Can someone please re-review? thanks!

tobiasholenstein · 2025-01-07T14:29:12Z

/integrate

openjdk · 2025-01-07T14:30:06Z

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

8b22517: 8211851: (ch) java/nio/channels/AsynchronousSocketChannel/StressLoopback.java times out (aix)
5e6cda4: 8347000: Bug in com/sun/net/httpserver/bugs/B6361557.java test
3f7052e: 8346868: RISC-V: compiler/sharedstubs tests fail after JDK-8332689
f119663: 8346573: Can't use custom default file system provider with custom system class loader
379ac34: 8346838: RISC-V: runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java crash with debug VMs
0285020: 8345676: [ubsan] ProcessImpl_md.c:561:40: runtime error: applying zero offset to null pointer on macOS aarch64
0a81676: 8346881: [ubsan] logSelection.cpp:154:24 / logSelectionList.cpp:72:94 : runtime error: applying non-zero offset 1 to null pointer
08debd3: 8346993: C2 SuperWord: refactor to make more vector nodes available in VectorNode::make
de02503: 8346832: runtime/CompressedOops/CompressedCPUSpecificClassSpaceReservation.java fails on RISC-V
27646e5: 8344148: Add an explicit compiler phase for warning generation
... and 384 more: https://git.openjdk.org/jdk/compare/a80ccf2cd2792c24b51f1143cb0e6c5b036c5b28...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-01-07T14:30:17Z

@tobiasholenstein Pushed as commit e5f0c19.

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

Tobias Holenstein added 17 commits November 18, 2024 11:12

JDK-8314512 IGV: clean up hierarchical layout code

2c0ec74

AllSoFar

2f4e3fd

Revert "AllSoFar"

bf6b263

This reverts commit 2f4e3fd.

8314512: IGV: clean up hierarchical layout code

a55fc6a

Node labels in the CFG view left aligned

7bfdeef

Fix crash: missing Figure after filter applied

b826587

cached

f01593f

remove dead code in LineWidget

f045e1d

batch add connectionLayer.addChildren(newWidgets);

7617a53

run IGV without asserts

f11f43d

update Figure height calculation for Slots

ffa021d

remove executability of igv.sh

ad4d076

fixed graph objects equality

2f1157e

revert copyright changes

c08d99e

8343705: IGV: Interactive Node Moving in Hierarchical Layout

648057c

Merge branch 'pr/22402' into JDK-8343705

b8444a3

JDK-8345041 IGV: Free Placement Mode in IGV Layout

4f7ca8e

fix

tobiasholenstein changed the base branch from master to pr/22430 November 28, 2024 13:47

openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Nov 28, 2024

tobiasholenstein changed the title ~~8345041: Free Placement Mode in IGV Layout~~ 8345041: IGV: Free Placement Mode in IGV Layout Nov 28, 2024

tobiasholenstein marked this pull request as ready for review November 28, 2024 14:15

openjdk bot added the rfr Pull request is ready for review label Nov 28, 2024

eme64 approved these changes Nov 28, 2024

View reviewed changes

...ayout/src/main/java/com/sun/hotspot/igv/hierarchicallayout/FreeInteractiveLayoutManager.java Show resolved Hide resolved

robcasloz suggested changes Nov 28, 2024

View reviewed changes

robcasloz approved these changes Nov 29, 2024

View reviewed changes

make applyForceBasedAdjustment more numerical stable

14d2018

openjdk bot added the delegated May be integrated by another Committer label Nov 30, 2024

chhagedorn approved these changes Nov 30, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Nov 30, 2024

robcasloz approved these changes Dec 2, 2024

View reviewed changes

bridgekeeper bot added the oca Needs verification of OCA signatory status label Dec 2, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Dec 2, 2024

Update LayoutGraph.java

f8d699a

bridgekeeper bot removed the oca Needs verification of OCA signatory status label Jan 6, 2025

openjdk bot added the rfr Pull request is ready for review label Jan 6, 2025

chhagedorn approved these changes Jan 7, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 7, 2025

openjdk bot added the integrated Pull request has been integrated label Jan 7, 2025

openjdk bot closed this Jan 7, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review delegated May be integrated by another Committer labels Jan 7, 2025

8345041: IGV: Free Placement Mode in IGV Layout #22438

8345041: IGV: Free Placement Mode in IGV Layout #22438

Uh oh!

Conversation

tobiasholenstein commented Nov 28, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Warning

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Nov 28, 2024

Uh oh!

openjdk bot commented Nov 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Nov 28, 2024

Uh oh!

mlbridge bot commented Nov 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

eme64 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robcasloz left a comment

Choose a reason for hiding this comment

Uh oh!

robcasloz commented Nov 28, 2024

Uh oh!

robcasloz left a comment

Choose a reason for hiding this comment

Uh oh!

tobiasholenstein commented Nov 30, 2024

Uh oh!

tobiasholenstein commented Nov 30, 2024

Uh oh!

openjdk bot commented Nov 30, 2024

Uh oh!

chhagedorn left a comment

Choose a reason for hiding this comment

Uh oh!

robcasloz left a comment

Choose a reason for hiding this comment

Uh oh!

robcasloz commented Dec 2, 2024

Uh oh!

openjdk bot commented Dec 2, 2024

Uh oh!

bridgekeeper bot commented Dec 30, 2024

Uh oh!

tobiasholenstein commented Jan 7, 2025

Uh oh!

openjdk bot commented Jan 7, 2025

Uh oh!

tobiasholenstein commented Jan 7, 2025

Uh oh!

tobiasholenstein commented Jan 7, 2025

Uh oh!

openjdk bot commented Jan 7, 2025

Uh oh!

openjdk bot commented Jan 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

tobiasholenstein commented Nov 28, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Nov 28, 2024 •

edited

Loading

mlbridge bot commented Nov 28, 2024 •

edited

Loading