RFR: JDK-8293480: IGV: Update Bytecode and ControlFlow Component immediately when opening a new graph [v6]
Roberto CastaƱeda Lozano
rcastanedalo at openjdk.org
Tue Sep 20 09:46:04 UTC 2022
On Fri, 16 Sep 2022 14:58:04 GMT, Tobias Holenstein <tholenstein at openjdk.org> wrote:
>> The `BytecodeViewTopComponent` and `ControlFlowTopComponent` represent information depending on which graph is opened in `EditorTopComponent`. Previously `BytecodeViewTopComponent` and `ControlFlowTopComponent` did not update their contents immediately when a new graph from another group was opened in `EditorTopComponent`. They were also not updated when switching between two tabs of an open graph.
>> `OutlineTopComponent` had the same problem to update the selected graphs according to the `EditorTopComponent`
>>
>> <img width="1291" alt="Update" src="https://user-images.githubusercontent.com/71546117/188860123-825a67fd-aab3-4500-8a47-8fa56eb9aeed.png">
>>
>> **Analysis**
>> `BytecodeViewTopComponent`, `ControlFlowTopComponent` and `OutlineTopComponent` each represent information that depends on the `InputGraph` of the currently or last active `EditorTopComponent`. This information is made available globally by adding a new `InputGraphProvider` to the `Lookup` of `EditorTopComponent` each time the `InputGraph` changes. `BytecodeViewTopComponent`, `ControlFlowTopComponent` and `OutlineTopComponent` implement a `LookupListener` that calls `resultChanged(LookupEvent lookupEvent)` whenever a `InputGraphProvider` changes in in the lookup of `Utilities.actionsGlobalContext()`. When such a change happens the last active `InputGraphProvider` is retrieved from the `LookupHistory` by the listening components.
>>
>> **Problem**
>> First, we missed to `fire()` a `diagramChangedEvent` in the constructor of `EditorTopComponent` which trigger to add the `InputGraphProvider` to the `Lookup`
>>
>> Second, `Utilities.actionsGlobalContext()` returns a `Lookup` of the active (focused) `TopComponent's` `Lookup`. Unfortunately, when the last `EditorTopComponent` the `LookupListener` does not get called and there is no way to call is manually.
>>
>> **New Approach**
>> We extends the `LookupHistory` class such that we can add a `ChangedListener` that gets called whenever the last active `InputGraphProvider` is cached. So instead of listening to changes in `Utilities.actionsGlobalContext()` and then consulting the `LookupHistory`, now `BytecodeViewTopComponent`, `ControlFlowTopComponent` and `OutlineTopComponent` listen to changes in the `LookupHistory` directly. This way we can now call `terminate` in the `LookupHistory` whenever we close a `EditorTopComponent`, which directly notifies the listeners.
>
> Tobias Holenstein has updated the pull request incrementally with four additional commits since the last revision:
>
> - Fix for: parent is null for DiffGraph
> - remove DiagramProvider
> - add ChangedListener to LookupHistory
> - update OutlineTopComponent graph closed
I tried out the latest changes and it works as expected, thanks! However, I found one more case where the Control Flow window get out of sync with the active graph: if you open a graph and then go to the Outline and click on "Remove selected graphs and groups" (where only the opened graph is selected), the active graph changes to another graph in the same group but the Control Flow window still shows the CFG of the removed graph. Even though this problem is not introduced by this changeset, I suggest addressing it here.
Also, please make sure that the NetBeans-generated code moved by this changeset is still editable in its new context.
-------------
Marked as reviewed by rcastanedalo (Reviewer).
PR: https://git.openjdk.org/jdk/pull/10196
More information about the hotspot-compiler-dev
mailing list