Traverse commit phase effects iteratively (#20094)

* Move traversal logic to ReactFiberCommitWork The current traversal logic is spread between ReactFiberWorkLoop and ReactFiberCommitWork, and it's a bit awkward, especially when refactoring. Idk the ideal module structure, so for now I'd rather keep it all in one file. * Traverse commit phase effects iteratively We suspect that using the JS stack to traverse through the tree in the commit phase is slower than traversing iteratively. I've kept the recursive implementation behind a flag, both so we have the option to run an experiment comparing the two, and so we can revert it easily later if needed.
facebook · Oct 27, 2020 · 25b18d3 · 25b18d3
1 parent 06a4615
commit 25b18d3
Show file tree

Hide file tree

Showing 11 changed files with 1,255 additions and 623 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js
@@ -136,3 +136,5 @@ export const enableDiscreteEventFlushingChange = false;
 export const enableDoubleInvokingEffects = false;
 
 export const enableUseRefAccessWarning = false;
+
+export const enableRecursiveCommitTraversal = false;
diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js
@@ -53,6 +53,8 @@ export const enableDiscreteEventFlushingChange = false;
 export const enableDoubleInvokingEffects = false;
 export const enableUseRefAccessWarning = false;
 
+export const enableRecursiveCommitTraversal = false;
+
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
 type Check<_X, Y: _X, X: Y = _X> = null;

diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js
@@ -52,6 +52,8 @@ export const enableDiscreteEventFlushingChange = false;
 export const enableDoubleInvokingEffects = false;
 export const enableUseRefAccessWarning = false;
 
+export const enableRecursiveCommitTraversal = false;
+
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
 type Check<_X, Y: _X, X: Y = _X> = null;

diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js
@@ -52,6 +52,8 @@ export const enableDiscreteEventFlushingChange = false;
 export const enableDoubleInvokingEffects = false;
 export const enableUseRefAccessWarning = false;
 
+export const enableRecursiveCommitTraversal = false;
+
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
 type Check<_X, Y: _X, X: Y = _X> = null;

diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
@@ -52,6 +52,8 @@ export const enableDiscreteEventFlushingChange = false;
 export const enableDoubleInvokingEffects = false;
 export const enableUseRefAccessWarning = false;
 
+export const enableRecursiveCommitTraversal = false;
+
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
 type Check<_X, Y: _X, X: Y = _X> = null;

diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
@@ -52,6 +52,8 @@ export const enableDiscreteEventFlushingChange = false;
 export const enableDoubleInvokingEffects = false;
 export const enableUseRefAccessWarning = false;
 
+export const enableRecursiveCommitTraversal = false;
+
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
 type Check<_X, Y: _X, X: Y = _X> = null;

diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js
@@ -52,6 +52,8 @@ export const enableDiscreteEventFlushingChange = false;
 export const enableDoubleInvokingEffects = false;
 export const enableUseRefAccessWarning = false;
 
+export const enableRecursiveCommitTraversal = false;
+
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
 type Check<_X, Y: _X, X: Y = _X> = null;

diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js
@@ -52,6 +52,8 @@ export const enableDiscreteEventFlushingChange = true;
 export const enableDoubleInvokingEffects = false;
 export const enableUseRefAccessWarning = false;
 
+export const enableRecursiveCommitTraversal = false;
+
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
 type Check<_X, Y: _X, X: Y = _X> = null;

diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js
@@ -84,6 +84,8 @@ export const enableDiscreteEventFlushingChange = true;
 // to the correct value.
 export const enableNewReconciler = __VARIANT__;
 
+export const enableRecursiveCommitTraversal = false;
+
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
 type Check<_X, Y: _X, X: Y = _X> = null;
Original file line number	Diff line number	Diff line change
Expand Up		@@ -136,3 +136,5 @@ export const enableDiscreteEventFlushingChange = false;
		export const enableDoubleInvokingEffects = false;

		export const enableUseRefAccessWarning = false;

		export const enableRecursiveCommitTraversal = false;