[Draft][Transition Tracing] More Accurate End Time #25105

lunaruan · 2022-08-16T20:22:45Z

No description provided.

sizebot · 2022-08-16T20:28:54Z

Comparing: a9dc73c...7f5b876

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	134.97 kB	134.97 kB	=	43.23 kB	43.23 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	141.64 kB	141.64 kB	=	45.20 kB	45.20 kB
facebook-www/ReactDOM-prod.classic.js	+0.30%	486.06 kB	487.51 kB	+0.30%	86.51 kB	86.77 kB
facebook-www/ReactDOM-prod.modern.js	+0.31%	471.34 kB	472.79 kB	+0.29%	84.29 kB	84.54 kB
facebook-www/ReactDOMForked-prod.classic.js	+0.30%	486.06 kB	487.51 kB	+0.30%	86.51 kB	86.77 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-experimental/react-noop-renderer/cjs/react-noop-renderer.production.min.js	+0.53%	12.37 kB	12.43 kB	+0.92%	3.68 kB	3.72 kB
oss-stable-semver/react-noop-renderer/cjs/react-noop-renderer.production.min.js	+0.53%	12.37 kB	12.43 kB	+0.92%	3.68 kB	3.72 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer.production.min.js	+0.53%	12.37 kB	12.43 kB	+0.92%	3.68 kB	3.72 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer-persistent.production.min.js	+0.53%	12.44 kB	12.50 kB	+0.92%	3.70 kB	3.74 kB
oss-stable-semver/react-noop-renderer/cjs/react-noop-renderer-persistent.production.min.js	+0.53%	12.44 kB	12.50 kB	+0.92%	3.70 kB	3.74 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer-persistent.production.min.js	+0.53%	12.44 kB	12.50 kB	+0.92%	3.70 kB	3.74 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer.development.js	+0.38%	34.45 kB	34.58 kB	+0.52%	7.76 kB	7.80 kB
oss-stable-semver/react-noop-renderer/cjs/react-noop-renderer.development.js	+0.38%	34.45 kB	34.58 kB	+0.52%	7.76 kB	7.80 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer.development.js	+0.38%	34.45 kB	34.58 kB	+0.52%	7.76 kB	7.80 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer-persistent.development.js	+0.38%	34.59 kB	34.72 kB	+0.49%	7.78 kB	7.81 kB
oss-stable-semver/react-noop-renderer/cjs/react-noop-renderer-persistent.development.js	+0.38%	34.59 kB	34.72 kB	+0.49%	7.78 kB	7.81 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer-persistent.development.js	+0.38%	34.59 kB	34.72 kB	+0.49%	7.78 kB	7.81 kB
facebook-www/ReactDOM-prod.modern.js	+0.31%	471.34 kB	472.79 kB	+0.29%	84.29 kB	84.54 kB
facebook-www/ReactDOMForked-prod.modern.js	+0.31%	471.34 kB	472.79 kB	+0.29%	84.29 kB	84.54 kB
facebook-www/ReactDOM-prod.classic.js	+0.30%	486.06 kB	487.51 kB	+0.30%	86.51 kB	86.77 kB
facebook-www/ReactDOMForked-prod.classic.js	+0.30%	486.06 kB	487.51 kB	+0.30%	86.51 kB	86.77 kB
facebook-www/ReactDOM-profiling.modern.js	+0.29%	501.15 kB	502.60 kB	+0.29%	88.79 kB	89.05 kB
facebook-www/ReactDOMForked-profiling.modern.js	+0.29%	501.15 kB	502.60 kB	+0.29%	88.79 kB	89.05 kB
facebook-www/ReactDOM-profiling.classic.js	+0.28%	515.93 kB	517.39 kB	+0.30%	91.13 kB	91.40 kB
facebook-www/ReactDOMForked-profiling.classic.js	+0.28%	515.93 kB	517.39 kB	+0.30%	91.14 kB	91.40 kB

Generated by 🚫 dangerJS against 7f5b876

lunaruan · 2022-08-18T15:55:37Z

