RFR: JDK-8294564: IGV: IllegalArgumentException for "Difference to current graph" [v2]

Christian Hagedorn chagedorn at openjdk.org
Mon Oct 3 14:23:18 UTC 2022


On Mon, 3 Oct 2022 14:16:20 GMT, Tobias Holenstein <tholenstein at openjdk.org> wrote:

>> "Difference to current graph" opens a new window with a difference graph in IGV. Unfortunately, it was throwing a  `java.lang.IllegalArgumentException`
>> 
>> # Overview 
>> In IGV, for every opened graph, that is not a difference graph, the user can right-click on any other graph in the Outline and select "Difference to current graph". This opens a difference graph showing the difference between the currently opened graph and the selected graph. 
>> <img width="478" alt="difference to current" src="https://user-images.githubusercontent.com/71546117/193585135-8e93cb2f-6c05-40c4-9b19-7253ce8d6bd8.png">
>> 
>> If the current graph is already a difference graph, the function is disabled. Same if the selected graph is identical to the opened one. 
>> <img width="508" alt="difference disabled" src="https://user-images.githubusercontent.com/71546117/193585932-4444bb46-7354-4138-81b6-1b11cfad4a8b.png">
>> 
>> # Implementation
>> The problem was that the difference graph did not keep track of which two `InputGraphs` it was based on. Therefore the functions `getFirstGraph()` and `getSecondGraph` in `DiagramViewModel` did not work properly. Now, `InputGraph` keeps track of the first and second `InputGraph` if it is a difference graph (`isDiffGraph`). And `getFirstGraph()` and `getSecondGraph` are updated to return the right `InputGraph`s for difference graphs. 
>> 
>> ---------
>> ### Progress
>> - [ ] Change must be properly reviewed (1 review required, with at least 1 [Reviewer](https://openjdk.org/bylaws#reviewer))
>> - [x] Change must not contain extraneous whitespace
>> - [x] Commit message must refer to an issue
>> 
>> 
>> 
>> ### Reviewing
>> <details><summary>Using <code>git</code></summary>
>> 
>> Checkout this PR locally: \
>> `$ git fetch https://git.openjdk.org/jdk pull/10533/head:pull/10533` \
>> `$ git checkout pull/10533`
>> 
>> Update a local copy of the PR: \
>> `$ git checkout pull/10533` \
>> `$ git pull https://git.openjdk.org/jdk pull/10533/head`
>> 
>> </details>
>> <details><summary>Using Skara CLI tools</summary>
>> 
>> Checkout this PR locally: \
>> `$ git pr checkout 10533`
>> 
>> View PR using the GUI difftool: \
>> `$ git pr show -t 10533`
>> 
>> </details>
>> <details><summary>Using diff file</summary>
>> 
>> Download this PR as a diff file: \
>> <a href="https://git.openjdk.org/jdk/pull/10533.diff">https://git.openjdk.org/jdk/pull/10533.diff</a>
>> 
>> </details>
>
> Tobias Holenstein has updated the pull request incrementally with one additional commit since the last revision:
> 
>   rename view_difference to viewDifference

Looks good!

src/utils/IdealGraphVisualizer/Data/src/main/java/com/sun/hotspot/igv/data/InputGraph.java line 42:

> 40:     private Map<Integer, InputBlock> nodeToBlock;
> 41:     private boolean isDiffGraph;
> 42:     private InputGraph firstGraph, secondGraph;

Can be made `final`. I would also split this line into two lines.

src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/DiagramViewModel.java line 418:

> 416:         }
> 417:         return firstGraph;
> 418: 

Empty line can be removed.

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

Marked as reviewed by chagedorn (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10533


More information about the hotspot-compiler-dev mailing list