RFR: 8302738: IGV: refine 'Simplify graph' filter
Tobias Holenstein
tholenstein at openjdk.org
Mon Mar 20 11:55:29 UTC 2023
On Thu, 9 Mar 2023 18:28:02 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:
> The "Simplify graph" filter abstracts away details from the graph that are typically unnecessary for debugging or analyzing the represented program. This changeset decouples this filter into two:
>
> - "Simplify graph", which hides elements that are typically (but not always) unnecessary, and
> - "Condense graph", which makes the graph more compact without loss of information.
>
> Together, these two filters reduce the average graph size by a factor of 1.6x (nodes) and 1.9x (edges):
>
> 
>
> Besides decoupling the "Simplify graph" filter, the changeset extends its functionality by:
> - combining Bool and conversion nodes into their predecessors,
> - inlining all Parm nodes except control into their successors (this removes lots of long edges),
> - removing "top" inputs from call-like nodes,
> - inlining more source nodes (such as MachTemp and ThreadLocal) into their successors,
> - pretty-printing the labels of many inlined and combined nodes such as Bool comparisons or Catch projections (via a new filter that edits node properties), and
> - using a sparse representation of nodes with empty inputs (e.g. call-like nodes after applying "Simplify graph").
>
> The sparse input representation shows dots between non-contiguous inputs, instead of horizontal space proportional to the number of empty inputs. This helps reducing node width, which is known to improve overall layout quality:
>
> 
>
> Note that the exact input indices can still be retrieved via the incoming edge's tooltips:
>
> 
>
> The control-flow graph view is also adapted to this representation:
>
> 
>
> #### Additional improvements
>
> Additionally, this changeset:
> - ensures that the selected filter subset is applied in the order listed in the "Filter" window (this is necessary for combining effectively the "Simplify graph" and "Condense graph" filters, but is also generally desirable for simplicity and consistency),
> - introduces a complementary filter "Show custom node info" (enabled by default) that extends the labels of call and exception-creation nodes with custom information,
> - extends the search functionality so that combined and inlined nodes can also be searched on and selected, and
> - defines and documents JavaScript helpers to simplify the new and existing available filters.
>
> Here is an example of the effect of the new "Show custom node info" filter:
>
> 
>
> ### Testing
>
> #### Functionality
>
> - Tested the functionality manually on a small selection of graphs.
>
> - Tested automatically that viewing thousands of graphs in the three views with different filter subsets enabled does not trigger any assertion failure (by instrumenting IGV to view graphs as they are loaded and running `java -Xcomp -XX:-TieredCompilation -XX:PrintIdealGraphLevel=4`).
>
> #### Performance
>
> Measured the combined filter application and view creation time for the sea-of-nodes view on a selection of 100 medium-sized graphs (200-500 nodes). On average, applying the new "Show custom node info" filter introduces a minimal overhead of around 1%, which motivates enabling it by default. Applying the "simplify graph" and "condense graph" on top actually gives a speedup of about 12%, since the additional filter application time is amortized by laying out and drawing fewer nodes. However, these filters are not enabled by default, since they cause a (minor) loss of information which is not desirable in every use case.
>
> The graph size reduction and performance results are [attached](https://github.com/openjdk/jdk/files/10934804/performance-evaluation.ods) (note that each time measurement in the sheet corresponds to the median of ten runs).
Thanks @robcasloz for working on this. Extending the filter in the way you did it is very useful in my opinion! I tested your changes and everything seems to work as expected. (See my comments in the code for more)
src/utils/IdealGraphVisualizer/Filter/src/main/java/com/sun/hotspot/igv/filter/CombineFilter.java line 71:
> 69: }
> 70: }
> 71:
I think `assert slot != null;` should be moved up here
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`)
src/utils/IdealGraphVisualizer/Graph/src/main/java/com/sun/hotspot/igv/graph/Figure.java line 343:
> 341: inputLabel = nodeTinyLabel;
> 342: }
> 343: if (inputLabel != null) {
according to my IDE inputLabel is here always non-null.
src/utils/IdealGraphVisualizer/Graph/src/main/java/com/sun/hotspot/igv/graph/InputSlot.java line 76:
> 74: int gapAmount = (int)((getPosition() + 1)*gapRatio);
> 75: return new Point(gapAmount + Figure.getSlotsWidth(Figure.getAllBefore(getFigure().getInputSlots(), this)) + getWidth()/2, -Figure.SLOT_START);
> 76: //return new Point((getFigure().getWidth() / (getFigure().getInputSlots().size() * 2)) * (getPosition() * 2 + 1), -Figure.SLOT_START);
perhaps remove this old comment
src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/DiagramScene.java line 218:
> 216: if (ids.contains(figure.getInputNode().getId())) {
> 217: selectedFigures.add(figure);
> 218: }
Suggestion:
}
for (Slot slot : figure.getSlots()) {
if (!Collections.disjoint(slot.getSource().getSourceNodesAsSet(), ids)) {
highlightedObjects.add(slot);
}
}
I am not sure what your intent was in adding the slots to the selected objects. If you wanted the slots to be selected globally in "link global node selection" mode, you need to add the following code to make it work
-------------
Changes requested by tholenstein (Committer).
PR: https://git.openjdk.org/jdk/pull/12955
More information about the hotspot-compiler-dev
mailing list