RFR: JDK-8302644: IGV: Apply filters per graph tab and not globally [v6]

Mon Mar 27 07:33:34 UTC 2023

On Thu, 23 Mar 2023 09:43:26 GMT, Tobias Holenstein <tholenstein at openjdk.org> wrote:

>> In IGV the user can apply a set of filters to a graph. Currently, the same set of selected filters is applied to all graphs (globally). 
>> 
>> - With this change the use can define a set of filters for each individual graph tab using the `--Local--` profile
>> - Further a filter profile can be created that represents a set of filter. This filter profile can the be selected in each graph tab individually.
>> 
>> ### Global profile
>> Each tab has a  `--Global--` filter profile which is selected when opening a graph. Filters applied to the `--Global--` profile are applied to all tabs that have the `--Global--` profile selected. 
>> 
>> ### Local profile
>> Each tab has its own  `--Local--` filter profile. Filters applied to the `--Local--` profile are applied only to the currently selected tabs. Only one tab can be selected at a time and a tab gets selected by clicking on it. To make it more clear which tab is currently selected, the title of the selected tab is displayed in **bold** font.
>> <img width="1252" alt="tabA" src="https://user-images.githubusercontent.com/71546117/222127949-38a3ed8f-8f61-4485-aca6-67cbc341a4d1.png">
>> 
>> When clicking on a different tab with a different `--Local--` profile, the selected filters get updated accordingly.
>> <img width="1256" alt="tabB" src="https://user-images.githubusercontent.com/71546117/222127988-66be7a76-a638-4c40-b8f1-ad56a1ab1fd7.png">
>> 
>> ### New profile
>> The user can also create a new filter profile and give it a name. E.g. `My Filters`
>> <img width="396" alt="newProfile" src="https://user-images.githubusercontent.com/71546117/222128202-ea1f8de6-9260-4a78-b396-b69a354dc883.png">
>> 
>> The `My Filters` profile is then globally available to other tabs as well 
>> <img width="254" alt="selectProfile" src="https://user-images.githubusercontent.com/71546117/222128229-e33ab7b1-ebbe-4abc-a759-b50965ca64e1.png">
>> 
>> 
>> ### Filters for cloned tabs
>> When the user clones a tab, the `--Local--` profile gets cloned as well. Further the clone has the same filter profile selected when it gets opened
>> <img width="1250" alt="cloneTab" src="https://user-images.githubusercontent.com/71546117/222128014-2ca5eac0-ae8a-42da-b407-acc62c09edcf.png">
>> 
>> ### Saving of filters and profiles
>> When the users closes IGV, the filters (in their exact order) are save, as well as the filter profiles. The profile that was last used is selected when opening IGV.
>
> Tobias Holenstein has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - always select previous profile for new tabs
>  - .js ending for filters
>  - save order of filters

Thanks for addressing my comments again, Toby! I like the additional functionality and that the changeset preserves the current workflow by default. I just have a question about graph viewing performance and a couple of minor comments.

src/utils/IdealGraphVisualizer/Filter/src/main/java/com/sun/hotspot/igv/filter/FilterChain.java line 71:

> 69:     }
> 70: 
> 71:     public void applyInOrder(Diagram diagram, FilterChain filterOrder) {

I have also addressed this in [#12955](https://github.com/openjdk/jdk/pull/12955), but I think your solution of sorting the filter list upfront, rather than every time filters are applied, is preferable. I will wait for this PR to be integrated and then exclude the corresponding changes from #12955.

src/utils/IdealGraphVisualizer/FilterWindow/src/main/java/com/sun/hotspot/igv/filterwindow/FilterTopComponent.java line 454:

> 452: 
> 453:             String after = (String) fo.getAttribute(AFTER_ID);
> 454:             System.out.println(displayName + " after " + after);

Please remove.

src/utils/IdealGraphVisualizer/FilterWindow/src/main/java/com/sun/hotspot/igv/filterwindow/FilterTopComponent.java line 632:

> 630:         allFiltersOrdered.sortBy(order);
> 631: 
> 632:         System.out.println("readExternal");

Please remove.

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

> 324: 
> 325:     // called when the filter in filterChain changed, but not filterChain itself
> 326:     private void filterChanged() {

After applying this PR, `DiagramViewModel::filterChanged()` is fired every time a new graph in a group is viewed. This is not a functional bug, but it causes the expensive `DiagramViewModel::rebuildDiagram()` to be called twice in that scenario (the other call comes from `DiagramViewModel::changed()`). Would it be possible to arrange the code so that `DiagramViewModel::rebuildDiagram()` is only called once when a new graph in a group is viewed?

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

Changes requested by rcastanedalo (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/12714#pullrequestreview-1358464092
PR Review Comment: https://git.openjdk.org/jdk/pull/12714#discussion_r1148879518
PR Review Comment: https://git.openjdk.org/jdk/pull/12714#discussion_r1148874121
PR Review Comment: https://git.openjdk.org/jdk/pull/12714#discussion_r1148874585
PR Review Comment: https://git.openjdk.org/jdk/pull/12714#discussion_r1148885678