refactor[react-devtools/fiber/renderer]: optimize durations resolution #31118
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
CLA Signed
React Core Team
hoxyq
force-pushed
the
react-devtools/optimize-durations-for-serialization
branch
from
976c607
to
830e823
Compare
| duration: | ||
| formatDurationToMicrosecondsGranularity(maxActualDuration), | ||
| effectDuration: | ||
| effectDuration !== null | ||
| ? formatDurationToMicrosecondsGranularity(effectDuration) | ||
| : null, | ||
| fiberActualDurations, | ||
| fiberSelfDurations, | ||
| passiveEffectDuration, | ||
| passiveEffectDuration: | ||
| passiveEffectDuration !== null | ||
| ? formatDurationToMicrosecondsGranularity(passiveEffectDuration) | ||
| : null, |
There was a problem hiding this comment.
Would it be cleaner to have formatDurationToMicrosecondsGranularity take null? (I don't have a preference)
|
|
||
| // 0.123456789 => 0.123 | ||
| // Expects high-resolution timestamp in milliseconds, like from performance.now() | ||
| // Mainly used for optimizing the size of serialized profiling payload | ||
| export function formatDurationToMicrosecondsGranularity( | ||
| duration: number, | ||
| ): number { | ||
| return Math.round(duration * 1000) / 1000; | ||
| } |
There was a problem hiding this comment.
Would there be a risk of the frontend precision being out of sync with this here? I'd imagine an all-in solution involving the frontend declaring a precision somehow. (not saying we need it in this PR tho)
There was a problem hiding this comment.
Technically, yes, if we decide to start showing durations with increased granularity. This is not the case for browser extension, though, because backend and frontend are shipped in a single version lockstep.
|
Also, do we have a rough estimate of how much bandwidth this would save/visible speed improvement? Very intrigued as I never thought of using number precision as an optimisation. Big if true! |
React DevTools profiling payload has data for each commit. Every commit will have 2 collections: [ In this PR we are mainly optimizing both It is safe to do it, since we don't actually use it from the Frontend side, see the description. |
Stacked on #31118. See last commit. We don't need to call `startProfiling()` here, because we delegate this to the Renderer itself: https://github.com/facebook/react/blob/830e823cd2c6ee675636d31320b10350e8ade9ae/packages/react-devtools-shared/src/backend/fiber/renderer.js#L5227-L5232 Since this is de-facto the constructor of Renderer, this will be called earlier. Validated via testing the reload-to-profile for Chrome browser extension.
Stacked on #31117.
No need for sending long float numbers and to have resolution less than a microsecond, we end up formatting it on a Frontend side:
react/packages/react-devtools-shared/src/devtools/views/Profiler/utils.js
Lines 359 to 360 in 6c7b41d