Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Flight] Emit Deduped Server Components Marker #31737

Merged
merged 1 commit into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions fixtures/flight/src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ function Foo({children}) {
return <div>{children}</div>;
}

function Bar({children}) {
async function Bar({children}) {
await new Promise(resolve => setTimeout(() => resolve('deferred text'), 10));
return <div>{children}</div>;
}

Expand Down Expand Up @@ -81,7 +82,7 @@ export default async function App({prerender}) {
<Client />
<Note />
<Foo>{dedupedChild}</Foo>
<Bar>{dedupedChild}</Bar>
<Bar>{Promise.resolve([dedupedChild])}</Bar>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add this hack because there's an unrelated bug that cause debug info to shift order when the props are delayed.

// TODO: This is not going to resolve in the right order if there's more than one.

</Container>
</body>
</html>
Expand Down
45 changes: 41 additions & 4 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ import {readTemporaryReference} from './ReactFlightTemporaryReferences';
import {
markAllTracksInOrder,
logComponentRender,
logDedupedComponentRender,
} from './ReactFlightPerformanceTrack';

import {
Expand Down Expand Up @@ -130,6 +131,7 @@ export type JSONValue =
type ProfilingResult = {
track: number,
endTime: number,
component: null | ReactComponentInfo,
};

const ROW_ID = 0;
Expand Down Expand Up @@ -647,7 +649,7 @@ export function reportGlobalError(response: Response, error: Error): void {
});
if (enableProfilerTimer && enableComponentPerformanceTrack) {
markAllTracksInOrder();
flushComponentPerformance(getChunk(response, 0), 0, -Infinity);
flushComponentPerformance(getChunk(response, 0), 0, -Infinity, -Infinity);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
flushComponentPerformance(getChunk(response, 0), 0, -Infinity, -Infinity);
flushComponentPerformance(getRoot(), 0, -Infinity, -Infinity);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally had that but it's not quite right because getRoot() is a public API and as such might end up with more stuff later on (like type checks of the arguments or something). However, it also means that the return type is constrained to a Thenable since that's what the public API is, but here I need it to be the chunk API.

Another consideration is that getRoot() being used in two places could also prevent it from being inlined in the wrapper unnecessarily.

}
}

Expand Down Expand Up @@ -2748,7 +2750,8 @@ function resolveTypedArray(
function flushComponentPerformance(
root: SomeChunk<any>,
trackIdx: number, // Next available track
trackTime: number, // The time after which it is available
trackTime: number, // The time after which it is available,
parentEndTime: number,
): ProfilingResult {
if (!enableProfilerTimer || !enableComponentPerformanceTrack) {
// eslint-disable-next-line react-internal/prod-error-codes
Expand All @@ -2765,6 +2768,22 @@ function flushComponentPerformance(
// chunk in two places. We should extend the current end time as if it was
// rendered as part of this tree.
const previousResult: ProfilingResult = root._children;
const previousEndTime = previousResult.endTime;
if (
parentEndTime > -Infinity &&
parentEndTime < previousEndTime &&
previousResult.component !== null
) {
// Log a placeholder for the deduped value under this child starting
// from the end of the self time of the parent and spanning until the
// the deduped end.
logDedupedComponentRender(
previousResult.component,
trackIdx,
parentEndTime,
previousEndTime,
);
}
// Since we didn't bump the track this time, we just return the same track.
previousResult.track = trackIdx;
return previousResult;
Expand Down Expand Up @@ -2792,15 +2811,27 @@ function flushComponentPerformance(
// The start time of this component is before the end time of the previous
// component on this track so we need to bump the next one to a parallel track.
trackIdx++;
trackTime = startTime;
}
trackTime = startTime;
break;
}
}
}
for (let i = debugInfo.length - 1; i >= 0; i--) {
const info = debugInfo[i];
if (typeof info.time === 'number') {
if (info.time > parentEndTime) {
parentEndTime = info.time;
}
}
}
}

const result: ProfilingResult = {track: trackIdx, endTime: -Infinity};
const result: ProfilingResult = {
track: trackIdx,
endTime: -Infinity,
component: null,
};
root._children = result;
let childrenEndTime = -Infinity;
let childTrackIdx = trackIdx;
Expand All @@ -2810,7 +2841,11 @@ function flushComponentPerformance(
children[i],
childTrackIdx,
childTrackTime,
parentEndTime,
);
if (childResult.component !== null) {
result.component = childResult.component;
}
childTrackIdx = childResult.track;
const childEndTime = childResult.endTime;
childTrackTime = childEndTime;
Expand Down Expand Up @@ -2842,6 +2877,8 @@ function flushComponentPerformance(
endTime,
childrenEndTime,
);
// Track the root most component of the result for deduping logging.
result.component = componentInfo;
}
}
}
Expand Down
16 changes: 16 additions & 0 deletions packages/react-client/src/ReactFlightPerformanceTrack.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,19 @@ export function logComponentRender(
performance.measure(name, reusableComponentOptions);
}
}

export function logDedupedComponentRender(
componentInfo: ReactComponentInfo,
trackIdx: number,
startTime: number,
endTime: number,
): void {
if (supportsUserTiming && endTime >= 0 && trackIdx < 10) {
const name = componentInfo.name;
reusableComponentDevToolDetails.color = 'tertiary-light';
reusableComponentDevToolDetails.track = trackNames[trackIdx];
reusableComponentOptions.start = startTime < 0 ? 0 : startTime;
reusableComponentOptions.end = endTime;
performance.measure(name + ' [deduped]', reusableComponentOptions);
}
}
Loading