From 94e4acaa1477e65cac02ba86058cde0afe4c8f1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Fri, 13 Sep 2024 12:27:00 -0400 Subject: [PATCH] [Fiber] Set profiler values to doubles (#30942) At some point this trick was added to initialize the value first to NaN and then replace them with zeros and negative ones. This is to address the issue noted in https://github.com/facebook/react/issues/14365 where these hidden classes can be initialized to SMIs at first and then deopt when we realize they're actually doubles. However, this fix has been long broken and has deopted the profiling build for years because closure compiler optimizes out the first write. I'm not sure because I haven't A/B-tested this in the JIT yet but I think we can use negative zero and -1.1 as the initial values instead since they're not simple integers. Negative zero `===` zero (but not Object.is) so is the same as far as our code is concerned. The negative value is just `< 0` comparisons. --- packages/react-reconciler/src/ReactFiber.js | 27 +++++++------------ .../src/ReactFiberBeginWork.js | 12 ++++----- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 4147f0d04e..29a8931038 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -187,18 +187,11 @@ function FiberNode( // Learn more about this here: // https://github.com/facebook/react/issues/14365 // https://bugs.chromium.org/p/v8/issues/detail?id=8538 - this.actualDuration = Number.NaN; - this.actualStartTime = Number.NaN; - this.selfBaseDuration = Number.NaN; - this.treeBaseDuration = Number.NaN; - // It's okay to replace the initial doubles with smis after initialization. - // This won't trigger the performance cliff mentioned above, - // and it simplifies other profiler code (including DevTools). - this.actualDuration = 0; - this.actualStartTime = -1; - this.selfBaseDuration = 0; - this.treeBaseDuration = 0; + this.actualDuration = -0; + this.actualStartTime = -1.1; + this.selfBaseDuration = -0; + this.treeBaseDuration = -0; } if (__DEV__) { @@ -286,10 +279,10 @@ function createFiberImplObject( }; if (enableProfilerTimer) { - fiber.actualDuration = 0; - fiber.actualStartTime = -1; - fiber.selfBaseDuration = 0; - fiber.treeBaseDuration = 0; + fiber.actualDuration = -0; + fiber.actualStartTime = -1.1; + fiber.selfBaseDuration = -0; + fiber.treeBaseDuration = -0; } if (__DEV__) { @@ -382,8 +375,8 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber { // This prevents time from endlessly accumulating in new commits. // This has the downside of resetting values for different priority renders, // But works for yielding (the common case) and should support resuming. - workInProgress.actualDuration = 0; - workInProgress.actualStartTime = -1; + workInProgress.actualDuration = -0; + workInProgress.actualStartTime = -1.1; } } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 34176bb83b..c71b704475 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -2430,10 +2430,10 @@ function mountSuspenseFallbackChildren( // final amounts. This seems counterintuitive, since we're intentionally // not measuring part of the render phase, but this makes it match what we // do in Concurrent Mode. - primaryChildFragment.actualDuration = 0; - primaryChildFragment.actualStartTime = -1; - primaryChildFragment.selfBaseDuration = 0; - primaryChildFragment.treeBaseDuration = 0; + primaryChildFragment.actualDuration = -0; + primaryChildFragment.actualStartTime = -1.1; + primaryChildFragment.selfBaseDuration = -0; + primaryChildFragment.treeBaseDuration = -0; } fallbackChildFragment = createFiberFromFragment( @@ -2560,8 +2560,8 @@ function updateSuspenseFallbackChildren( // final amounts. This seems counterintuitive, since we're intentionally // not measuring part of the render phase, but this makes it match what we // do in Concurrent Mode. - primaryChildFragment.actualDuration = 0; - primaryChildFragment.actualStartTime = -1; + primaryChildFragment.actualDuration = -0; + primaryChildFragment.actualStartTime = -1.1; primaryChildFragment.selfBaseDuration = currentPrimaryChildFragment.selfBaseDuration; primaryChildFragment.treeBaseDuration =