Skip to content

Commit

Permalink
Show unresolved suspends as gray diamond marks
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn committed Jul 30, 2021
1 parent 8c87704 commit 3002ebf
Show file tree
Hide file tree
Showing 12 changed files with 166 additions and 106 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {copy} from 'clipboard-js';
import prettyMilliseconds from 'pretty-ms';

import {
ColorView,
HorizontalPanAndZoomView,
ResizableView,
Surface,
Expand Down Expand Up @@ -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.
Expand All @@ -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]);

Expand Down
15 changes: 1 addition & 14 deletions packages/react-devtools-scheduling-profiler/src/EventTooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,7 @@ const TooltipFlamechartNode = ({
stackFrame: FlamechartStackFrame,
tooltipRef: Return<typeof useRef>,
}) => {
const {
name,
timestamp,
duration,
scriptUrl,
locationLine,
locationColumn,
} = stackFrame;
const {name, timestamp, duration, locationLine, locationColumn} = stackFrame;
return (
<div className={styles.Tooltip} ref={tooltipRef}>
<div className={styles.TooltipSection}>
Expand All @@ -154,12 +147,6 @@ const TooltipFlamechartNode = ({
<div>{formatTimestamp(timestamp)}</div>
<div className={styles.DetailsGridLabel}>Duration:</div>
<div>{formatDuration(duration)}</div>
{scriptUrl && (
<>
<div className={styles.DetailsGridLabel}>Script URL:</div>
<div className={styles.DetailsGridURL}>{scriptUrl}</div>
</>
)}
{(locationLine !== undefined || locationColumn !== undefined) && (
<>
<div className={styles.DetailsGridLabel}>Location:</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
positioningScaleFactor,
positionToTimestamp,
timestampToPosition,
widthToDuration,
} from './utils/positioning';
import {drawText} from './utils/text';
import {formatDuration} from '../utils/formatting';
Expand All @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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: '',
Expand Down Expand Up @@ -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',
),
Expand All @@ -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',
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-scheduling-profiler/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<number> | null,
+type: 'suspense',
|};
Expand Down
Loading

0 comments on commit 3002ebf

Please sign in to comment.