-
Notifications
You must be signed in to change notification settings - Fork 49.6k
[Fiber] Don't wait on Suspensey Images if we guess that we don't load them all in time anyway #34481
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
[Fiber] Don't wait on Suspensey Images if we guess that we don't load them all in time anyway #34481
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -143,6 +143,7 @@ import ReactDOMSharedInternals from 'shared/ReactDOMSharedInternals'; | |
export {default as rendererVersion} from 'shared/ReactVersion'; | ||
|
||
import noop from 'shared/noop'; | ||
import estimateBandwidth from './estimateBandwidth'; | ||
|
||
export const rendererPackageName = 'react-dom'; | ||
export const extraDevToolsConfig = null; | ||
|
@@ -5907,6 +5908,7 @@ type SuspendedState = { | |
stylesheets: null | Map<StylesheetResource, HoistableRoot>, | ||
count: number, // suspensey css and active view transitions | ||
imgCount: number, // suspensey images | ||
imgBytes: number, // number of bytes we estimate needing to download | ||
waitingForImages: boolean, // false when we're no longer blocking on images | ||
unsuspend: null | (() => void), | ||
}; | ||
|
@@ -5917,6 +5919,7 @@ export function startSuspendingCommit(): void { | |
stylesheets: null, | ||
count: 0, | ||
imgCount: 0, | ||
imgBytes: 0, | ||
waitingForImages: true, | ||
// We use a noop function when we begin suspending because if possible we want the | ||
// waitfor step to finish synchronously. If it doesn't we'll return a function to | ||
|
@@ -5926,10 +5929,6 @@ export function startSuspendingCommit(): void { | |
}; | ||
} | ||
|
||
const SUSPENSEY_STYLESHEET_TIMEOUT = 60000; | ||
|
||
const SUSPENSEY_IMAGE_TIMEOUT = 500; | ||
|
||
export function suspendInstance( | ||
instance: Instance, | ||
type: Type, | ||
|
@@ -5953,6 +5952,18 @@ export function suspendInstance( | |
// The loading should have already started at this point, so it should be enough to | ||
// just call decode() which should also wait for the data to finish loading. | ||
state.imgCount++; | ||
// Estimate the byte size that we're about to download based on the width/height | ||
// specified in the props. This is best practice to know ahead of time but if it's | ||
// unspecified we'll fallback to a guess of 100x100 pixels. | ||
if (!(instance: any).complete) { | ||
const width: number = (instance: any).width || 100; | ||
const height: number = (instance: any).height || 100; | ||
const pixelRatio: number = | ||
typeof devicePixelRatio === 'number' ? devicePixelRatio : 1; | ||
const pixelsToDownload = width * height * pixelRatio; | ||
const AVERAGE_BYTE_PER_PIXEL = 0.25; | ||
state.imgBytes += pixelsToDownload * AVERAGE_BYTE_PER_PIXEL; | ||
} | ||
const ping = onUnsuspendImg.bind(state); | ||
// $FlowFixMe[prop-missing] | ||
instance.decode().then(ping, ping); | ||
|
@@ -6070,6 +6081,14 @@ export function suspendOnActiveViewTransition(rootContainer: Container): void { | |
activeViewTransition.finished.then(ping, ping); | ||
} | ||
|
||
const SUSPENSEY_STYLESHEET_TIMEOUT = 60000; | ||
|
||
const SUSPENSEY_IMAGE_TIMEOUT = 800; | ||
|
||
const SUSPENSEY_IMAGE_TIME_ESTIMATE = 500; | ||
|
||
let estimatedBytesWithinLimit: number = 0; | ||
|
||
export function waitForCommitToBeReady( | ||
timeoutOffset: number, | ||
): null | ((() => void) => () => void) { | ||
|
@@ -6109,6 +6128,18 @@ export function waitForCommitToBeReady( | |
} | ||
}, SUSPENSEY_STYLESHEET_TIMEOUT + timeoutOffset); | ||
|
||
if (state.imgBytes > 0 && estimatedBytesWithinLimit === 0) { | ||
// Estimate how many bytes we can download in 500ms. | ||
const mbps = estimateBandwidth(); | ||
estimatedBytesWithinLimit = mbps * 125 * SUSPENSEY_IMAGE_TIME_ESTIMATE; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why a 500ms constant vs the remaining time: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because we have already started the download before we get here. The download could've started before the render even started or it could've started right before commit if the image is the last sibling we discovered and it didn't start downloading earlier. So if we say that it started sometime within the render, then the start time of when we started is really the same thing. Basically the question isn't "If we started downloading now will we have time to download this image?", it's more like "Will we have time to download the images within the span we're willing to have waited in total?" |
||
} | ||
// If we have more images to download than we expect to fit in the timeout, then | ||
// don't wait for images longer than 50ms. The 50ms lets us still do decoding and | ||
// hitting caches if it turns out that they're already in the HTTP cache. | ||
const imgTimeout = | ||
state.imgBytes > estimatedBytesWithinLimit | ||
? 50 | ||
: SUSPENSEY_IMAGE_TIMEOUT; | ||
const imgTimer = setTimeout(() => { | ||
// We're no longer blocked on images. If CSS resolves after this we can commit. | ||
state.waitingForImages = false; | ||
|
@@ -6122,7 +6153,7 @@ export function waitForCommitToBeReady( | |
unsuspend(); | ||
} | ||
} | ||
}, SUSPENSEY_IMAGE_TIMEOUT + timeoutOffset); | ||
}, imgTimeout + timeoutOffset); | ||
|
||
state.unsuspend = commit; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
/** | ||
* Copyright (c) Meta Platforms, Inc. and affiliates. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
* | ||
* @flow | ||
*/ | ||
|
||
function isLikelyStaticResource(initiatorType: string) { | ||
switch (initiatorType) { | ||
case 'css': | ||
case 'script': | ||
case 'font': | ||
case 'img': | ||
case 'image': | ||
case 'input': | ||
case 'link': | ||
return true; | ||
default: | ||
return false; | ||
} | ||
} | ||
|
||
export default function estimateBandwidth(): number { | ||
// Estimate the current bandwidth for downloading static resources given resources already | ||
// loaded. | ||
// $FlowFixMe[method-unbinding] | ||
if (typeof performance.getEntriesByType === 'function') { | ||
let count = 0; | ||
let bits = 0; | ||
const resourceEntries = performance.getEntriesByType('resource'); | ||
for (let i = 0; i < resourceEntries.length; i++) { | ||
const entry = resourceEntries[i]; | ||
// $FlowFixMe[prop-missing] | ||
const transferSize: number = entry.transferSize; | ||
// $FlowFixMe[prop-missing] | ||
const initiatorType: string = entry.initiatorType; | ||
const duration = entry.duration; | ||
if ( | ||
!transferSize || | ||
!duration || | ||
!isLikelyStaticResource(initiatorType) | ||
) { | ||
// Skip cached, cross-orgin entries and resources likely to be dynamically generated. | ||
continue; | ||
} | ||
// Find any overlapping entries that were transferring at the same time since the total | ||
// bps at the time will include those bytes. | ||
let overlappingBytes = 0; | ||
// $FlowFixMe[prop-missing] | ||
const parentEndTime: number = entry.responseEnd; | ||
let j; | ||
for (j = i + 1; j < resourceEntries.length; j++) { | ||
const overlapEntry = resourceEntries[j]; | ||
const overlapStartTime = overlapEntry.startTime; | ||
if (overlapStartTime > parentEndTime) { | ||
break; | ||
} | ||
// $FlowFixMe[prop-missing] | ||
const overlapTransferSize: number = overlapEntry.transferSize; | ||
// $FlowFixMe[prop-missing] | ||
const overlapInitiatorType: string = overlapEntry.initiatorType; | ||
if ( | ||
!overlapTransferSize || | ||
!isLikelyStaticResource(overlapInitiatorType) | ||
) { | ||
// Skip cached, cross-orgin entries and resources likely to be dynamically generated. | ||
continue; | ||
} | ||
// $FlowFixMe[prop-missing] | ||
const overlapEndTime: number = overlapEntry.responseEnd; | ||
const overlapFactor = | ||
overlapEndTime < parentEndTime | ||
? 1 | ||
: (parentEndTime - overlapStartTime) / | ||
(overlapEndTime - overlapStartTime); | ||
overlappingBytes += overlapTransferSize * overlapFactor; | ||
} | ||
// Skip past any entries we already considered overlapping. Otherwise we'd have to go | ||
// back to consider previous entries when we then handled them. | ||
i = j - 1; | ||
|
||
const bps = | ||
((transferSize + overlappingBytes) * 8) / (entry.duration / 1000); | ||
bits += bps; | ||
count++; | ||
if (count > 10) { | ||
// We have enough to get an average. | ||
break; | ||
} | ||
} | ||
if (count > 0) { | ||
return bits / count / 1e6; | ||
} | ||
} | ||
|
||
// Fallback to the navigator.connection estimate if available | ||
// $FlowFixMe[prop-missing] | ||
if (navigator.connection) { | ||
// $FlowFixMe | ||
const downlink: ?number = navigator.connection.downlink; | ||
if (typeof downlink === 'number') { | ||
return downlink; | ||
} | ||
} | ||
|
||
// Otherwise, use a default of 5mbps to compute heuristics. | ||
// This can happen commonly in Safari if all static resources and images are loaded | ||
// cross-orgin. | ||
return 5; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we now estimate before the timeout, I extended the actual timeout so that if we get the estimate wrong it's worth waiting a bit longer to at least get some benefit from having waited a bit already.