From 614dbbff2f1e4284cbd0a5b1fb7e017ca7137805 Mon Sep 17 00:00:00 2001 From: Danylo Boiko Date: Fri, 29 Jul 2022 21:10:29 +0300 Subject: [PATCH] [turbolizer] TurboFan nodes history improvements Added: - history's circles titles - history's records titles - ability to move to node from history view - new hotkey for turboshaft layout Bug: v8:7327 Change-Id: I7ecfdbef2c1bf9534c76f8ac253e846beeea8cb3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779909 Reviewed-by: Tobias Tebbi Commit-Queue: Danylo Boiko Cr-Commit-Position: refs/heads/main@{#82089} --- tools/turbolizer/css/turbo-visualizer.css | 5 ++ tools/turbolizer/info-view.html | 6 +- tools/turbolizer/src/common/constants.ts | 2 +- tools/turbolizer/src/graphmultiview.ts | 13 ++-- .../src/selection/selection-broker.ts | 6 +- .../src/selection/selection-handler.ts | 5 +- tools/turbolizer/src/turbo-visualizer.ts | 3 +- .../turbolizer/src/views/disassembly-view.ts | 4 +- tools/turbolizer/src/views/graph-view.ts | 15 ++-- tools/turbolizer/src/views/history-view.ts | 75 +++++++++++++------ tools/turbolizer/src/views/movable-view.ts | 6 +- tools/turbolizer/src/views/text-view.ts | 4 +- .../src/views/turboshaft-graph-view.ts | 32 +++++--- 13 files changed, 119 insertions(+), 57 deletions(-) diff --git a/tools/turbolizer/css/turbo-visualizer.css b/tools/turbolizer/css/turbo-visualizer.css index a2946ef68b..6119f1f39f 100644 --- a/tools/turbolizer/css/turbo-visualizer.css +++ b/tools/turbolizer/css/turbo-visualizer.css @@ -787,3 +787,8 @@ svg.history-svg-container .history-content-scroll { svg.history-svg-container .history-item tspan { font-size: var(--history-item-tspan-font-size); } + +svg.history-svg-container .history-item-record tspan:hover { + fill: #606060; + cursor: pointer; +} diff --git a/tools/turbolizer/info-view.html b/tools/turbolizer/info-view.html index 729e6e2a37..6b396d1421 100644 --- a/tools/turbolizer/info-view.html +++ b/tools/turbolizer/info-view.html @@ -89,9 +89,13 @@ Copy hovered node's info - u + y Collapse unused blocks (blocks that don't have direct inputs and outputs of a hovered node) + + u + Collapse unused blocks (blocks that don't have direct inputs and outputs of selected nodes) + diff --git a/tools/turbolizer/src/common/constants.ts b/tools/turbolizer/src/common/constants.ts index 5993fff279..4fdcb4cd2a 100644 --- a/tools/turbolizer/src/common/constants.ts +++ b/tools/turbolizer/src/common/constants.ts @@ -24,7 +24,7 @@ export const SOURCE_PANE_DEFAULT_PERCENT = 1 / 4; export const DISASSEMBLY_PANE_DEFAULT_PERCENT = 3 / 4; export const RANGES_PANE_HEIGHT_DEFAULT_PERCENT = 3 / 4; export const RANGES_PANE_WIDTH_DEFAULT_PERCENT = 1 / 2; -export const HISTORY_DEFAULT_HEIGHT_PERCENT = 1 / 5; +export const HISTORY_DEFAULT_HEIGHT_PERCENT = 1 / 3.5; export const HISTORY_CONTENT_INDENT = 8; export const HISTORY_SCROLLBAR_WIDTH = 6; export const CLOSE_BUTTON_RADIUS = 25; diff --git a/tools/turbolizer/src/graphmultiview.ts b/tools/turbolizer/src/graphmultiview.ts index 6d6775398d..53b1c95caa 100644 --- a/tools/turbolizer/src/graphmultiview.ts +++ b/tools/turbolizer/src/graphmultiview.ts @@ -93,6 +93,13 @@ export class GraphMultiView extends View { this.displayPhase(this.sourceResolver.getPhase(initialPhaseIndex)); } + public displayPhaseByName(phaseName: string, selection?: SelectionStorage): void { + this.currentPhaseView.hide(); + const phaseId = this.sourceResolver.getPhaseIdByName(phaseName); + this.selectMenu.selectedIndex = phaseId; + this.displayPhase(this.sourceResolver.getPhase(phaseId), selection); + } + public onresize(): void { this.currentPhaseView?.onresize(); } @@ -116,12 +123,6 @@ export class GraphMultiView extends View { this.currentPhaseView = view; } - private displayPhaseByName(phaseName: string, selection?: SelectionStorage): void { - const phaseId = this.sourceResolver.getPhaseIdByName(phaseName); - this.selectMenu.selectedIndex = phaseId; - this.displayPhase(this.sourceResolver.getPhase(phaseId), selection); - } - private displayNextGraphPhase(): void { let nextPhaseIndex = this.selectMenu.selectedIndex + 1; while (nextPhaseIndex < this.sourceResolver.phases.length) { diff --git a/tools/turbolizer/src/selection/selection-broker.ts b/tools/turbolizer/src/selection/selection-broker.ts index 2d8e8db272..3a1284cf36 100644 --- a/tools/turbolizer/src/selection/selection-broker.ts +++ b/tools/turbolizer/src/selection/selection-broker.ts @@ -86,11 +86,11 @@ export class SelectionBroker { } } - // TODO (danylo boiko) Add instructionOffsets type - public broadcastInstructionSelect(from, instructionOffsets, selected: boolean): void { + public broadcastInstructionSelect(from, instructionOffsets: Array, selected: boolean): + void { // Select the lines from the disassembly (right panel) for (const handler of this.instructionHandlers) { - if (handler != from) handler.brokeredInstructionSelect(instructionOffsets, selected); + if (handler != from) handler.brokeredInstructionSelect([instructionOffsets], selected); } // Select the lines from the source panel (left panel) diff --git a/tools/turbolizer/src/selection/selection-handler.ts b/tools/turbolizer/src/selection/selection-handler.ts index 96b3bf5f16..3c62ea440c 100644 --- a/tools/turbolizer/src/selection/selection-handler.ts +++ b/tools/turbolizer/src/selection/selection-handler.ts @@ -31,7 +31,8 @@ export interface BlockSelectionHandler { export interface InstructionSelectionHandler { select(instructionIds: Array, selected: boolean): void; clear(): void; - brokeredInstructionSelect(instructionsOffsets: Array<[number, number]>, selected: boolean): void; + brokeredInstructionSelect(instructionsOffsets: Array<[number, number]> | Array>, + selected: boolean): void; } export interface SourcePositionSelectionHandler { @@ -42,7 +43,7 @@ export interface SourcePositionSelectionHandler { export interface RegisterAllocationSelectionHandler { // These are called instructionIds since the class of the divs is "instruction-id" - select(instructionIds: Array, selected: boolean): void; + select(instructionIds: Array, selected: boolean): void; clear(): void; brokeredRegisterAllocationSelect(instructionsOffsets: Array<[number, number]>, selected: boolean): void; diff --git a/tools/turbolizer/src/turbo-visualizer.ts b/tools/turbolizer/src/turbo-visualizer.ts index 122734d0bb..e4e47a6c5a 100644 --- a/tools/turbolizer/src/turbo-visualizer.ts +++ b/tools/turbolizer/src/turbo-visualizer.ts @@ -97,7 +97,8 @@ window.onload = function () { multiview = new GraphMultiView(C.INTERMEDIATE_PANE_ID, selectionBroker, sourceResolver); multiview.show(); - historyView = new HistoryView(C.HISTORY_ID, selectionBroker, sourceResolver); + historyView = new HistoryView(C.HISTORY_ID, selectionBroker, sourceResolver, + multiview.displayPhaseByName.bind(multiview)); historyView.show(); } catch (err) { if (window.confirm("Error: Exception during load of TurboFan JSON file:\n" + diff --git a/tools/turbolizer/src/views/disassembly-view.ts b/tools/turbolizer/src/views/disassembly-view.ts index c82f3dcc99..5ef2ddd421 100644 --- a/tools/turbolizer/src/views/disassembly-view.ts +++ b/tools/turbolizer/src/views/disassembly-view.ts @@ -196,8 +196,8 @@ export class DisassemblyView extends TextView { view.updateSelection(); broker.broadcastClear(this); }, - brokeredInstructionSelect: function (instructionsOffsets: Array<[number, number]>, - selected: boolean) { + brokeredInstructionSelect: function (instructionsOffsets: Array<[number, number]> | + Array>, selected: boolean) { const firstSelect = view.offsetSelection.isEmpty(); for (const instructionOffset of instructionsOffsets) { const keyPcOffsets = view.sourceResolver.instructionsPhase diff --git a/tools/turbolizer/src/views/graph-view.ts b/tools/turbolizer/src/views/graph-view.ts index 334e7a6909..af9b0e26a0 100644 --- a/tools/turbolizer/src/views/graph-view.ts +++ b/tools/turbolizer/src/views/graph-view.ts @@ -28,7 +28,8 @@ export class GraphView extends MovableView { drag: d3.DragBehavior; constructor(idOrContainer: string | HTMLElement, broker: SelectionBroker, - showPhaseByName: (name: string) => void, toolbox: HTMLElement) { + showPhaseByName: (name: string, selection: SelectionStorage) => void, + toolbox: HTMLElement) { super(idOrContainer, broker, showPhaseByName, toolbox); this.state.selection = new SelectionMap(node => node.identifier(), @@ -79,6 +80,7 @@ export class GraphView extends MovableView { if (selectedNodes?.length > 0) { this.connectVisibleSelectedElements(this.state.selection); + this.updateGraphVisibility(); this.viewSelection(); } else { if (this.state.cacheLayout && data.transform) { @@ -367,6 +369,10 @@ export class GraphView extends MovableView { return new SelectionStorage(); } + for (const node of rememberedSelection.adaptedNodes) { + this.graph.makeNodeVisible(node); + } + for (const [key, node] of rememberedSelection.nodes.entries()) { // Adding survived nodes (with the same id) const survivedNode = this.graph.nodeMap[key]; @@ -711,9 +717,9 @@ export class GraphView extends MovableView { } private selectOrigins(): void { + const selection = new SelectionStorage(); const origins = new Array(); let phase = this.phaseName; - const selection = new Set(); for (const node of this.state.selection) { const origin = node.nodeLabel.origin; if (origin && origin instanceof NodeOrigin) { @@ -722,14 +728,13 @@ export class GraphView extends MovableView { if (phase === this.phaseName && node) { origins.push(node); } else { - selection.add(origin.identifier()); + selection.adaptNode(origin.identifier()); } } } // Only go through phase reselection if we actually need // to display another phase. - if (selection.size > 0 && phase !== this.phaseName) { - this.hide(); + if (selection.isAdapted() && phase !== this.phaseName) { this.showPhaseByName(phase, selection); } else if (origins.length > 0) { this.nodeSelectionHandler.clear(); diff --git a/tools/turbolizer/src/views/history-view.ts b/tools/turbolizer/src/views/history-view.ts index bce3631e05..9ce2067d63 100644 --- a/tools/turbolizer/src/views/history-view.ts +++ b/tools/turbolizer/src/views/history-view.ts @@ -12,6 +12,7 @@ import { GraphNode } from "../phases/graph-phase/graph-node"; import { HistoryHandler } from "../selection/selection-handler"; import { GraphPhase } from "../phases/graph-phase/graph-phase"; import { NodeOrigin } from "../origin"; +import { SelectionStorage } from "../selection/selection-storage"; export class HistoryView extends View { node: GraphNode; @@ -27,11 +28,14 @@ export class HistoryView extends View { y: number; maxNodeWidth: number; maxPhaseNameWidth: number; + showPhaseByName: (name: string, selection: SelectionStorage) => void; - constructor(id: string, broker: SelectionBroker, sourceResolver: SourceResolver) { + constructor(id: string, broker: SelectionBroker, sourceResolver: SourceResolver, + showPhaseByName: (name: string, selection: SelectionStorage) => void) { super(id); this.broker = broker; this.sourceResolver = sourceResolver; + this.showPhaseByName = showPhaseByName; this.historyHandler = this.initializeNodeSelectionHandler(); this.broker.addHistoryHandler(this.historyHandler); this.phaseIdToHistory = new Map(); @@ -60,8 +64,11 @@ export class HistoryView extends View { .style("visibility", "hidden"); const dragHandler = d3.drag().on("drag", () => { - this.x += d3.event.dx; - this.y += d3.event.dy; + const rect = document.body.getBoundingClientRect(); + const x = this.x + d3.event.dx; + this.x = d3.event.dx > 0 ? Math.min(x, rect.width - this.getWidth()) : Math.max(x, 0); + const y = this.y + d3.event.dy; + this.y = d3.event.dy > 0 ? Math.min(y, rect.height - this.getHeight()) : Math.max(y, 0); this.svg.attr("transform", _ => `translate(${this.x},${this.y})`); }); @@ -125,7 +132,9 @@ export class HistoryView extends View { let recordY = 0; for (const [phaseId, phaseHistory] of this.phaseIdToHistory.entries()) { + if (!phaseHistory.hasChanges()) continue; const phaseName = this.sourceResolver.getPhaseNameById(phaseId); + this.historyList .append("text") .classed("history-item", true) @@ -164,16 +173,25 @@ export class HistoryView extends View { .classed("history-item", true) .attr("r", this.labelBox.height / 3.5) .attr("cx", this.labelBox.height / 3) - .attr("cy", this.labelBox.height / 3 + recordY) - .attr("fill", `url(#${circleId})`); + .attr("cy", this.labelBox.height / 2.5 + recordY) + .attr("fill", `url(#${circleId})`) + .append("title") + .text(`[${record.toString()}]`); this.historyList .append("text") - .classed("history-item", true) + .classed("history-item history-item-record", true) .attr("dy", recordY) .attr("dx", this.labelBox.height * 0.75) .append("tspan") - .text(record.node.displayLabel); + .text(record.node.displayLabel) + .on("click", () => { + const selectionStorage = new SelectionStorage(); + selectionStorage.adaptNode(record.node.identifier()); + this.showPhaseByName(phaseName, selectionStorage); + }) + .append("title") + .text(record.node.getTitle()); recordY += this.labelBox.height; } } @@ -198,13 +216,6 @@ export class HistoryView extends View { content.node().appendChild(d3.select(this).node() as HTMLElement); }); - this.historyList - .append("rect") - .attr("width", historyArea.width) - .attr("height", historyArea.height) - .attr("transform", `translate(${historyArea.x},${historyArea.y})`) - .attr("opacity", 0); - this.historyList .append("clipPath") .attr("id", "history-clip-path") @@ -240,7 +251,7 @@ export class HistoryView extends View { if (!isNaN(scrollBarPosition)) scrollBar.attr("y", scrollBarPosition); }; - this.historyList.on("wheel", () => { + this.svg.on("wheel", () => { updateScrollPosition(d3.event.deltaY); }); @@ -267,6 +278,7 @@ export class HistoryView extends View { const uniqueAncestors = new Set(); const coefficient = this.getCoefficient("history-item-tspan-font-size"); let prevNode = null; + let first = true; for (let i = 0; i < this.sourceResolver.phases.length; i++) { const phase = this.sourceResolver.getPhase(i); if (!(phase instanceof GraphPhase)) continue; @@ -305,6 +317,11 @@ export class HistoryView extends View { this.addToHistory(i, node, HistoryChange.InplaceUpdated); } + if (first) { + this.addToHistory(i, node, HistoryChange.Emerged); + first = false; + } + this.addToHistory(i, node, HistoryChange.Current); prevNode = node; } @@ -395,18 +412,19 @@ export class HistoryView extends View { } private getHeight(): number { - const clientRect = document.body.getBoundingClientRect(); - return clientRect.height * C.HISTORY_DEFAULT_HEIGHT_PERCENT; + return window.screen.availHeight * C.HISTORY_DEFAULT_HEIGHT_PERCENT; } private getHistoryChangeColor(historyChange: HistoryChange): string { switch (historyChange) { case HistoryChange.Current: return "rgb(255, 167, 0)"; + case HistoryChange.Emerged: + return "rgb(160, 83, 236)"; case HistoryChange.Lowered: return "rgb(0, 255, 0)"; case HistoryChange.InplaceUpdated: - return "rgb(0, 0, 255)"; + return "rgb(57, 57, 208)"; case HistoryChange.Removed: return "rgb(255, 0, 0)"; case HistoryChange.Survived: @@ -420,9 +438,7 @@ export class HistoryView extends View { console.log(`${key} ${this.sourceResolver.getPhaseNameById(key)}`); const phaseHistory = this.phaseIdToHistory.get(key); for (const record of phaseHistory.nodeIdToRecord.values()) { - const changes = Array.from(record.changes.values()).sort() - .map(i => HistoryChange[i]).join(", "); - console.log(`[${changes}] `, record.node); + console.log(record.toString(), record.node); } } } @@ -444,6 +460,13 @@ export class PhaseHistory { } this.nodeIdToRecord.get(key).addChange(change); } + + public hasChanges(): boolean { + for (const record of this.nodeIdToRecord.values()) { + if (record.hasChanges()) return true; + } + return false; + } } export class HistoryRecord { @@ -458,10 +481,20 @@ export class HistoryRecord { public addChange(change: HistoryChange): void { this.changes.add(change); } + + public hasChanges(): boolean { + return this.changes.size > 1 || + (this.changes.size == 1 && !this.changes.has(HistoryChange.Current)); + } + + public toString(): string { + return Array.from(this.changes.values()).sort().map(i => HistoryChange[i]).join(", "); + } } export enum HistoryChange { Current, + Emerged, Lowered, InplaceUpdated, Removed, diff --git a/tools/turbolizer/src/views/movable-view.ts b/tools/turbolizer/src/views/movable-view.ts index 37627e3b16..e5e5428b26 100644 --- a/tools/turbolizer/src/views/movable-view.ts +++ b/tools/turbolizer/src/views/movable-view.ts @@ -16,12 +16,13 @@ import { TurboshaftGraph } from "../turboshaft-graph"; import { Graph } from "../graph"; import { TurboshaftGraphNode } from "../phases/turboshaft-graph-phase/turboshaft-graph-node"; import { GraphNode } from "../phases/graph-phase/graph-node"; +import { SelectionStorage } from "../selection/selection-storage"; export abstract class MovableView extends PhaseView { phaseName: string; graph: GraphType; broker: SelectionBroker; - showPhaseByName: (name: string, selection: Set) => void; + showPhaseByName: (name: string, selection: SelectionStorage) => void; toolbox: HTMLElement; state: MovableViewState; nodeSelectionHandler: NodeSelectionHandler & ClearableHandler; @@ -35,7 +36,8 @@ export abstract class MovableView ext public abstract svgKeyDown(): void; constructor(idOrContainer: string | HTMLElement, broker: SelectionBroker, - showPhaseByName: (name: string) => void, toolbox: HTMLElement) { + showPhaseByName: (name: string, selection: SelectionStorage) => void, + toolbox: HTMLElement) { super(idOrContainer); this.broker = broker; this.showPhaseByName = showPhaseByName; diff --git a/tools/turbolizer/src/views/text-view.ts b/tools/turbolizer/src/views/text-view.ts index 7f38a0dba9..428abd85cd 100644 --- a/tools/turbolizer/src/views/text-view.ts +++ b/tools/turbolizer/src/views/text-view.ts @@ -258,10 +258,10 @@ export abstract class TextView extends PhaseView { & ClearableHandler { const view = this; return { - select: function (instructionIds: Array, selected: boolean) { + select: function (instructionIds: Array, selected: boolean) { view.registerAllocationSelection.select(instructionIds, selected); view.updateSelection(); - view.broker.broadcastInstructionSelect(null, [instructionIds], selected); + view.broker.broadcastInstructionSelect(null, instructionIds, selected); }, clear: function () { view.registerAllocationSelection.clear(); diff --git a/tools/turbolizer/src/views/turboshaft-graph-view.ts b/tools/turbolizer/src/views/turboshaft-graph-view.ts index 5bdd7592d0..d881842c73 100644 --- a/tools/turbolizer/src/views/turboshaft-graph-view.ts +++ b/tools/turbolizer/src/views/turboshaft-graph-view.ts @@ -35,7 +35,8 @@ export class TurboshaftGraphView extends MovableView { blockDrag: d3.DragBehavior; constructor(idOrContainer: string | HTMLElement, broker: SelectionBroker, - showPhaseByName: (name: string) => void, toolbox: HTMLElement) { + showPhaseByName: (name: string, selection: SelectionStorage) => void, + toolbox: HTMLElement) { super(idOrContainer, broker, showPhaseByName, toolbox); this.state.selection = new SelectionMap(node => node.identifier()); @@ -155,7 +156,12 @@ export class TurboshaftGraphView extends MovableView { } break; case 85: // 'u' - this.collapseUnusedBlocks(); + this.collapseUnusedBlocks(this.state.selection.selection.values()); + break; + case 89: // 'y' + const node = this.graph.nodeMap[this.hoveredNodeIdentifier]; + if (!node) return; + this.collapseUnusedBlocks([node]); break; default: eventHandled = false; @@ -735,18 +741,22 @@ export class TurboshaftGraphView extends MovableView { this.updateGraphVisibility(); } - private collapseUnusedBlocks(): void { - const node = this.graph.nodeMap[this.hoveredNodeIdentifier]; - if (!node) return; + private collapseUnusedBlocks(usedNodes: Iterable): void { + const usedBlocks = new Set(); + for (const node of usedNodes) { + usedBlocks.add(node.block); - const usedBlocks = new Set([node.block]); - for (const input of node.inputs) { - usedBlocks.add(input.source.block); - } - for (const output of node.outputs) { - usedBlocks.add(output.target.block); + for (const input of node.inputs) { + usedBlocks.add(input.source.block); + } + + for (const output of node.outputs) { + usedBlocks.add(output.target.block); + } } + if (usedBlocks.size == 0) return; + for (const block of this.graph.blockMap) { block.collapsed = !usedBlocks.has(block); }