During the course of Emmy Yin's master's thesis (https://urn.kb.se/resolve?urn=urn:nbn:se:kth:diva-329352), she identified some opportunities for cleaning up HierarchicalLayoutManager.java:
- The code block in https://github.com/openjdk/jdk/blob/c634bdf9d917c96c38efe826239eab7900c33e74/src/utils/IdealGraphVisualizer/HierarchicalLayout/src/main/java/com/sun/hotspot/igv/hierarchicallayout/HierarchicalLayoutManager.java#L395-L476 (for (LayoutEdge e : n.succs) loop within WriteResult::run()) might not be necessary since all edges can be drawn from bottom-up.
- Outgoing edges from the same port are combined, but there is no support for combining edges going into the same port, just an exception being thrown: https://github.com/openjdk/jdk/blob/c634bdf9d917c96c38efe826239eab7900c33e74/src/utils/IdealGraphVisualizer/HierarchicalLayout/src/main/java/com/sun/hotspot/igv/hierarchicallayout/HierarchicalLayoutManager.java#L1219-L1220.
- calculateOptimalBoth is not being used at all: https://github.com/openjdk/jdk/blob/c634bdf9d917c96c38efe826239eab7900c33e74/src/utils/IdealGraphVisualizer/HierarchicalLayout/src/main/java/com/sun/hotspot/igv/hierarchicallayout/HierarchicalLayoutManager.java#L678-L697.
- I never understood what the vip field in LayoutEdge is for. I never encountered it being used, so unless you know what it does it could be looked into if it should be cleaned up as well. There are many instances where that field causes special treatment in the code.
- When the data structures are being set up, there are no checks for duplicates. Could be worth adding this here: https://github.com/openjdk/jdk/blob/c634bdf9d917c96c38efe826239eab7900c33e74/src/utils/IdealGraphVisualizer/HierarchicalLayout/src/main/java/com/sun/hotspot/igv/hierarchicallayout/HierarchicalLayoutManager.java#L1715-L1717.
- The width of dummy nodes is currently set to 1. When connecting edges to/from a dummy node, the edge is given relativeFrom or relativeTo 0, but this is currently written as dummy.width / 2 . Could either be cleaned up and just set directly to 0, or looked into whether this structure is useful to keep, e.g. if the dummy nodes might have width greater than 1.
- The code block in https://github.com/openjdk/jdk/blob/c634bdf9d917c96c38efe826239eab7900c33e74/src/utils/IdealGraphVisualizer/HierarchicalLayout/src/main/java/com/sun/hotspot/igv/hierarchicallayout/HierarchicalLayoutManager.java#L395-L476 (for (LayoutEdge e : n.succs) loop within WriteResult::run()) might not be necessary since all edges can be drawn from bottom-up.
- Outgoing edges from the same port are combined, but there is no support for combining edges going into the same port, just an exception being thrown: https://github.com/openjdk/jdk/blob/c634bdf9d917c96c38efe826239eab7900c33e74/src/utils/IdealGraphVisualizer/HierarchicalLayout/src/main/java/com/sun/hotspot/igv/hierarchicallayout/HierarchicalLayoutManager.java#L1219-L1220.
- calculateOptimalBoth is not being used at all: https://github.com/openjdk/jdk/blob/c634bdf9d917c96c38efe826239eab7900c33e74/src/utils/IdealGraphVisualizer/HierarchicalLayout/src/main/java/com/sun/hotspot/igv/hierarchicallayout/HierarchicalLayoutManager.java#L678-L697.
- I never understood what the vip field in LayoutEdge is for. I never encountered it being used, so unless you know what it does it could be looked into if it should be cleaned up as well. There are many instances where that field causes special treatment in the code.
- When the data structures are being set up, there are no checks for duplicates. Could be worth adding this here: https://github.com/openjdk/jdk/blob/c634bdf9d917c96c38efe826239eab7900c33e74/src/utils/IdealGraphVisualizer/HierarchicalLayout/src/main/java/com/sun/hotspot/igv/hierarchicallayout/HierarchicalLayoutManager.java#L1715-L1717.
- The width of dummy nodes is currently set to 1. When connecting edges to/from a dummy node, the edge is given relativeFrom or relativeTo 0, but this is currently written as dummy.width / 2 . Could either be cleaned up and just set directly to 0, or looked into whether this structure is useful to keep, e.g. if the dummy nodes might have width greater than 1.
- relates to
-
JDK-8309463 IGV: Dynamic graph layout algorithm
-
- Resolved
-
- links to
-
Commit(master)
openjdk/jdk/4da7c354
-
Review(master)
openjdk/jdk/22402