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.
Dan Abramov committed
Apr 1, 2018 at 19:10 UTC
59dac9d7a6a2f0b66003cf717d71b5587265423f
3 files changed
+96
-3
packages/react-reconciler/src/ReactFiber.js
+44
@@ -471,3 +471,47 @@ export function createFiberFromPortal(
471
};
472
return fiber;
473
}
474
+
475
+// Used for stashing WIP properties to replay failed work in DEV.
476
+export function assignFiberPropertiesInDEV(
477
+ target: Fiber | null,
478
+ source: Fiber,
479
+): Fiber {
480
+ if (target === null) {
481
+ // This Fiber's initial properties will always be overwritten.
482
+ // We only use a Fiber to ensure the same hidden class so DEV isn't slow.
483
+ target = createFiber(IndeterminateComponent, null, null, NoContext);
484
+ }
485
+
486
+ // This is intentionally written as a list of all properties.
487
+ // We tried to use Object.assign() instead but this is called in
488
+ // the hottest path, and Object.assign() was too slow:
489
+ // https://github.com/facebook/react/issues/12502
490
+ // This code is DEV-only so size is not a concern.
491
+
492
+ target.tag = source.tag;
493
+ target.key = source.key;
494
+ target.type = source.type;
495
+ target.stateNode = source.stateNode;
496
+ target.return = source.return;
497
+ target.child = source.child;
498
+ target.sibling = source.sibling;
499
+ target.index = source.index;
500
+ target.ref = source.ref;
501
+ target.pendingProps = source.pendingProps;
502
+ target.memoizedProps = source.memoizedProps;
503
+ target.updateQueue = source.updateQueue;
504
+ target.memoizedState = source.memoizedState;
505
+ target.mode = source.mode;
506
+ target.effectTag = source.effectTag;
507
+ target.nextEffect = source.nextEffect;
508
+ target.firstEffect = source.firstEffect;
509
+ target.lastEffect = source.lastEffect;
510
+ target.expirationTime = source.expirationTime;
511
+ target.alternate = source.alternate;
512
+ target._debugID = source._debugID;
513
+ target._debugSource = source._debugSource;
514
+ target._debugOwner = source._debugOwner;
515
+ target._debugIsCurrentlyTiming = source._debugIsCurrentlyTiming;
516
+ return target;
517
+}
packages/react-reconciler/src/ReactFiberScheduler.js
+9
-3
@@ -76,7 +76,7 @@ import {
76
startCommitLifeCyclesTimer,
77
stopCommitLifeCyclesTimer,
78
} from './ReactDebugFiberPerf';
79
-import {createWorkInProgress} from './ReactFiber';
79
+import {createWorkInProgress, assignFiberPropertiesInDEV} from './ReactFiber';
80
import {onCommitRoot} from './ReactFiberDevToolsHook';
81
import {
82
NoWork,
@@ -267,7 +267,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
267
stashedWorkInProgressProperties = null;
268
replayUnitOfWork = (failedUnitOfWork: Fiber, isAsync: boolean) => {
269
// Retore the original state of the work-in-progress
270
- Object.assign(failedUnitOfWork, stashedWorkInProgressProperties);
270
+ assignFiberPropertiesInDEV(
271
+ failedUnitOfWork,
272
+ stashedWorkInProgressProperties,
273
+ );
274
switch (failedUnitOfWork.tag) {
275
case HostRoot:
276
popHostContainer(failedUnitOfWork);
@@ -846,7 +849,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
849
}
850
851
if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
849
- stashedWorkInProgressProperties = Object.assign({}, workInProgress);
852
+ stashedWorkInProgressProperties = assignFiberPropertiesInDEV(
853
+ stashedWorkInProgressProperties,
854
+ workInProgress,
855
+ );
856
}
857
let next = beginWork(current, workInProgress, nextRenderExpirationTime);
858
packages/react-reconciler/src/__tests__/ReactIncrementalErrorReplay-test.internal.js
new
+43
@@ -0,0 +1,43 @@
1
+/**
2
+ * Copyright (c) 2013-present, Facebook, Inc.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ *
7
+ * @jest-environment node
8
+ */
9
+
10
+'use strict';
11
+
12
+describe('ReactIncrementalErrorReplay-test', () => {
13
+ it('copies all keys when stashing potentially failing work', () => {
14
+ // Note: this test is fragile and relies on internals.
15
+ // We almost always try to avoid such tests, but here the cost of
16
+ // the list getting out of sync (and causing subtle bugs in rare cases)
17
+ // is higher than the cost of maintaing the test.
18
+ const {
19
+ // Any Fiber factory function will do.
20
+ createHostRootFiber,
21
+ // This is the method we're going to test.
22
+ // If this is no longer used, you can delete this test file.
23
+ assignFiberPropertiesInDEV,
24
+ } = require('../ReactFiber');
25
+
26
+ // Get a real fiber.
27
+ const realFiber = createHostRootFiber(false);
28
+ const stash = assignFiberPropertiesInDEV(null, realFiber);
29
+
30
+ // Verify we get all the same fields.
31
+ expect(realFiber).toEqual(stash);
32
+
33
+ // Mutate the original.
34
+ for (let key in realFiber) {
35
+ realFiber[key] = key + '_' + Math.random();
36
+ }
37
+ expect(realFiber).not.toEqual(stash);
38
+
39
+ // Verify we can still "revert" to the stashed properties.
40
+ expect(assignFiberPropertiesInDEV(realFiber, stash)).toBe(realFiber);
41
+ expect(realFiber).toEqual(stash);
42
+ });
43
+});