Skip to content

Commit 6ccf328

Browse files
authored
[Fizz] Shorten throttle to hit a specific target metric (#33463)
Adding throttling or delaying on images, can obviously impact metrics. However, it's all in the name of better actual user experience overall. (Note that it's not strictly worse even for metric. Often it's actually strictly better due to less work being done overall thanks to batching.) Metrics can impact things like search ranking but I believe this is on a curve. If you're already pretty good, then a slight delay won't suddenly make you rank in a completely different category. Similarly, if you're already pretty bad then a slight delay won't make it suddenly way worse. It's still in the same realm. It's just one weight of many. I don't think this will make a meaningful practical impact and if it does, that's probably a bug in the weights that will get fixed. However, because there's a race to try to "make everything green" in terms of web vitals, if you go from green to yellow only because of some throttling or suspensey images, it can feel bad. Therefore this implements a heuristic where if the only reason we'd miss a specific target is because of throttling or suspensey images, then we shorten the timeout to hit the metric. This is a worse user experience because it can lead to extra flashing but feeling good about "green" matters too. If you then have another reveal that happens to be the largest contentful paint after that, then that's throttled again so that it doesn't become flashy after that. If you've already missed the deadline then you're not going to hit your metric target anyway. It can affect average but not median. This is mainly about LCP. It doesn't affect FCP since that doesn't have a throttle. If your LCP is the same as your FCP then it also doesn't matter. We assume that `performance.now()`'s zero point starts at the "start of the navigation" which makes this simple. Even if we used the `PerformanceNavigationTiming` API it would just tell us the same thing. This only implements for Fizz since these metrics tend to currently only by tracked for initial loads, but with soft navs tracking we could consider implementing the same for Fiber throttles.
1 parent a374e0e commit 6ccf328

File tree

2 files changed

+30
-8
lines changed

2 files changed

+30
-8
lines changed

packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetShared.js

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,16 @@ const SUSPENSE_PENDING_START_DATA = '$?';
1313
const SUSPENSE_QUEUED_START_DATA = '$~';
1414
const SUSPENSE_FALLBACK_START_DATA = '$!';
1515

16+
const FALLBACK_THROTTLE_MS = 300;
17+
1618
const SUSPENSEY_FONT_AND_IMAGE_TIMEOUT = 500;
1719

20+
// If you have a target goal in mind for a metric to hit, you don't want the
21+
// only reason you miss it by a little bit to be throttling heuristics.
22+
// This tries to avoid throttling if avoiding it would let you hit this metric.
23+
// This is derived from trying to hit an LCP of 2.5 seconds with some head room.
24+
const TARGET_VANITY_METRIC = 2300;
25+
1826
// TODO: Symbols that are referenced outside this module use dynamic accessor
1927
// notation instead of dot notation to prevent Closure's advanced compilation
2028
// mode from renaming. We could use extern files instead, but I couldn't get it
@@ -290,9 +298,18 @@ export function revealCompletedBoundariesWithViewTransitions(
290298
}
291299
return Promise.race([
292300
Promise.all(blockingPromises),
293-
new Promise(resolve =>
294-
setTimeout(resolve, SUSPENSEY_FONT_AND_IMAGE_TIMEOUT),
295-
),
301+
new Promise(resolve => {
302+
const currentTime = performance.now();
303+
const msUntilTimeout =
304+
// If the throttle would make us miss the target metric, then shorten the throttle.
305+
// performance.now()'s zero value is assumed to be the start time of the metric.
306+
currentTime < TARGET_VANITY_METRIC &&
307+
currentTime > TARGET_VANITY_METRIC - FALLBACK_THROTTLE_MS
308+
? TARGET_VANITY_METRIC - currentTime
309+
: // Otherwise it's throttled starting from last commit time.
310+
SUSPENSEY_FONT_AND_IMAGE_TIMEOUT;
311+
setTimeout(resolve, msUntilTimeout);
312+
}),
296313
]);
297314
},
298315
types: [], // TODO: Add a hard coded type for Suspense reveals.
@@ -360,8 +377,6 @@ export function clientRenderBoundary(
360377
}
361378
}
362379

363-
const FALLBACK_THROTTLE_MS = 300;
364-
365380
export function completeBoundary(suspenseBoundaryID, contentID) {
366381
const contentNodeOuter = document.getElementById(contentID);
367382
if (!contentNodeOuter) {
@@ -395,8 +410,15 @@ export function completeBoundary(suspenseBoundaryID, contentID) {
395410
// to flush the batch. This is delayed by the throttle heuristic.
396411
const globalMostRecentFallbackTime =
397412
typeof window['$RT'] !== 'number' ? 0 : window['$RT'];
413+
const currentTime = performance.now();
398414
const msUntilTimeout =
399-
globalMostRecentFallbackTime + FALLBACK_THROTTLE_MS - performance.now();
415+
// If the throttle would make us miss the target metric, then shorten the throttle.
416+
// performance.now()'s zero value is assumed to be the start time of the metric.
417+
currentTime < TARGET_VANITY_METRIC &&
418+
currentTime > TARGET_VANITY_METRIC - FALLBACK_THROTTLE_MS
419+
? TARGET_VANITY_METRIC - currentTime
420+
: // Otherwise it's throttled starting from last commit time.
421+
globalMostRecentFallbackTime + FALLBACK_THROTTLE_MS - currentTime;
400422
// We always schedule the flush in a timer even if it's very low or negative to allow
401423
// for multiple completeBoundary calls that are already queued to have a chance to
402424
// make the batch.

0 commit comments

Comments
 (0)