RFR: 8302738: IGV: refine 'Simplify graph' filter [v2]

Roberto Castañeda Lozano rcastanedalo at openjdk.org
Mon Mar 27 11:09:34 UTC 2023


On Mon, 20 Mar 2023 10:13:15 GMT, Tobias Holenstein <tholenstein at openjdk.org> wrote:

>> Roberto Castañeda Lozano has updated the pull request incrementally with five additional commits since the last revision:
>> 
>>  - Increase the bold text line factor slightly
>>  - Add extra horizontal margin for long labels and let them overflow within the node
>>  - Select slots as well
>>  - Remove code that is commented out
>>  - Assert inputLabel is non-null
>
> src/utils/IdealGraphVisualizer/Filter/src/main/java/com/sun/hotspot/igv/filter/FilterChain.java line 1:
> 
>> 1: /*
> 
> I think `applyInOrder` can be simplified as this :
> 
> public void applyInOrder(Diagram d, FilterChain sequence) {
>         for (Filter f : sequence.getFilters()) {
>             if (filters.contains(f)) {
>                 f.apply(d);
>             }
>         }
>     }
> 
> 
> Reason: `FilterChain ordering` is the same as `this` in `FilterChain`. Usually `filters` are already in the order that we want them to apply. Only exception is when the user manually reoders the filters. `FilterChain sequence` contains all the filters in the order that they appear in the list. `filters` are the filters that are selected by the user and should alway be a subset of `sequence`. Therefore we can just iterate through `sequence` to get the correct order and apply each filter that is selected (contained in `filters`)

Thanks for the suggestion! I tested your assumption ("`FilterChain ordering` is the same as `this` in `FilterChain`") but it does not hold in this PR. Note that the filter list is never ordered outside of `applyInOrder`. In any case, as I mentioned in https://github.com/openjdk/jdk/pull/12714#discussion_r1148879518, I propose to go with the fix in JDK-8302644 and discard the filter ordering changes from this PR.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/12955#discussion_r1149152222


More information about the hotspot-compiler-dev mailing list