RFR: 8291801: IGV: Broken button "Hide graphs which are the same as the previous graph" [v3]

Koichi Sakata ksakata at openjdk.org
Wed Jan 18 02:42:26 UTC 2023


> This pull request makes the "Hide graphs..." button working.
> 
> The parser for bgv files, `BinaryParaser` class, enables the "hide graphs" function. But the xml parser, `Parser` class, doesn't have any code about the function. So I wrote code by referring to BinaryParser class.
> 
> # Tests
> 
> I tested manually. Screenshots are as follows.
> 
> <img width="1175" alt="スクリーンショット 2022-09-27 16 52 16" src="https://user-images.githubusercontent.com/60008/192467385-328b8298-9055-484d-813d-1c50200e2239.png">
> 
> In this case, there are 13 graphs and 10 graphs from "After Parsing" to "Before matching" are the same. So only 4 graphs are shown after we push the button. 
> 
> <img width="1175" alt="スクリーンショット 2022-09-27 16 52 23" src="https://user-images.githubusercontent.com/60008/192467331-fbeb4911-460d-476c-9445-098805c39d27.png">
> 
> Pushing it again or opening the graph that is hidden in the view restores the view to its original state. I attached [the graph file that I used in the test](https://github.com/openjdk/jdk/files/9653179/hello.zip).
> 
> Furthermore, I added a test method for the class I changed. Running `mvn test` command was successful.
> 
> # Concerns
> 
> I'm concerned about the equivalence of 2 graphs. I regard them as the same graphs when all fields in `InputGraph` class are equal. But in the test class, 2 graphs are equal when nodes, edges, blocks and blocks of each `InputNode` are equal. Here is an extract of the test code.
> 
> 
> // src/utils/IdealGraphVisualizer/Data/src/test/java/com/sun/hotspot/igv/data/Util.java
> 
>     public static void assertGraphEquals(InputGraph a, InputGraph b) {
> 
>         if(!a.getNodesAsSet().equals(b.getNodesAsSet())) {
>             fail();
>         }
> 
>         if (!a.getEdges().equals(b.getEdges())) {
>             fail();
>         }
> 
>         if (a.getBlocks().equals(b.getBlocks())) {
>             fail();
>         }
> 
>         for (InputNode n : a.getNodes()) {
>             assertEquals(a.getBlock(n), b.getBlock(n));
>         }
>     }
> 
> 
> But opening a graph is very slow when I compare blocks of each InputNode. So I didn't add that comparison in `isSameContent` method.

Koichi Sakata has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits:

 - Remove unnecessary call
 - Change a timing of check for duplicate graphs
 - Change a timing of check for duplicate graphs
 - Merge branch 'master' into JDK-8291803
 - Set value to hideDuplicates field
 - Merge remote-tracking branch 'upstream/master' into JDK-8291801
 - Remove extra whitespaces
 - Improve the test
 - Add the test method
 - Remove extra code
 - ... and 1 more: https://git.openjdk.org/jdk/compare/f95346e8...deb77791

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

Changes: https://git.openjdk.org/jdk/pull/10440/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=10440&range=02
  Stats: 59 lines in 3 files changed: 59 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/10440.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10440/head:pull/10440

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


More information about the hotspot-compiler-dev mailing list