[Optimize flamechart][2/4] Replace Speedscope types with custom stack…

… frame type Lays groundwork for #75 by reducing the flamechart data passed around the app. Was also intended to help #50 by moving some division operations from the hot render loops into preprocessFlamechart, but any performance gains are negligible. * `yarn flow`: no errors in modified code * `yarn lint` * `yarn start`: no performance difference
MLH-Fellowship · Jul 28, 2020 · 64a8324 · 64a8324
1 parent dbb59c9
commit 64a8324
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 67 deletions.
diff --git a/src/CanvasPage.js b/src/CanvasPage.js
@@ -191,7 +191,7 @@ function AutoSizedCanvas({data, height, width}: AutoSizedCanvasProps) {
         hoveredEvent &&
         (hoveredEvent.event ||
           hoveredEvent.measure ||
-          hoveredEvent.flamechartNode)
+          hoveredEvent.flamechartStackFrame)
       ) {
         setMouseLocation({
           x: interaction.payload.event.x,
@@ -226,7 +226,7 @@ function AutoSizedCanvas({data, height, width}: AutoSizedCanvasProps) {
         if (!hoveredEvent || hoveredEvent.event !== event) {
           setHoveredEvent({
             event,
-            flamechartNode: null,
+            flamechartStackFrame: null,
             measure: null,
             data,
           });
@@ -240,7 +240,7 @@ function AutoSizedCanvas({data, height, width}: AutoSizedCanvasProps) {
         if (!hoveredEvent || hoveredEvent.measure !== measure) {
           setHoveredEvent({
             event: null,
-            flamechartNode: null,
+            flamechartStackFrame: null,
             measure,
             data,
           });
@@ -250,11 +250,14 @@ function AutoSizedCanvas({data, height, width}: AutoSizedCanvasProps) {
 
     const {current: flamegraphView} = flamegraphViewRef;
     if (flamegraphView) {
-      flamegraphView.onHover = flamechartNode => {
-        if (!hoveredEvent || hoveredEvent.flamechartNode !== flamechartNode) {
+      flamegraphView.onHover = flamechartStackFrame => {
+        if (
+          !hoveredEvent ||
+          hoveredEvent.flamechartStackFrame !== flamechartStackFrame
+        ) {
           setHoveredEvent({
             event: null,
-            flamechartNode,
+            flamechartStackFrame,
             measure: null,
             data,
           });
@@ -285,7 +288,7 @@ function AutoSizedCanvas({data, height, width}: AutoSizedCanvasProps) {
     const {current: flamegraphView} = flamegraphViewRef;
     if (flamegraphView) {
       flamegraphView.setHoveredFlamechartNode(
-        hoveredEvent ? hoveredEvent.flamechartNode : null,
+        hoveredEvent ? hoveredEvent.flamechartStackFrame : null,
       );
     }
   }, [
@@ -311,7 +314,11 @@ function AutoSizedCanvas({data, height, width}: AutoSizedCanvasProps) {
           if (contextData.hoveredEvent == null) {
             return null;
           }
-          const {event, flamechartNode, measure} = contextData.hoveredEvent;
+          const {
+            event,
+            flamechartStackFrame,
+            measure,
+          } = contextData.hoveredEvent;
           return (
             <Fragment>
               {event !== null && (
@@ -342,19 +349,20 @@ function AutoSizedCanvas({data, height, width}: AutoSizedCanvasProps) {
                   Copy summary
                 </ContextMenuItem>
               )}
-              {flamechartNode !== null && (
+              {flamechartStackFrame !== null && (
                 <ContextMenuItem
-                  onClick={() => copy(flamechartNode.node.frame.file)}
+                  onClick={() => copy(flamechartStackFrame.scriptUrl)}
                   title="Copy file path">
                   Copy file path
                 </ContextMenuItem>
               )}
-              {flamechartNode !== null && (
+              {flamechartStackFrame !== null && (
                 <ContextMenuItem
                   onClick={() =>
                     copy(
-                      `line ${flamechartNode.node.frame.line ||
-                        ''}, column ${flamechartNode.node.frame.col || ''}`,
+                      `line ${flamechartStackFrame.locationLine ||
+                        ''}, column ${flamechartStackFrame.locationColumn ||
+                        ''}`,
                     )
                   }
                   title="Copy location">

diff --git a/src/EventTooltip.js b/src/EventTooltip.js
@@ -1,8 +1,8 @@
 // @flow
 
 import type {Point} from './layout';
-import type {FlamechartFrame} from '@elg/speedscope';
 import type {
+  FlamechartStackFrame,
   ReactEvent,
   ReactMeasure,
   ReactProfilerData,
@@ -48,7 +48,7 @@ export default function EventTooltip({data, hoveredEvent, origin}: Props) {
     return null;
   }
 
-  const {event, flamechartNode, measure} = hoveredEvent;
+  const {event, flamechartStackFrame, measure} = hoveredEvent;
 
   if (event !== null) {
     switch (event.type) {
@@ -107,10 +107,10 @@ export default function EventTooltip({data, hoveredEvent, origin}: Props) {
         console.warn(`Unexpected measure type "${measure.type}"`);
         break;
     }
-  } else if (flamechartNode !== null) {
+  } else if (flamechartStackFrame !== null) {
     return (
       <TooltipFlamechartNode
-        flamechartNode={flamechartNode}
+        stackFrame={flamechartStackFrame}
         tooltipRef={tooltipRef}
       />
     );
@@ -129,14 +129,20 @@ function formatComponentStack(componentStack: string): string {
 }
 
 const TooltipFlamechartNode = ({
-  flamechartNode,
+  stackFrame,
   tooltipRef,
 }: {
-  flamechartNode: FlamechartFrame,
+  stackFrame: FlamechartStackFrame,
   tooltipRef: Return<typeof useRef>,
 }) => {
-  const {end, node, start} = flamechartNode;
-  const {col, file, line, name} = node.frame;
+  const {
+    name,
+    timestamp,
+    duration,
+    scriptUrl,
+    locationLine,
+    locationColumn,
+  } = stackFrame;
   return (
     <div
       className={styles.Tooltip}
@@ -145,21 +151,21 @@ const TooltipFlamechartNode = ({
         color: COLORS.TOOLTIP,
       }}
       ref={tooltipRef}>
-      {formatDuration((end - start) / 1000)} {trimComponentName(name)}
+      {formatDuration(duration)} {trimComponentName(name)}
       <div className={styles.DetailsGrid}>
         <div className={styles.DetailsGridLabel}>Timestamp:</div>
-        <div>{formatTimestamp(start / 1000)}</div>
-        {file && (
+        <div>{formatTimestamp(timestamp)}</div>
+        {scriptUrl && (
           <>
             <div className={styles.DetailsGridLabel}>Script URL:</div>
-            <div className={styles.DetailsGridURL}>{file}</div>
+            <div className={styles.DetailsGridURL}>{scriptUrl}</div>
           </>
         )}
-        {(line !== undefined || col !== undefined) && (
+        {(locationLine !== undefined || locationColumn !== undefined) && (
           <>
             <div className={styles.DetailsGridLabel}>Location:</div>
             <div>
-              line {line}, column {col}
+              line {locationLine}, column {locationColumn}
             </div>
           </>
         )}

diff --git a/src/canvas/views/FlamegraphView.js b/src/canvas/views/FlamegraphView.js
@@ -1,8 +1,11 @@
 // @flow
 
-import type {FlamechartFrame} from '@elg/speedscope';
 import type {Interaction, HoverInteraction} from '../../useCanvasInteraction';
-import type {FlamechartData, ReactProfilerData} from '../../types';
+import type {
+  Flamechart,
+  FlamechartStackFrame,
+  ReactProfilerData,
+} from '../../types';
 import type {Rect, Size} from '../../layout';
 
 import {
@@ -28,45 +31,45 @@ import {
 } from '../constants';
 
 export class FlamegraphView extends View {
-  flamechart: FlamechartData;
+  flamechart: Flamechart;
   profilerData: ReactProfilerData;
   intrinsicSize: Size;
 
-  hoveredFlamechartNode: FlamechartFrame | null = null;
-  onHover: ((node: FlamechartFrame | null) => void) | null = null;
+  hoveredStackFrame: FlamechartStackFrame | null = null;
+  onHover: ((node: FlamechartStackFrame | null) => void) | null = null;
 
   constructor(
     surface: Surface,
     frame: Rect,
-    flamechart: FlamechartData,
+    flamechart: Flamechart,
     profilerData: ReactProfilerData,
   ) {
     super(surface, frame);
     this.flamechart = flamechart;
     this.profilerData = profilerData;
     this.intrinsicSize = {
       width: this.profilerData.duration,
-      height: this.flamechart.getLayers().length * FLAMECHART_FRAME_HEIGHT,
+      height: this.flamechart.length * FLAMECHART_FRAME_HEIGHT,
     };
   }
 
   desiredSize() {
     return this.intrinsicSize;
   }
 
-  setHoveredFlamechartNode(hoveredFlamechartNode: FlamechartFrame | null) {
-    if (this.hoveredFlamechartNode === hoveredFlamechartNode) {
+  setHoveredFlamechartNode(hoveredStackFrame: FlamechartStackFrame | null) {
+    if (this.hoveredStackFrame === hoveredStackFrame) {
       return;
     }
-    this.hoveredFlamechartNode = hoveredFlamechartNode;
+    this.hoveredStackFrame = hoveredStackFrame;
     this.setNeedsDisplay();
   }
 
   draw(context: CanvasRenderingContext2D) {
     const {
       frame,
       flamechart,
-      hoveredFlamechartNode,
+      hoveredStackFrame: hoveredFlamechartNode,
       intrinsicSize,
       visibleArea,
     } = this;
@@ -85,8 +88,8 @@ export class FlamegraphView extends View {
 
     const scaleFactor = positioningScaleFactor(intrinsicSize.width, frame);
 
-    for (let i = 0; i < flamechart.getLayers().length; i++) {
-      const nodes = flamechart.getLayers()[i];
+    for (let i = 0; i < flamechart.length; i++) {
+      const stackLayer = flamechart[i];
 
       const layerY = Math.floor(frame.origin.y + i * FLAMECHART_FRAME_HEIGHT);
       if (
@@ -96,17 +99,16 @@ export class FlamegraphView extends View {
         continue; // Not in view
       }
 
-      for (let j = 0; j < nodes.length; j++) {
-        const {end, node, start} = nodes[j];
-        const {name} = node.frame;
+      for (let j = 0; j < stackLayer.length; j++) {
+        const {name, timestamp, duration} = stackLayer[j];
 
-        const width = durationToWidth((end - start) / 1000, scaleFactor);
+        const width = durationToWidth(duration, scaleFactor);
         if (width < 1) {
           continue; // Too small to render at this zoom level
         }
 
         const x = Math.floor(
-          timestampToPosition(start / 1000, scaleFactor, frame),
+          timestampToPosition(timestamp, scaleFactor, frame),
         );
         const nodeRect: Rect = {
           origin: {x, y: layerY},
@@ -121,7 +123,7 @@ export class FlamegraphView extends View {
           continue; // Not in view
         }
 
-        const showHoverHighlight = hoveredFlamechartNode === nodes[j];
+        const showHoverHighlight = hoveredFlamechartNode === stackLayer[j];
         context.fillStyle = showHoverHighlight
           ? COLORS.FLAME_GRAPH_HOVER
           : COLORS.FLAME_GRAPH;
@@ -195,11 +197,11 @@ export class FlamegraphView extends View {
     const layerIndex = Math.floor(
       adjustedCanvasMouseY / FLAMECHART_FRAME_HEIGHT,
     );
-    if (layerIndex < 0 || layerIndex >= flamechart.getLayers().length) {
+    if (layerIndex < 0 || layerIndex >= flamechart.length) {
       onHover(null);
       return;
     }
-    const layer = flamechart.getLayers()[layerIndex];
+    const layer = flamechart[layerIndex];
 
     if (!layer) {
       return null;
@@ -211,18 +213,13 @@ export class FlamegraphView extends View {
     let stopIndex = layer.length - 1;
     while (startIndex <= stopIndex) {
       const currentIndex = Math.floor((startIndex + stopIndex) / 2);
-      const flamechartNode = layer[currentIndex];
-
-      const {end, start} = flamechartNode;
-
-      const width = durationToWidth((end - start) / 1000, scaleFactor);
-
-      const x = Math.floor(
-        timestampToPosition(start / 1000, scaleFactor, frame),
-      );
+      const flamechartStackFrame = layer[currentIndex];
+      const {timestamp, duration} = flamechartStackFrame;
 
+      const width = durationToWidth(duration, scaleFactor);
+      const x = Math.floor(timestampToPosition(timestamp, scaleFactor, frame));
       if (x <= location.x && x + width >= location.x) {
-        onHover(flamechartNode);
+        onHover(flamechartStackFrame);
         return;
       }
 

diff --git a/src/types.js b/src/types.js
@@ -1,7 +1,5 @@
 // @flow
 
-import type {Flamechart, FlamechartFrame} from '@elg/speedscope';
-
 // Type utilities
 
 // Source: https://github.com/facebook/flow/issues/4002#issuecomment-323612798
@@ -87,19 +85,37 @@ export type ReactMeasure = {|
   +depth: number,
 |};
 
-export type FlamechartData = Flamechart;
+/**
+ * A flamechart stack frame belonging to a stack trace.
+ */
+export type FlamechartStackFrame = {|
+  name: string,
+  timestamp: Milliseconds,
+  duration: Milliseconds,
+  scriptUrl?: string,
+  locationLine?: number,
+  locationColumn?: number,
+|};
+
+/**
+ * A "layer" of stack frames in the profiler UI, i.e. all stack frames of the
+ * same depth across all stack traces. Displayed as a flamechart row in the UI.
+ */
+export type FlamechartStackLayer = FlamechartStackFrame[];
+
+export type Flamechart = FlamechartStackLayer[];
 
 export type ReactProfilerData = {|
   startTime: number,
   duration: number,
   events: ReactEvent[],
   measures: ReactMeasure[],
-  flamechart: FlamechartData,
+  flamechart: Flamechart,
 |};
 
 export type ReactHoverContextInfo = {|
   event: ReactEvent | null,
   measure: ReactMeasure | null,
   data: $ReadOnly<ReactProfilerData> | null,
-  flamechartNode: FlamechartFrame | null,
+  flamechartStackFrame: FlamechartStackFrame | null,
 |};