Fix DEV performance regression by avoiding Object.assign on Fibers (#12510)

* Fix DEV performance regression by avoiding Object.assign on Fibers

* Reduce allocations in hot path by reusing the stash

Since performUnitOfWork() is not reentrant, it should be safe to reuse the same stash every time instead of creating a new object.

Author: gaearon

packages/react-reconciler/src/ReactFiber.js

@@ -471,3 +471,47 @@ export function createFiberFromPortal(
   };
   return fiber;
 }
+
+// Used for stashing WIP properties to replay failed work in DEV.
+export function assignFiberPropertiesInDEV(
+  target: Fiber | null,
+  source: Fiber,
+): Fiber {
+  if (target === null) {
+    // This Fiber's initial properties will always be overwritten.
+    // We only use a Fiber to ensure the same hidden class so DEV isn't slow.
+    target = createFiber(IndeterminateComponent, null, null, NoContext);
+  }
+
+  // This is intentionally written as a list of all properties.
+  // We tried to use Object.assign() instead but this is called in
+  // the hottest path, and Object.assign() was too slow:
+  // https://github.com/facebook/react/issues/12502
+  // This code is DEV-only so size is not a concern.
+
+  target.tag = source.tag;
+  target.key = source.key;
+  target.type = source.type;
+  target.stateNode = source.stateNode;
+  target.return = source.return;
+  target.child = source.child;
+  target.sibling = source.sibling;
+  target.index = source.index;
+  target.ref = source.ref;
+  target.pendingProps = source.pendingProps;
+  target.memoizedProps = source.memoizedProps;
+  target.updateQueue = source.updateQueue;
+  target.memoizedState = source.memoizedState;
+  target.mode = source.mode;
+  target.effectTag = source.effectTag;
+  target.nextEffect = source.nextEffect;
+  target.firstEffect = source.firstEffect;
+  target.lastEffect = source.lastEffect;
+  target.expirationTime = source.expirationTime;
+  target.alternate = source.alternate;
+  target._debugID = source._debugID;
+  target._debugSource = source._debugSource;
+  target._debugOwner = source._debugOwner;
+  target._debugIsCurrentlyTiming = source._debugIsCurrentlyTiming;
+  return target;
+}

packages/react-reconciler/src/ReactFiberScheduler.js

@@ -76,7 +76,7 @@ import {
   startCommitLifeCyclesTimer,
   stopCommitLifeCyclesTimer,
 } from './ReactDebugFiberPerf';
-import {createWorkInProgress} from './ReactFiber';
+import {createWorkInProgress, assignFiberPropertiesInDEV} from './ReactFiber';
 import {onCommitRoot} from './ReactFiberDevToolsHook';
 import {
   NoWork,
@@ -267,7 +267,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
     stashedWorkInProgressProperties = null;
     replayUnitOfWork = (failedUnitOfWork: Fiber, isAsync: boolean) => {
       // Retore the original state of the work-in-progress
-      Object.assign(failedUnitOfWork, stashedWorkInProgressProperties);
+      assignFiberPropertiesInDEV(
+        failedUnitOfWork,
+        stashedWorkInProgressProperties,
+      );
       switch (failedUnitOfWork.tag) {
         case HostRoot:
           popHostContainer(failedUnitOfWork);
@@ -846,7 +849,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
     }
 
     if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
-      stashedWorkInProgressProperties = Object.assign({}, workInProgress);
+      stashedWorkInProgressProperties = assignFiberPropertiesInDEV(
+        stashedWorkInProgressProperties,
+        workInProgress,
+      );
     }
     let next = beginWork(current, workInProgress, nextRenderExpirationTime);
 

packages/react-reconciler/src/__tests__/ReactIncrementalErrorReplay-test.internal.js

@@ -0,0 +1,43 @@
+/**
+ * Copyright (c) 2013-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @jest-environment node
+ */
+
+'use strict';
+
+describe('ReactIncrementalErrorReplay-test', () => {
+  it('copies all keys when stashing potentially failing work', () => {
+    // Note: this test is fragile and relies on internals.
+    // We almost always try to avoid such tests, but here the cost of
+    // the list getting out of sync (and causing subtle bugs in rare cases)
+    // is higher than the cost of maintaing the test.
+    const {
+      // Any Fiber factory function will do.
+      createHostRootFiber,
+      // This is the method we're going to test.
+      // If this is no longer used, you can delete this test file.
+      assignFiberPropertiesInDEV,
+    } = require('../ReactFiber');
+
+    // Get a real fiber.
+    const realFiber = createHostRootFiber(false);
+    const stash = assignFiberPropertiesInDEV(null, realFiber);
+
+    // Verify we get all the same fields.
+    expect(realFiber).toEqual(stash);
+
+    // Mutate the original.
+    for (let key in realFiber) {
+      realFiber[key] = key + '_' + Math.random();
+    }
+    expect(realFiber).not.toEqual(stash);
+
+    // Verify we can still "revert" to the stashed properties.
+    expect(assignFiberPropertiesInDEV(realFiber, stash)).toBe(realFiber);
+    expect(realFiber).toEqual(stash);
+  });
+});