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