RFR: JDK-8293364: IGV: Refactor Action in EditorTopComponent and fix minor bugs [v8]

Tobias Holenstein tholenstein at openjdk.org
Mon Sep 19 14:33:03 UTC 2022


On Mon, 19 Sep 2022 08:28:18 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Tobias Holenstein has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   undo removing variable generated by form editor
>
> src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/DiagramViewModel.java line 188:
> 
>> 186:     public void setHideDuplicates(boolean b) {
>> 187:         InputGraph currentGraph = getFirstGraph();
>> 188:         if (b) {
> 
> I suggest to rename `b` to `hideDuplicates`.

done

> src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/EditorTopComponent.java line 171:
> 
>> 169:             }
>> 170: 
>> 171:         };
> 
> Could be changed into using a lambda:
> Suggestion:
> 
>         ChangedListener<DiagramViewModel> diagramChangedListener = source -> {
>             updateDisplayName();
>             Collection<Object> list = new ArrayList<>();
>             list.add(new EditorInputGraphProvider(EditorTopComponent.this));
>             graphContent.set(list, null);
>             diagramProvider.getChangedEvent().fire();
>         };

done

> src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/EditorTopComponent.java line 306:
> 
>> 304: 
>> 305:     public static EditorTopComponent getActive() {
>> 306:         TopComponent topComponent = EditorTopComponent.getRegistry().getActivated();
> 
> Can be simplified to:
> Suggestion:
> 
>         TopComponent topComponent = getRegistry().getActivated();

done

> src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/EnableBlockLayoutAction.java line 42:
> 
>> 40: public class EnableBlockLayoutAction extends AbstractAction implements PropertyChangeListener {
>> 41: 
>> 42:     EditorTopComponent editor;
> 
> Should be made `private` (and also `final`):
> Suggestion:
> 
>     private final EditorTopComponent editor;

done

> src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/ReduceDiffAction.java line 104:
> 
>> 102:     public Action createContextAwareInstance(Lookup actionContext) {
>> 103:         return this;
>> 104:     }
> 
> It seems you are not really creating instances anymore by looking at all the new overrides of `createContextAwareInstance()` which just return `this`. Is this method still required or can the usages be replaced by directly using the caller object?

Unfortunately not, we need to override it because we extend `CallableSystemAction`

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

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


More information about the hotspot-compiler-dev mailing list