From 3002ebffb43dfccd369bdc91e89f04e4900032a2 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 30 Jul 2021 15:39:24 -0400 Subject: [PATCH] Show unresolved suspends as gray diamond marks --- .../src/CanvasPage.js | 8 +- .../src/EventTooltip.js | 15 +- .../src/content-views/SuspenseEventsView.js | 159 ++++++++++++------ .../src/content-views/UserTimingMarksView.js | 6 +- .../src/content-views/constants.js | 17 +- .../src/import-worker/preprocessData.js | 30 +++- .../src/types.js | 2 +- .../src/view-base/ResizableView.js | 5 - .../view-base/VerticalScrollOverflowView.js | 3 + .../src/view-base/layouter.js | 7 +- .../views/Settings/SettingsContext.js | 12 +- .../src/devtools/views/root.css | 8 +- 12 files changed, 166 insertions(+), 106 deletions(-) create mode 100644 packages/react-devtools-scheduling-profiler/src/view-base/VerticalScrollOverflowView.js diff --git a/packages/react-devtools-scheduling-profiler/src/CanvasPage.js b/packages/react-devtools-scheduling-profiler/src/CanvasPage.js index b08e01b9fe37b..9aa4b5230af23 100644 --- a/packages/react-devtools-scheduling-profiler/src/CanvasPage.js +++ b/packages/react-devtools-scheduling-profiler/src/CanvasPage.js @@ -31,6 +31,7 @@ import {copy} from 'clipboard-js'; import prettyMilliseconds from 'pretty-ms'; import { + ColorView, HorizontalPanAndZoomView, ResizableView, Surface, @@ -257,7 +258,7 @@ function AutoSizedCanvas({data, height, width}: AutoSizedCanvasProps) { data.duration, ); flamechartViewRef.current = flamechartView; - const flamechartViewWrapper = createViewHelper(flamechartView, true); + const flamechartViewWrapper = createViewHelper(flamechartView, true, true); // Root view contains all of the sub views defined above. // The order we add them below determines their vertical position. @@ -279,6 +280,11 @@ function AutoSizedCanvas({data, height, width}: AutoSizedCanvasProps) { rootView.addSubview(reactMeasuresViewWrapper); rootView.addSubview(flamechartViewWrapper); + // If subviews are less than the available height, fill remaining height with a solid color. + rootView.addSubview( + new ColorView(surface, defaultFrame, COLORS.BACKGROUND), + ); + surfaceRef.current.rootView = rootView; }, [data]); diff --git a/packages/react-devtools-scheduling-profiler/src/EventTooltip.js b/packages/react-devtools-scheduling-profiler/src/EventTooltip.js index 6af9a63733448..d6019cb254434 100644 --- a/packages/react-devtools-scheduling-profiler/src/EventTooltip.js +++ b/packages/react-devtools-scheduling-profiler/src/EventTooltip.js @@ -137,14 +137,7 @@ const TooltipFlamechartNode = ({ stackFrame: FlamechartStackFrame, tooltipRef: Return, }) => { - const { - name, - timestamp, - duration, - scriptUrl, - locationLine, - locationColumn, - } = stackFrame; + const {name, timestamp, duration, locationLine, locationColumn} = stackFrame; return (
@@ -154,12 +147,6 @@ const TooltipFlamechartNode = ({
{formatTimestamp(timestamp)}
Duration:
{formatDuration(duration)}
- {scriptUrl && ( - <> -
Script URL:
-
{scriptUrl}
- - )} {(locationLine !== undefined || locationColumn !== undefined) && ( <>
Location:
diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/SuspenseEventsView.js b/packages/react-devtools-scheduling-profiler/src/content-views/SuspenseEventsView.js index 21a7a1a916e5f..89fa602d4111a 100644 --- a/packages/react-devtools-scheduling-profiler/src/content-views/SuspenseEventsView.js +++ b/packages/react-devtools-scheduling-profiler/src/content-views/SuspenseEventsView.js @@ -21,6 +21,7 @@ import { positioningScaleFactor, positionToTimestamp, timestampToPosition, + widthToDuration, } from './utils/positioning'; import {drawText} from './utils/text'; import {formatDuration} from '../utils/formatting'; @@ -31,7 +32,12 @@ import { rectIntersectsRect, intersectionOfRects, } from '../view-base'; -import {COLORS, SUSPENSE_EVENT_HEIGHT, BORDER_SIZE} from './constants'; +import { + BORDER_SIZE, + COLORS, + PENDING_SUSPENSE_EVENT_SIZE, + SUSPENSE_EVENT_HEIGHT, +} from './constants'; const ROW_WITH_BORDER_HEIGHT = SUSPENSE_EVENT_HEIGHT + BORDER_SIZE; @@ -112,72 +118,106 @@ export class SuspenseEventsView extends View { baseY += depth * ROW_WITH_BORDER_HEIGHT; - const xStart = timestampToPosition(timestamp, scaleFactor, frame); - const xStop = timestampToPosition(timestamp + duration, scaleFactor, frame); - const eventRect: Rect = { - origin: { - x: xStart, - y: baseY, - }, - size: {width: xStop - xStart, height: SUSPENSE_EVENT_HEIGHT}, - }; - if (!rectIntersectsRect(eventRect, rect)) { - return; // Not in view - } - - if (duration === null) { - // TODO (scheduling profiler) We should probably draw a different representation for incomplete suspense measures. - // Maybe a dot? Maybe a gray measure? - return; // For now, don't show unresolved. - } - - const width = durationToWidth(duration, scaleFactor); - if (width < 1) { - return; // Too small to render at this zoom level - } - - const drawableRect = intersectionOfRects(eventRect, rect); - context.beginPath(); + let fillStyle = ((null: any): string); if (warning !== null) { - context.fillStyle = showHoverHighlight + fillStyle = showHoverHighlight ? COLORS.WARNING_BACKGROUND_HOVER : COLORS.WARNING_BACKGROUND; } else { switch (resolution) { - case 'pending': - context.fillStyle = showHoverHighlight - ? COLORS.REACT_SUSPENSE_PENDING_EVENT_HOVER - : COLORS.REACT_SUSPENSE_PENDING_EVENT; - break; case 'rejected': - context.fillStyle = showHoverHighlight + fillStyle = showHoverHighlight ? COLORS.REACT_SUSPENSE_REJECTED_EVENT_HOVER : COLORS.REACT_SUSPENSE_REJECTED_EVENT; break; case 'resolved': - context.fillStyle = showHoverHighlight + fillStyle = showHoverHighlight ? COLORS.REACT_SUSPENSE_RESOLVED_EVENT_HOVER : COLORS.REACT_SUSPENSE_RESOLVED_EVENT; break; + case 'unresolved': + fillStyle = showHoverHighlight + ? COLORS.REACT_SUSPENSE_UNRESOLVED_EVENT_HOVER + : COLORS.REACT_SUSPENSE_UNRESOLVED_EVENT; + break; } } - context.fillRect( - drawableRect.origin.x, - drawableRect.origin.y, - drawableRect.size.width, - drawableRect.size.height, - ); - let label = 'suspended'; - if (componentName != null) { - label = `${componentName} ${label}`; - } - if (phase !== null) { - label += ` during ${phase}`; - } - label += ` - ${formatDuration(duration)}`; + const xStart = timestampToPosition(timestamp, scaleFactor, frame); + + // Pending suspense events (ones that never resolved) won't have durations. + // So instead we draw them as diamonds. + if (duration === null) { + const size = PENDING_SUSPENSE_EVENT_SIZE; + const halfSize = size / 2; + + baseY += (SUSPENSE_EVENT_HEIGHT - PENDING_SUSPENSE_EVENT_SIZE) / 2; + + const y = baseY + halfSize; + + const suspenseRect: Rect = { + origin: { + x: xStart - halfSize, + y: baseY, + }, + size: {width: size, height: size}, + }; + if (!rectIntersectsRect(suspenseRect, rect)) { + return; // Not in view + } + + context.beginPath(); + context.fillStyle = fillStyle; + context.moveTo(xStart, y - halfSize); + context.lineTo(xStart + halfSize, y); + context.lineTo(xStart, y + halfSize); + context.lineTo(xStart - halfSize, y); + context.fill(); + } else { + const xStop = timestampToPosition( + timestamp + duration, + scaleFactor, + frame, + ); + const eventRect: Rect = { + origin: { + x: xStart, + y: baseY, + }, + size: {width: xStop - xStart, height: SUSPENSE_EVENT_HEIGHT}, + }; + if (!rectIntersectsRect(eventRect, rect)) { + return; // Not in view + } - drawText(label, context, eventRect, drawableRect, width); + const width = durationToWidth(duration, scaleFactor); + if (width < 1) { + return; // Too small to render at this zoom level + } + + const drawableRect = intersectionOfRects(eventRect, rect); + context.beginPath(); + context.fillStyle = fillStyle; + context.fillRect( + drawableRect.origin.x, + drawableRect.origin.y, + drawableRect.size.width, + drawableRect.size.height, + ); + + let label = 'suspended'; + if (componentName != null) { + label = `${componentName} ${label}`; + } + if (phase !== null) { + label += ` during ${phase}`; + } + if (resolution !== 'unresolved') { + label += ` - ${formatDuration(duration)}`; + } + + drawText(label, context, eventRect, drawableRect, width); + } } draw(context: CanvasRenderingContext2D) { @@ -269,7 +309,24 @@ export class SuspenseEventsView extends View { const suspenseEvent = suspenseEventsAtDepth[index]; const {duration, timestamp} = suspenseEvent; - if ( + if (duration === null) { + const timestampAllowance = widthToDuration( + PENDING_SUSPENSE_EVENT_SIZE / 2, + scaleFactor, + ); + + if ( + timestamp - timestampAllowance <= hoverTimestamp && + hoverTimestamp <= timestamp + timestampAllowance + ) { + this.currentCursor = 'pointer'; + + viewRefs.hoveredView = this; + + onHover(suspenseEvent); + return; + } + } else if ( hoverTimestamp >= timestamp && hoverTimestamp <= timestamp + duration ) { diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/UserTimingMarksView.js b/packages/react-devtools-scheduling-profiler/src/content-views/UserTimingMarksView.js index 92c82c119d2ef..a0640923ebc63 100644 --- a/packages/react-devtools-scheduling-profiler/src/content-views/UserTimingMarksView.js +++ b/packages/react-devtools-scheduling-profiler/src/content-views/UserTimingMarksView.js @@ -209,7 +209,7 @@ export class UserTimingMarksView extends View { frame, ); const hoverTimestamp = positionToTimestamp(location.x, scaleFactor, frame); - const markTimestampAllowance = widthToDuration( + const timestampAllowance = widthToDuration( USER_TIMING_MARK_SIZE / 2, scaleFactor, ); @@ -221,8 +221,8 @@ export class UserTimingMarksView extends View { const {timestamp} = mark; if ( - timestamp - markTimestampAllowance <= hoverTimestamp && - hoverTimestamp <= timestamp + markTimestampAllowance + timestamp - timestampAllowance <= hoverTimestamp && + hoverTimestamp <= timestamp + timestampAllowance ) { this.currentCursor = 'pointer'; viewRefs.hoveredView = this; diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/constants.js b/packages/react-devtools-scheduling-profiler/src/content-views/constants.js index c4e4ecafdd643..5a241c97742f4 100644 --- a/packages/react-devtools-scheduling-profiler/src/content-views/constants.js +++ b/packages/react-devtools-scheduling-profiler/src/content-views/constants.js @@ -16,6 +16,7 @@ export const COLOR_HOVER_DIM_DELTA = 5; export const TOP_ROW_PADDING = 4; export const NATIVE_EVENT_HEIGHT = 14; export const SUSPENSE_EVENT_HEIGHT = 14; +export const PENDING_SUSPENSE_EVENT_SIZE = 8; export const REACT_EVENT_DIAMETER = 6; export const USER_TIMING_MARK_SIZE = 8; export const REACT_MEASURE_HEIGHT = 9; @@ -65,12 +66,12 @@ export let COLORS = { REACT_RESIZE_BAR_DOT: '', REACT_SCHEDULE: '', REACT_SCHEDULE_HOVER: '', - REACT_SUSPENSE_PENDING_EVENT: '', - REACT_SUSPENSE_PENDING_EVENT_HOVER: '', REACT_SUSPENSE_REJECTED_EVENT: '', REACT_SUSPENSE_REJECTED_EVENT_HOVER: '', REACT_SUSPENSE_RESOLVED_EVENT: '', REACT_SUSPENSE_RESOLVED_EVENT_HOVER: '', + REACT_SUSPENSE_UNRESOLVED_EVENT: '', + REACT_SUSPENSE_UNRESOLVED_EVENT_HOVER: '', REACT_WORK_BORDER: '', TEXT_COLOR: '', TIME_MARKER_LABEL: '', @@ -150,12 +151,6 @@ export function updateColorsToMatchTheme(): void { REACT_SCHEDULE_HOVER: computedStyle.getPropertyValue( '--color-scheduling-profiler-react-schedule-hover', ), - REACT_SUSPENSE_PENDING_EVENT: computedStyle.getPropertyValue( - '--color-scheduling-profiler-react-suspense-pending', - ), - REACT_SUSPENSE_PENDING_EVENT_HOVER: computedStyle.getPropertyValue( - '--color-scheduling-profiler-react-suspense-pending-hover', - ), REACT_SUSPENSE_REJECTED_EVENT: computedStyle.getPropertyValue( '--color-scheduling-profiler-react-suspense-rejected', ), @@ -168,6 +163,12 @@ export function updateColorsToMatchTheme(): void { REACT_SUSPENSE_RESOLVED_EVENT_HOVER: computedStyle.getPropertyValue( '--color-scheduling-profiler-react-suspense-resolved-hover', ), + REACT_SUSPENSE_UNRESOLVED_EVENT: computedStyle.getPropertyValue( + '--color-scheduling-profiler-react-suspense-unresolved', + ), + REACT_SUSPENSE_UNRESOLVED_EVENT_HOVER: computedStyle.getPropertyValue( + '--color-scheduling-profiler-react-suspense-unresolved-hover', + ), REACT_WORK_BORDER: computedStyle.getPropertyValue( '--color-scheduling-profiler-react-work-border', ), diff --git a/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js b/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js index 1c591321ca098..a365be120897c 100644 --- a/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js +++ b/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js @@ -257,7 +257,7 @@ function processTimelineEvent( let warning = null; if (state.measureStack.find(({type}) => type === 'commit')) { - // TODO (scheduling profiler) Only warn if the subsequent updat is longer than some threshold. + // TODO (scheduling profiler) Only warn if the subsequent update is longer than some threshold. warning = WARNING_STRINGS.NESTED_UPDATE; } @@ -276,7 +276,7 @@ function processTimelineEvent( let warning = null; if (state.measureStack.find(({type}) => type === 'commit')) { - // TODO (scheduling profiler) Only warn if the subsequent updat is longer than some threshold. + // TODO (scheduling profiler) Only warn if the subsequent update is longer than some threshold. warning = WARNING_STRINGS.NESTED_UPDATE; } @@ -314,21 +314,33 @@ function processTimelineEvent( } } - let depth = 0; - state.unresolvedSuspenseEvents.forEach(unresolvedSuspenseEvent => { - depth = Math.max(depth, unresolvedSuspenseEvent.depth + 1); + const availableDepths = new Array( + state.unresolvedSuspenseEvents.size + 1, + ).fill(true); + state.unresolvedSuspenseEvents.forEach(({depth}) => { + availableDepths[depth] = false; }); - // TODO (scheduling profiler) - // Maybe default duration to be the end of the profiler data (for unresolved suspense?) - // Or should we just draw these are diamonds where they started instead? + let depth = 0; + for (let i = 0; i < availableDepths.length; i++) { + if (availableDepths[i]) { + depth = i; + break; + } + } + + // TODO (scheduling profiler) Maybe we should calculate depth in post, + // so unresolved Suspense requests don't take up space. + // We can't know if they'll be resolved or not at this point. + // We'll just give them a default (fake) duration width. + const suspenseEvent = { componentName, depth, duration: null, id, phase, - resolution: 'pending', + resolution: 'unresolved', resuspendTimestamps: null, timestamp: startTime, type: 'suspense', diff --git a/packages/react-devtools-scheduling-profiler/src/types.js b/packages/react-devtools-scheduling-profiler/src/types.js index d340afd64f065..674dabdc8e93a 100644 --- a/packages/react-devtools-scheduling-profiler/src/types.js +++ b/packages/react-devtools-scheduling-profiler/src/types.js @@ -59,7 +59,7 @@ export type SuspenseEvent = {| duration: number | null, +id: string, +phase: 'mount' | 'update' | null, - resolution: 'pending' | 'resolved' | 'rejected', + resolution: 'rejected' | 'resolved' | 'unresolved', resuspendTimestamps: Array | null, +type: 'suspense', |}; diff --git a/packages/react-devtools-scheduling-profiler/src/view-base/ResizableView.js b/packages/react-devtools-scheduling-profiler/src/view-base/ResizableView.js index 4e841db688a88..18cc70c745897 100644 --- a/packages/react-devtools-scheduling-profiler/src/view-base/ResizableView.js +++ b/packages/react-devtools-scheduling-profiler/src/view-base/ResizableView.js @@ -42,7 +42,6 @@ const RESIZE_BAR_SIZE = 8; const RESIZE_BAR_DOT_RADIUS = 1; const RESIZE_BAR_DOT_SPACING = 4; -// TODO (ResizableView) Draw borders on top and bottom in case two bars are collapsed next to each other. class ResizeBar extends View { _intrinsicContentSize: Size = { width: 0, @@ -222,12 +221,9 @@ export class ResizableView extends View { super.layoutSubviews(); } - // TODO (ResizableView) Change ResizeBar view style slightly when fully collapsed. - // TODO (ResizableView) Double click on ResizeBar to collapse/toggle. _updateLayoutState() { const {frame, _resizingState} = this; - // TODO (ResizableView) Allow subviews to specify min size too. // Allow bar to travel to bottom of the visible area of this view but no further const subviewDesiredSize = this._subview.desiredSize(); const maxBarOffset = subviewDesiredSize.height; @@ -278,7 +274,6 @@ export class ResizableView extends View { ); if (cursorInView) { if (this._layoutState.barOffsetY === 0) { - // TODO (ResizableView) Allow subviews to specify min size too. // Allow bar to travel to bottom of the visible area of this view but no further const subviewDesiredSize = this._subview.desiredSize(); const maxBarOffset = subviewDesiredSize.height; diff --git a/packages/react-devtools-scheduling-profiler/src/view-base/VerticalScrollOverflowView.js b/packages/react-devtools-scheduling-profiler/src/view-base/VerticalScrollOverflowView.js new file mode 100644 index 0000000000000..7ddf6c1842691 --- /dev/null +++ b/packages/react-devtools-scheduling-profiler/src/view-base/VerticalScrollOverflowView.js @@ -0,0 +1,3 @@ +// TODO Vertically stack views (via verticallyStackedLayout). +// If stacked views are taller than the available height, a vertical scrollbar will be shown on the side, +// and width will be adjusted to subtract the width of the scrollbar. diff --git a/packages/react-devtools-scheduling-profiler/src/view-base/layouter.js b/packages/react-devtools-scheduling-profiler/src/view-base/layouter.js index ca5ba1ebfdf47..2cad9659be066 100644 --- a/packages/react-devtools-scheduling-profiler/src/view-base/layouter.js +++ b/packages/react-devtools-scheduling-profiler/src/view-base/layouter.js @@ -43,8 +43,7 @@ export function collapseLayoutIntoViews(layout: Layout) { export const noopLayout: Layouter = layout => layout; /** - * Layer views on top of each other. All views' frames will be set to - * `containerFrame`. + * Layer views on top of each other. All views' frames will be set to `containerFrame`. * * Equivalent to composing: * - `alignToContainerXLayout`, @@ -56,8 +55,8 @@ export const layeredLayout: Layouter = (layout, containerFrame) => layout.map(layoutInfo => ({...layoutInfo, frame: containerFrame})); /** - * Stacks `views` vertically in `frame`. All views in `views` will have their - * widths set to the frame's width. + * Stacks `views` vertically in `frame`. + * All views in `views` will have their widths set to the frame's width. */ export const verticallyStackedLayout: Layouter = (layout, containerFrame) => { let currentY = containerFrame.origin.y; diff --git a/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js b/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js index 690b149f6239c..29b1b12be0289 100644 --- a/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js @@ -526,32 +526,32 @@ export function updateThemeVariables( ); updateStyleHelper( theme, - 'color-scheduling-profiler-react-suspense-pending', + 'color-scheduling-profiler-react-suspense-rejected-event', documentElements, ); updateStyleHelper( theme, - 'color-scheduling-profiler-react-suspense-pending-hover', + 'color-scheduling-profiler-react-suspense-rejected-hover', documentElements, ); updateStyleHelper( theme, - 'color-scheduling-profiler-react-suspense-rejected-event', + 'color-scheduling-profiler-react-suspense-resolved', documentElements, ); updateStyleHelper( theme, - 'color-scheduling-profiler-react-suspense-rejected-hover', + 'color-scheduling-profiler-react-suspense-resolved-hover', documentElements, ); updateStyleHelper( theme, - 'color-scheduling-profiler-react-suspense-resolved', + 'color-scheduling-profiler-react-suspense-unresolved', documentElements, ); updateStyleHelper( theme, - 'color-scheduling-profiler-react-suspense-resolved-hover', + 'color-scheduling-profiler-react-suspense-unresolved-hover', documentElements, ); updateStyleHelper( diff --git a/packages/react-devtools-shared/src/devtools/views/root.css b/packages/react-devtools-shared/src/devtools/views/root.css index bf29c87886397..10b68fb01e901 100644 --- a/packages/react-devtools-shared/src/devtools/views/root.css +++ b/packages/react-devtools-shared/src/devtools/views/root.css @@ -99,12 +99,12 @@ --light-color-scheduling-profiler-react-passive-effects-hover: #e7c20a; --light-color-scheduling-profiler-react-schedule: #9fc3f3; --light-color-scheduling-profiler-react-schedule-hover: #2683E2; - --light-color-scheduling-profiler-react-suspense-pending: #c9cacd; - --light-color-scheduling-profiler-react-suspense-pending-hover: #93959a; --light-color-scheduling-profiler-react-suspense-rejected: #f1cc14; --light-color-scheduling-profiler-react-suspense-rejected-hover: #ffdf37; --light-color-scheduling-profiler-react-suspense-resolved: #a6e59f; --light-color-scheduling-profiler-react-suspense-resolved-hover: #89d281; + --light-color-scheduling-profiler-react-suspense-unresolved: #c9cacd; + --light-color-scheduling-profiler-react-suspense-unresolved-hover: #93959a; --light-color-scheduling-profiler-text-color: #000000; --light-color-scheduling-profiler-react-work-border: #ffffff; --light-color-scroll-thumb: #c2c2c2; @@ -225,12 +225,12 @@ --dark-color-scheduling-profiler-react-passive-effects-hover: #e4c00f; --dark-color-scheduling-profiler-react-schedule: #2683E2; --dark-color-scheduling-profiler-react-schedule-hover: #1a76d4; - --dark-color-scheduling-profiler-react-suspense-pending: #c9cacd; - --dark-color-scheduling-profiler-react-suspense-pending-hover: #93959a; --dark-color-scheduling-profiler-react-suspense-rejected: #f1cc14; --dark-color-scheduling-profiler-react-suspense-rejected-hover: #e4c00f; --dark-color-scheduling-profiler-react-suspense-resolved: #a6e59f; --dark-color-scheduling-profiler-react-suspense-resolved-hover: #89d281; + --dark-color-scheduling-profiler-react-suspense-unresolved: #c9cacd; + --dark-color-scheduling-profiler-react-suspense-unresolved-hover: #93959a; --dark-color-scheduling-profiler-text-color: #000000; --dark-color-scheduling-profiler-react-work-border: #ffffff; --dark-color-scroll-thumb: #afb3b9;