From b9f310cecb99711dd9814195ce6cf521e0f95154 Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Tue, 28 Jul 2020 13:23:41 +0800 Subject: [PATCH] [Optimize flamechart][1/4] Add flamechart to ReactProfilerData This commit begins a stack of PRs that optimizes our flamechart. Broadly, we need to replace the use of Speedscope's flamechart types with our own types that are optimized for our rendering code. As flamechart was always moved around together with ReactProfilerData, it makes sense to move flamechart into it. That also necessitates moving preprocessFlamechart into preprocessData. This PR works towards #50. * `yarn flow`: errors present, but no errors in affected code. * `yarn lint` * `yarn start` --- src/App.js | 23 +++++------------------ src/CanvasPage.js | 30 ++++++------------------------ src/ImportPage.js | 17 +++-------------- src/types.js | 5 +++-- src/util/preprocessData.js | 22 ++++++++++++++++++++-- src/util/preprocessFlamechart.js | 21 --------------------- 6 files changed, 37 insertions(+), 81 deletions(-) delete mode 100644 src/util/preprocessFlamechart.js diff --git a/src/App.js b/src/App.js index 00c9172..8de8b1a 100644 --- a/src/App.js +++ b/src/App.js @@ -1,9 +1,8 @@ // @flow -import type {FlamechartData, ReactProfilerData} from './types'; +import type {ReactProfilerData} from './types'; -import React, {useState, useCallback} from 'react'; -import {unstable_batchedUpdates} from 'react-dom'; +import React, {useState} from 'react'; import ImportPage from './ImportPage'; import CanvasPage from './CanvasPage'; @@ -12,22 +11,10 @@ export default function App() { const [profilerData, setProfilerData] = useState( null, ); - const [flamechart, setFlamechart] = useState(null); - const handleDataImported = useCallback( - ( - importedProfilerData: ReactProfilerData, - importedFlamechart: FlamechartData, - ) => { - unstable_batchedUpdates(() => { - setProfilerData(importedProfilerData); - setFlamechart(importedFlamechart); - }); - }, - ); - if (profilerData && flamechart) { - return ; + if (profilerData) { + return ; } else { - return ; + return ; } } diff --git a/src/CanvasPage.js b/src/CanvasPage.js index c89719f..7767e82 100644 --- a/src/CanvasPage.js +++ b/src/CanvasPage.js @@ -33,11 +33,7 @@ import {ContextMenu, ContextMenuItem, useContextMenu} from './context'; const CONTEXT_MENU_ID = 'canvas'; -import type { - FlamechartData, - ReactHoverContextInfo, - ReactProfilerData, -} from './types'; +import type {ReactHoverContextInfo, ReactProfilerData} from './types'; import {useCanvasInteraction} from './useCanvasInteraction'; import { FlamegraphView, @@ -48,28 +44,21 @@ import { type ContextMenuContextData = {| data: ReactProfilerData, - flamechart: FlamechartData, hoveredEvent: ReactHoverContextInfo | null, |}; type Props = {| profilerData: ReactProfilerData, - flamechart: FlamechartData, |}; -function CanvasPage({profilerData, flamechart}: Props) { +function CanvasPage({profilerData}: Props) { return (
{({height, width}: {height: number, width: number}) => ( - + )}
@@ -104,17 +93,11 @@ const copySummary = (data, measure) => { type AutoSizedCanvasProps = {| data: ReactProfilerData, - flamechart: FlamechartData, height: number, width: number, |}; -function AutoSizedCanvas({ - data, - flamechart, - height, - width, -}: AutoSizedCanvasProps) { +function AutoSizedCanvas({data, height, width}: AutoSizedCanvasProps) { const canvasRef = useRef(null); const [isContextMenuShown, setIsContextMenuShown] = useState(false); @@ -156,7 +139,7 @@ function AutoSizedCanvas({ const flamegraphView = new FlamegraphView( surfaceRef.current, {origin: zeroPoint, size: {width, height}}, - flamechart, + data.flamechart, data, ); flamegraphViewRef.current = flamegraphView; @@ -194,7 +177,7 @@ function AutoSizedCanvas({ ); surfaceRef.current.rootView = rootViewRef.current; - }, [data, flamechart, setHoveredEvent]); + }, [data, setHoveredEvent]); useLayoutEffect(() => { if (canvasRef.current) { @@ -229,7 +212,6 @@ function AutoSizedCanvas({ useContextMenu({ data: { data, - flamechart, hoveredEvent, }, id: CONTEXT_MENU_ID, diff --git a/src/ImportPage.js b/src/ImportPage.js index e67e427..825311a 100644 --- a/src/ImportPage.js +++ b/src/ImportPage.js @@ -1,38 +1,27 @@ // @flow import type {TimelineEvent} from '@elg/speedscope'; -import type {FlamechartData, ReactProfilerData} from './types'; +import type {ReactProfilerData} from './types'; import React, {useEffect, useCallback, useRef} from 'react'; import profilerBrowser from './assets/profilerBrowser.png'; import style from './ImportPage.css'; import preprocessData from './util/preprocessData'; -import preprocessFlamechart from './util/preprocessFlamechart'; import {readInputData} from './util/readInputData'; // TODO: Use for dev only, switch to import file after import JSON_PATH from 'url:../static/perfprofilev2.json'; type Props = {| - onDataImported: ( - profilerData: ReactProfilerData, - flamechart: FlamechartData, - ) => void, + onDataImported: (profilerData: ReactProfilerData) => void, |}; export default function ImportPage({onDataImported}: Props) { const processTimeline = useCallback( (events: TimelineEvent[]) => { - // Filter null entries and sort by timestamp. - // I would not expect to have to do either of this, - // but some of the data being passed in requires it. - events = events.filter(Boolean).sort((a, b) => (a.ts > b.ts ? 1 : -1)); - if (events.length > 0) { - const processedData = preprocessData(events); - const processedFlamechart = preprocessFlamechart(events); - onDataImported(processedData, processedFlamechart); + onDataImported(preprocessData(events)); } }, [onDataImported], diff --git a/src/types.js b/src/types.js index 4a52abe..5d01111 100644 --- a/src/types.js +++ b/src/types.js @@ -87,11 +87,14 @@ export type ReactMeasure = {| +depth: number, |}; +export type FlamechartData = Flamechart; + export type ReactProfilerData = {| startTime: number, duration: number, events: ReactEvent[], measures: ReactMeasure[], + flamechart: FlamechartData, |}; export type ReactHoverContextInfo = {| @@ -100,5 +103,3 @@ export type ReactHoverContextInfo = {| data: $ReadOnly | null, flamechartNode: FlamechartFrame | null, |}; - -export type FlamechartData = Flamechart; diff --git a/src/util/preprocessData.js b/src/util/preprocessData.js index 0c28c1d..fd2119d 100644 --- a/src/util/preprocessData.js +++ b/src/util/preprocessData.js @@ -1,9 +1,11 @@ // @flow +import {importFromChromeTimeline, Flamechart} from '@elg/speedscope'; import type {TimelineEvent} from '@elg/speedscope'; import type { Milliseconds, BatchUID, + FlamechartData, ReactLane, ReactMeasureType, ReactProfilerData, @@ -345,18 +347,34 @@ function processTimelineEvent( } } +function preprocessFlamechart(rawData: TimelineEvent[]): FlamechartData { + const parsedData = importFromChromeTimeline(rawData, 'react-devtools'); + const profile = parsedData.profiles[0]; // TODO: Choose the main CPU thread only + const flamechart = new Flamechart({ + getTotalWeight: profile.getTotalWeight.bind(profile), + forEachCall: profile.forEachCall.bind(profile), + formatValue: profile.formatValue.bind(profile), + getColorBucketForFrame: () => 0, + }); + return flamechart; +} + export default function preprocessData( timeline: TimelineEvent[], ): ReactProfilerData { - const profilerData = { + const flamechart = preprocessFlamechart(timeline); + + const profilerData: ReactProfilerData = { startTime: 0, duration: 0, events: [], measures: [], + flamechart, }; - // TODO: Sort `timeline`. JSON Array Format trace events need not be ordered. See: + // Sort `timeline`. JSON Array Format trace events need not be ordered. See: // https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.f2f0yd51wi15 + timeline = timeline.filter(Boolean).sort((a, b) => (a.ts > b.ts ? 1 : -1)); // Events displayed in flamechart have timestamps relative to the profile // event's startTime. Source: https://github.com/v8/v8/blob/44bd8fd7/src/inspector/js_protocol.json#L1486 diff --git a/src/util/preprocessFlamechart.js b/src/util/preprocessFlamechart.js deleted file mode 100644 index dc99185..0000000 --- a/src/util/preprocessFlamechart.js +++ /dev/null @@ -1,21 +0,0 @@ -// @flow - -import {importFromChromeTimeline, Flamechart} from '@elg/speedscope'; - -import type {TimelineEvent} from '@elg/speedscope'; -import type {FlamechartData} from '../types'; - -export default function preprocessFlamechart( - rawData: TimelineEvent[], -): FlamechartData { - const parsedData = importFromChromeTimeline(rawData, 'react-devtools'); - const profile = parsedData.profiles[0]; // TODO Choose the main CPU thread only - const flamechart = new Flamechart({ - getTotalWeight: profile.getTotalWeight.bind(profile), - forEachCall: profile.forEachCall.bind(profile), - formatValue: profile.formatValue.bind(profile), - getColorBucketForFrame: () => null, - }); - - return flamechart; -}