packages/react-reconciler/src/ReactFiberWorkLoop.new.js

-          prevRootTransitionCallbacks,
-        ),
-      );
+      setNextPaintCallback(endTime => {


move this schedule call synchronous commit phase

If they've already fired, then we can fire the callbacks. But if they haven't, we stash the time somewhere and when they do get flushed use it

acdlite · 2022-09-07T15:42:52Z

packages/react-reconciler/src/ReactFiberWorkLoop.new.js

@@ -2639,6 +2641,26 @@ function commitRootImpl(
    markCommitStopped();
  }

+  // if (enableTransitionTracing) {
+  const prevRootTransitionCallbacks = root.transitionCallbacks;


This really needs a comment explaining what this block does. I know what it does, but I think it'd be really difficult for the next person to infer just from the code.

acdlite · 2022-09-07T15:45:30Z

packages/react-reconciler/src/ReactFiberWorkLoop.new.js

@@ -2639,6 +2641,26 @@ function commitRootImpl(
    markCommitStopped();
  }

+  // if (enableTransitionTracing) {
+  const prevRootTransitionCallbacks = root.transitionCallbacks;
+  if (prevRootTransitionCallbacks !== null) {


Maybe this should check currentPendingTransitionCallback instead?

For example, if there were multiple roots scheduled, we still only need a single profiling callback.

There should be a test(s) for multiple roots because that can sometimes be tricky to get right.

Do you also have tests that show multiple commits happening before a paint?

A simple example of multiple commits in a single paint is something that uses useLayoutEffect to re-render based on layout.

I actually think that we won't be able to check currentPendingTransitionCallback, because it might not be populated at the end of commit phase (only the passive effects phase). This check is just making sure the callbacks exist so we don't do extra work when the user didn't actually have any callbacks.

We should only ever have one currentPendingTransitionCallback because we push everything onto it and clear it when we process it.

Maybe a different variable like isPostPaintCallbackScheduled then. Then it doesn't matter whether the already-scheduled callbacks are on this root or a different root.

Hm, I'm not actually sure what you mean. Different roots will have different end times depending on the commit right? So we actually probably likely do want to schedule different callbacks for them.

One paint callback

…th `requestPostPaintCallback` function

lunaruan · 2022-09-08T15:14:19Z

packages/react-reconciler/src/ReactFiberWorkLoop.old.js

@@ -2639,6 +2641,36 @@ function commitRootImpl(
    markCommitStopped();
  }

+  if (enableTransitionTracing) {


move in another module maybe

acdlite · 2022-09-13T15:13:22Z

packages/react-reconciler/src/ReactFiberWorkLoop.old.js

+    // and then call the callback via the correct end time.
+    const prevRootTransitionCallbacks = root.transitionCallbacks;
+    if (prevRootTransitionCallbacks !== null) {
+      schedulPostPaintCallback(endTime => {


Suggested change

schedulPostPaintCallback(endTime => {

schedulePostPaintCallback(endTime => {

Summary: This sync includes the following changes: - **[0cac4d54c](facebook/react@0cac4d54c )**: Double invoked effects on suspended children ([#25307](facebook/react#25307)) //<Samuel Susla>// - **[3d615fc14](facebook/react@3d615fc14 )**: Grammar. Removed doubles of the word "the". ([#25295](facebook/react#25295)) //<Victoria Graf>// - **[6e3bc8a2e](facebook/react@6e3bc8a2e )**: [DevTools] Check if Proxy exists before creating DispatcherProxy ([#25278](facebook/react#25278)) //<Tianyu Yao>// - **[e7fc04b29](facebook/react@e7fc04b29 )**: [react-dom] Reorganize react-dom internals to match react ([#25277](facebook/react#25277)) //<Josh Story>// - **[0b54e0047](facebook/react@0b54e0047 )**: Handle rejections to avoid uncaught rejections ([#25272](facebook/react#25272)) //<Sebastian Markbåge>// - **[c5d06fdc5](facebook/react@c5d06fdc5 )**: [Flight] Fix Webpack Chunk Loading ([#25271](facebook/react#25271)) //<Sebastian Markbåge>// - **[975b64464](facebook/react@975b64464 )**: [Flight] response.readRoot() -> use(response) ([#25267](facebook/react#25267)) //<Sebastian Markbåge>// - **[60fbb7b14](facebook/react@60fbb7b14 )**: [Flight] Implement FlightClient in terms of Thenable/Promises instead of throwing Promises ([#25260](facebook/react#25260)) //<Sebastian Markbåge>// - **[c91a1e03b](facebook/react@c91a1e03b )**: experimental_useEvent ([#25229](facebook/react#25229)) //<Lauren Tan>// - **[346c7d4c4](facebook/react@346c7d4c4 )**: straightford explicit types ([#25253](facebook/react#25253)) //<Jan Kassens>// - **[3401e9200](facebook/react@3401e9200 )**: useMemoCache implementation ([#25143](facebook/react#25143)) //<Joseph Savona>// - **[0556bab32](facebook/react@0556bab32 )**: [Transition Tracing] More Accurate End Time ([#25105](facebook/react#25105)) //<Luna Ruan>// - **[5fdcd23aa](facebook/react@5fdcd23aa )**: Flow: upgrade to 0.140 ([#25252](facebook/react#25252)) //<Jan Kassens>// - **[5c43c6f02](facebook/react@5c43c6f02 )**: Unwind the current workInProgress if it's suspended ([#25247](facebook/react#25247)) //<Sebastian Markbåge>// - **[e52fa4c57](facebook/react@e52fa4c57 )**: Add early exit to strict mode ([#25235](facebook/react#25235)) //<Samuel Susla>// - **[6aa38e74c](facebook/react@6aa38e74c )**: Flow: enable unsafe-addition error ([#25242](facebook/react#25242)) //<Jan Kassens>// - **[ba7b6f418](facebook/react@ba7b6f418 )**: Flow: upgrade to 0.132 ([#25244](facebook/react#25244)) //<Jan Kassens>// - **[9328988c0](facebook/react@9328988c0 )**: Flow: fix Fiber typed as any ([#25241](facebook/react#25241)) //<Jan Kassens>// - **[c739cef2f](facebook/react@c739cef2f )**: Flow: ReactFiberHotReloading recursive type ([#25225](facebook/react#25225)) //<Jan Kassens>// - **[c156ecd48](facebook/react@c156ecd48 )**: Add some test coverage for some error cases ([#25240](facebook/react#25240)) //<Sebastian Markbåge>// - **[3613284dc](facebook/react@3613284dc )**: experimental_use(context) for server components and ssr ([#25226](facebook/react#25226)) //<mofeiZ>// - **[269c4e975](facebook/react@269c4e975 )**: Prevent infinite re-renders in StrictMode + Offscreen ([#25203](facebook/react#25203)) //<Samuel Susla>// - **[8003ab9cf](facebook/react@8003ab9cf )**: Flow: remove explicit object syntax ([#25223](facebook/react#25223)) //<Jan Kassens>// - **[492c6e29e](facebook/react@492c6e29e )**: Flow: upgrade to 0.127 ([#25221](facebook/react#25221)) //<Jan Kassens>// - **[8a9e7b6ce](facebook/react@8a9e7b6ce )**: Flow: implicit-inexact-object=error ([#25210](facebook/react#25210)) //<Jan Kassens>// - **[37cc6bf12](facebook/react@37cc6bf12 )**: Remove useDeferredValue and useTransition from Flight subset ([#25215](facebook/react#25215)) //<Sebastian Markbåge>// Changelog: [General][Changed] - React Native sync for revisions c28f313...0cac4d5 jest_e2e[run_all_tests] Reviewed By: rickhanlonii Differential Revision: D39696377 fbshipit-source-id: 113878d22d6244b8555b5fb86db1da5d43f7cfd9

add more accurate end time for transitions and update host configs with `requestPostPaintCallback` function and move post paint logic to another module and use it in the work loop

Summary: This sync includes the following changes: - **[0cac4d54c](facebook/react@0cac4d54c )**: Double invoked effects on suspended children ([facebook#25307](facebook/react#25307)) //<Samuel Susla>// - **[3d615fc14](facebook/react@3d615fc14 )**: Grammar. Removed doubles of the word "the". ([facebook#25295](facebook/react#25295)) //<Victoria Graf>// - **[6e3bc8a2e](facebook/react@6e3bc8a2e )**: [DevTools] Check if Proxy exists before creating DispatcherProxy ([facebook#25278](facebook/react#25278)) //<Tianyu Yao>// - **[e7fc04b29](facebook/react@e7fc04b29 )**: [react-dom] Reorganize react-dom internals to match react ([facebook#25277](facebook/react#25277)) //<Josh Story>// - **[0b54e0047](facebook/react@0b54e0047 )**: Handle rejections to avoid uncaught rejections ([facebook#25272](facebook/react#25272)) //<Sebastian Markbåge>// - **[c5d06fdc5](facebook/react@c5d06fdc5 )**: [Flight] Fix Webpack Chunk Loading ([facebook#25271](facebook/react#25271)) //<Sebastian Markbåge>// - **[975b64464](facebook/react@975b64464 )**: [Flight] response.readRoot() -> use(response) ([facebook#25267](facebook/react#25267)) //<Sebastian Markbåge>// - **[60fbb7b14](facebook/react@60fbb7b14 )**: [Flight] Implement FlightClient in terms of Thenable/Promises instead of throwing Promises ([facebook#25260](facebook/react#25260)) //<Sebastian Markbåge>// - **[c91a1e03b](facebook/react@c91a1e03b )**: experimental_useEvent ([facebook#25229](facebook/react#25229)) //<Lauren Tan>// - **[346c7d4c4](facebook/react@346c7d4c4 )**: straightford explicit types ([facebook#25253](facebook/react#25253)) //<Jan Kassens>// - **[3401e9200](facebook/react@3401e9200 )**: useMemoCache implementation ([facebook#25143](facebook/react#25143)) //<Joseph Savona>// - **[0556bab32](facebook/react@0556bab32 )**: [Transition Tracing] More Accurate End Time ([facebook#25105](facebook/react#25105)) //<Luna Ruan>// - **[5fdcd23aa](facebook/react@5fdcd23aa )**: Flow: upgrade to 0.140 ([facebook#25252](facebook/react#25252)) //<Jan Kassens>// - **[5c43c6f02](facebook/react@5c43c6f02 )**: Unwind the current workInProgress if it's suspended ([facebook#25247](facebook/react#25247)) //<Sebastian Markbåge>// - **[e52fa4c57](facebook/react@e52fa4c57 )**: Add early exit to strict mode ([facebook#25235](facebook/react#25235)) //<Samuel Susla>// - **[6aa38e74c](facebook/react@6aa38e74c )**: Flow: enable unsafe-addition error ([facebook#25242](facebook/react#25242)) //<Jan Kassens>// - **[ba7b6f418](facebook/react@ba7b6f418 )**: Flow: upgrade to 0.132 ([facebook#25244](facebook/react#25244)) //<Jan Kassens>// - **[9328988c0](facebook/react@9328988c0 )**: Flow: fix Fiber typed as any ([facebook#25241](facebook/react#25241)) //<Jan Kassens>// - **[c739cef2f](facebook/react@c739cef2f )**: Flow: ReactFiberHotReloading recursive type ([facebook#25225](facebook/react#25225)) //<Jan Kassens>// - **[c156ecd48](facebook/react@c156ecd48 )**: Add some test coverage for some error cases ([facebook#25240](facebook/react#25240)) //<Sebastian Markbåge>// - **[3613284dc](facebook/react@3613284dc )**: experimental_use(context) for server components and ssr ([facebook#25226](facebook/react#25226)) //<mofeiZ>// - **[269c4e975](facebook/react@269c4e975 )**: Prevent infinite re-renders in StrictMode + Offscreen ([facebook#25203](facebook/react#25203)) //<Samuel Susla>// - **[8003ab9cf](facebook/react@8003ab9cf )**: Flow: remove explicit object syntax ([facebook#25223](facebook/react#25223)) //<Jan Kassens>// - **[492c6e29e](facebook/react@492c6e29e )**: Flow: upgrade to 0.127 ([facebook#25221](facebook/react#25221)) //<Jan Kassens>// - **[8a9e7b6ce](facebook/react@8a9e7b6ce )**: Flow: implicit-inexact-object=error ([facebook#25210](facebook/react#25210)) //<Jan Kassens>// - **[37cc6bf12](facebook/react@37cc6bf12 )**: Remove useDeferredValue and useTransition from Flight subset ([facebook#25215](facebook/react#25215)) //<Sebastian Markbåge>// Changelog: [General][Changed] - React Native sync for revisions c28f313...0cac4d5 jest_e2e[run_all_tests] Reviewed By: rickhanlonii Differential Revision: D39696377 fbshipit-source-id: 113878d22d6244b8555b5fb86db1da5d43f7cfd9

lunaruan requested a review from acdlite August 16, 2022 20:22

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Aug 16, 2022

lunaruan commented Aug 18, 2022

View reviewed changes

lunaruan force-pushed the end_time branch 5 times, most recently from 59e8d7f to 1fb719b Compare August 25, 2022 13:02

lunaruan force-pushed the end_time branch 3 times, most recently from cefc22d to 3ce1cb5 Compare September 7, 2022 00:29

acdlite reviewed Sep 7, 2022

View reviewed changes

lunaruan force-pushed the end_time branch from 3ce1cb5 to 27c36b0 Compare September 7, 2022 21:28

lunaruan added 2 commits September 7, 2022 14:43

add more accurate end time for transitions and update host configs wi…

61e77a9

…th `requestPostPaintCallback` function

old

52f8f7b

lunaruan force-pushed the end_time branch from 27c36b0 to 52f8f7b Compare September 7, 2022 21:49

lunaruan commented Sep 8, 2022

View reviewed changes

acdlite reviewed Sep 13, 2022

View reviewed changes

acdlite approved these changes Sep 13, 2022

View reviewed changes

lunaruan force-pushed the end_time branch from 598e074 to 7e0407f Compare September 13, 2022 15:46

lunaruan added 2 commits September 13, 2022 11:57

move post paint logic to another module and use it in the work loop

6e9edd3

old

7f5b876

lunaruan force-pushed the end_time branch from 7e0407f to 7f5b876 Compare September 13, 2022 15:57

lunaruan merged commit 0556bab into facebook:main Sep 13, 2022

CodyJasonBennett mentioned this pull request Apr 30, 2024

[React 19] react-reconciler README is missing scheduler and form hooks #28956

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Draft][Transition Tracing] More Accurate End Time #25105

[Draft][Transition Tracing] More Accurate End Time #25105

lunaruan commented Aug 16, 2022

sizebot commented Aug 16, 2022 •

edited

Loading

lunaruan Aug 18, 2022

lunaruan Aug 18, 2022

acdlite Sep 7, 2022

acdlite Sep 7, 2022

acdlite Sep 7, 2022

lunaruan Sep 7, 2022

acdlite Sep 7, 2022

lunaruan Sep 7, 2022

lunaruan Sep 8, 2022

lunaruan Sep 8, 2022

lunaruan Sep 8, 2022

acdlite Sep 13, 2022

	schedulPostPaintCallback(endTime => {
	schedulePostPaintCallback(endTime => {

[Draft][Transition Tracing] More Accurate End Time #25105

[Draft][Transition Tracing] More Accurate End Time #25105

Conversation

lunaruan commented Aug 16, 2022

sizebot commented Aug 16, 2022 • edited Loading

Critical size changes

Significant size changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sizebot commented Aug 16, 2022 •

edited

Loading