RFR: JDK-8290011: IGV: Remove dead code and cleanup

[Christian Hagedorn]

On Wed, 7 Sep 2022 11:45:45 GMT, Tobias Holenstein <tholenstein at openjdk.org> wrote:

> Remove dead code from the IGV code base. There are many unused or redundant functions in the code

Nice cleanup! I only have some general comments as I'm not very familiar with the code details. @robcasloz should also have a look.

What kind of testing did you do?

src/utils/IdealGraphVisualizer/Data/src/main/java/com/sun/hotspot/igv/data/Group.java line 83:

> 81: 
> 82:     public List<InputGraph> getGraphs() {
> 83:         return  Collections.unmodifiableList(graphs);

Suggestion:

        return Collections.unmodifiableList(graphs);

src/utils/IdealGraphVisualizer/Data/src/main/java/com/sun/hotspot/igv/data/Group.java line 102:

> 100: 
> 101:     @Override
> 102:     public String toString() {

You might want to keep this method for debugging purposes?

src/utils/IdealGraphVisualizer/Data/src/main/java/com/sun/hotspot/igv/data/InputNode.java line 34:

> 32: public class InputNode extends Properties.Entity {
> 33: 
> 34:     private int id;

While cleaning this class up anyways: Feels like a node id should probably not change anymore once it's set. Can this be turned into a `final` field? Looks like `setId()` is only called from this class and once from another class when creating a new input node anyways.

src/utils/IdealGraphVisualizer/Data/src/main/java/com/sun/hotspot/igv/data/InputNode.java line 36:

> 34:     private int id;
> 35: 
> 36:     public static final Comparator<InputNode> COMPARATOR = new Comparator<InputNode>() {

Is unused as well and can be removed. Same for `getPropertyComparator()`.

src/utils/IdealGraphVisualizer/Graph/src/main/java/com/sun/hotspot/igv/graph/Diagram.java line 44:

> 42:     private static final Font font = new Font("Arial", Font.PLAIN, 12);
> 43:     private static final Font slotFont = new Font("Arial", Font.PLAIN, 10);
> 44:     private static final Font boldFont = font.deriveFont(Font.BOLD);

Maybe make them `public` and access them directly instead of going over `static` getters. Also, I suggest to use upper case letters for static final fields

src/utils/IdealGraphVisualizer/Graph/src/main/java/com/sun/hotspot/igv/graph/Figure.java line 235:

> 233: 
> 234:     public InputNode getInputNode() {
> 235:         return this.inputNode;

`this` can be omitted.
Suggestion:

        return inputNode;

src/utils/IdealGraphVisualizer/SelectionCoordinator/src/main/java/com/sun/hotspot/igv/selectioncoordinator/SelectionCoordinator.java line 52:

> 50:         highlightedChangedEvent = new ChangedEvent<SelectionCoordinator>(this);
> 51:         selectedObjects = new HashSet<Object>();
> 52:         highlightedObjects = new HashSet<Object>();

The explicit generic type argument can be omitted.

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

> 90:         boolean viewPropertiesChanged = false;
> 91: 
> 92:         boolean groupChanged = (group != newModel.group);

Was that a bug?

src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/EditorTopComponent.java line 322:

> 320: 
> 321:     public DiagramViewModel getModel() {
> 322:         return  scene.getModel();

Suggestion:

        return scene.getModel();

src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/ShowAllAction.java line 43:

> 41:         EditorTopComponent editor = EditorTopComponent.getActive();
> 42:         if (editor != null) {
> 43:             editor.getModel().setHiddenNodes(new HashSet<Integer>());

Suggestion:

            editor.getModel().setHiddenNodes(new HashSet<>());

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

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