-
Notifications
You must be signed in to change notification settings - Fork 47k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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.
- Loading branch information
Showing
3 changed files
with
96 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
43 changes: 43 additions & 0 deletions
43
packages/react-reconciler/src/__tests__/ReactIncrementalErrorReplay-test.internal.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); | ||
}); | ||
}); |