@samitouri / QOS-React / commits / f90a6bcc4c

[DevTools] Reconcile Fibers Against Previous Children Instances (#30822)

This loops over the remainingReconcilingChildren to find existing FiberInstances that match the updated Fiber. This is the same thing we already do for virtual instances. This avoids the need for a `fiberToFiberInstanceMap`. This loop is fast but there is a downside when the children set is very large and gets reordered with keys since we might have to loop over the set multiple times to get to the instances in the bottom. If that becomes a problem we can optimize it the same way ReactChildFiber does which is to create a temporary Map only when the children don't line up properly. That way everything except the first pass can use the Map but there's no need to create it eagerly. Now that we have the loop we don't need the previousSibling field so we can save some memory there.

Sebastian Markbåge committed Aug 27, 2024 at 12:05 UTC f90a6bcc4c988f7524ce2be675b3257a530a51e9
1 file changed +89 -77
packages/react-devtools-shared/src/backend/fiber/renderer.js
+89 -77
@@ -158,7 +158,6 @@ type FiberInstance = {
158 id: number,
159 parent: null | DevToolsInstance, // filtered parent, including virtual
160 firstChild: null | DevToolsInstance, // filtered first child, including virtual
161 - previousSibling: null | DevToolsInstance, // filtered next sibling, including virtual
161 nextSibling: null | DevToolsInstance, // filtered next sibling, including virtual
162 flags: number, // Force Error/Suspense
163 source: null | string | Error | Source, // source location of this component function, or owned child stack
@@ -174,7 +173,6 @@ function createFiberInstance(fiber: Fiber): FiberInstance {
173 id: getUID(),
174 parent: null,
175 firstChild: null,
177 - previousSibling: null,
176 nextSibling: null,
177 flags: 0,
178 source: null,
@@ -195,7 +193,6 @@ type VirtualInstance = {
193 id: number,
194 parent: null | DevToolsInstance, // filtered parent, including virtual
195 firstChild: null | DevToolsInstance, // filtered first child, including virtual
198 - previousSibling: null | DevToolsInstance, // filtered next sibling, including virtual
196 nextSibling: null | DevToolsInstance, // filtered next sibling, including virtual
197 flags: number,
198 source: null | string | Error | Source, // source location of this server component, or owned child stack
@@ -218,7 +215,6 @@ function createVirtualInstance(
215 id: getUID(),
216 parent: null,
217 firstChild: null,
221 - previousSibling: null,
218 nextSibling: null,
219 flags: 0,
220 source: null,
@@ -1088,8 +1084,6 @@ export function attach(
1084 ' '.repeat(indent) + '- ' + instance.id + ' (' + name + ')',
1085 'parent',
1086 instance.parent === null ? ' ' : instance.parent.id,
1091 - 'prev',
1092 - instance.previousSibling === null ? ' ' : instance.previousSibling.id,
1087 'next',
1088 instance.nextSibling === null ? ' ' : instance.nextSibling.id,
1089 );
@@ -2321,21 +2315,25 @@ export function attach(
2315 if (previouslyReconciledSibling === null) {
2316 previouslyReconciledSibling = instance;
2317 parentInstance.firstChild = instance;
2324 - instance.previousSibling = null;
2318 } else {
2319 previouslyReconciledSibling.nextSibling = instance;
2327 - instance.previousSibling = previouslyReconciledSibling;
2320 previouslyReconciledSibling = instance;
2321 }
2322 instance.nextSibling = null;
2323 }
2324
2333 - function moveChild(instance: DevToolsInstance): void {
2334 - removeChild(instance);
2325 + function moveChild(
2326 + instance: DevToolsInstance,
2327 + previousSibling: null | DevToolsInstance,
2328 + ): void {
2329 + removeChild(instance, previousSibling);
2330 insertChild(instance);
2331 }
2332
2338 - function removeChild(instance: DevToolsInstance): void {
2333 + function removeChild(
2334 + instance: DevToolsInstance,
2335 + previousSibling: null | DevToolsInstance,
2336 + ): void {
2337 if (instance.parent === null) {
2338 if (remainingReconcilingChildren === instance) {
2339 throw new Error(
@@ -2343,8 +2341,6 @@ export function attach(
2341 );
2342 } else if (instance.nextSibling !== null) {
2343 throw new Error('A deleted instance should not have next siblings');
2346 - } else if (instance.previousSibling !== null) {
2347 - throw new Error('A deleted instance should not have previous siblings');
2344 }
2345 // Already deleted.
2346 return;
@@ -2360,7 +2356,7 @@ export function attach(
2356 }
2357 // Remove an existing child from its current position, which we assume is in the
2358 // remainingReconcilingChildren set.
2363 - if (instance.previousSibling === null) {
2359 + if (previousSibling === null) {
2360 // We're first in the remaining set. Remove us.
2361 if (remainingReconcilingChildren !== instance) {
2362 throw new Error(
@@ -2369,13 +2365,9 @@ export function attach(
2365 }
2366 remainingReconcilingChildren = instance.nextSibling;
2367 } else {
2372 - instance.previousSibling.nextSibling = instance.nextSibling;
2373 - }
2374 - if (instance.nextSibling !== null) {
2375 - instance.nextSibling.previousSibling = instance.previousSibling;
2368 + previousSibling.nextSibling = instance.nextSibling;
2369 }
2370 instance.nextSibling = null;
2378 - instance.previousSibling = null;
2371 instance.parent = null;
2372 }
2373
@@ -2655,7 +2647,7 @@ export function attach(
2647 } else {
2648 recordVirtualUnmount(instance);
2649 }
2658 - removeChild(instance);
2650 + removeChild(instance, null);
2651 }
2652
2653 function recordProfilingDurations(fiberInstance: FiberInstance) {
@@ -2889,8 +2881,7 @@ export function attach(
2881 );
2882 }
2883 }
2892 - // TODO: Find the best matching existing child based on the key if defined.
2893 -
2884 + let previousSiblingOfBestMatch = null;
2885 let bestMatch = remainingReconcilingChildren;
2886 if (componentInfo.key != null) {
2887 // If there is a key try to find a matching key in the set.
@@ -2902,6 +2893,7 @@ export function attach(
2893 ) {
2894 break;
2895 }
2896 + previousSiblingOfBestMatch = bestMatch;
2897 bestMatch = bestMatch.nextSibling;
2898 }
2899 }
@@ -2916,7 +2908,7 @@ export function attach(
2908 // with the same name, then we claim it and reuse it for this update.
2909 // Update it with the latest entry.
2910 bestMatch.data = componentInfo;
2919 - moveChild(bestMatch);
2911 + moveChild(bestMatch, previousSiblingOfBestMatch);
2912 previousVirtualInstance = bestMatch;
2913 previousVirtualInstanceWasMount = false;
2914 } else {
@@ -2965,42 +2957,93 @@ export function attach(
2957 }
2958 previousVirtualInstance = null;
2959 }
2960 +
2961 // We've reached the end of the virtual levels, but not beyond,
2962 // and now continue with the regular fiber.
2963 +
2964 + // Do a fast pass over the remaining children to find the previous instance.
2965 + // TODO: This doesn't have the best O(n) for a large set of children that are
2966 + // reordered. Consider using a temporary map if it's not the very next one.
2967 + let prevChild;
2968 if (prevChildAtSameIndex === nextChild) {
2969 // This set is unchanged. We're just going through it to place all the
2970 // children again.
2971 + prevChild = nextChild;
2972 + } else {
2973 + // We don't actually need to rely on the alternate here. We could also
2974 + // reconcile against stateNode, key or whatever. Doesn't have to be same
2975 + // Fiber pair.
2976 + prevChild = nextChild.alternate;
2977 + }
2978 + let previousSiblingOfExistingInstance = null;
2979 + let existingInstance = null;
2980 + if (prevChild !== null) {
2981 + existingInstance = remainingReconcilingChildren;
2982 + while (existingInstance !== null) {
2983 + if (existingInstance.data === prevChild) {
2984 + break;
2985 + }
2986 + previousSiblingOfExistingInstance = existingInstance;
2987 + existingInstance = existingInstance.nextSibling;
2988 + }
2989 + }
2990 + if (existingInstance !== null) {
2991 + // Common case. Match in the same parent.
2992 + const fiberInstance: FiberInstance = (existingInstance: any); // Only matches if it's a Fiber.
2993 +
2994 + // We keep track if the order of the children matches the previous order.
2995 + // They are always different referentially, but if the instances line up
2996 + // conceptually we'll want to know that.
2997 + if (prevChild !== prevChildAtSameIndex) {
2998 + shouldResetChildren = true;
2999 + }
3000 +
3001 + // Register the new alternate in case it's not already in.
3002 + fiberToFiberInstanceMap.set(nextChild, fiberInstance);
3003 +
3004 + // Update the Fiber so we that we always keep the current Fiber on the data.
3005 + fiberInstance.data = nextChild;
3006 + moveChild(fiberInstance, previousSiblingOfExistingInstance);
3007 +
3008 if (
3009 updateFiberRecursively(
3010 + fiberInstance,
3011 nextChild,
2976 - nextChild,
3012 + (prevChild: any),
3013 traceNearestHostComponentUpdate,
3014 )
3015 ) {
2980 - throw new Error('Updating the same fiber should not cause reorder');
3016 + // If a nested tree child order changed but it can't handle its own
3017 + // child order invalidation (e.g. because it's filtered out like host nodes),
3018 + // propagate the need to reset child order upwards to this Fiber.
3019 + shouldResetChildren = true;
3020 }
2982 - } else if (nextChild.alternate) {
2983 - const prevChild = nextChild.alternate;
3021 + } else if (prevChild !== null && shouldFilterFiber(nextChild)) {
3022 + // If this Fiber should be filtered, we need to still update its children.
3023 + // This relies on an alternate since we don't have an Instance with the previous
3024 + // child on it. Ideally, the reconciliation wouldn't need previous Fibers that
3025 + // are filtered from the tree.
3026 if (
3027 updateFiberRecursively(
3028 + null,
3029 nextChild,
3030 prevChild,
3031 traceNearestHostComponentUpdate,
3032 )
3033 ) {
2991 - // If a nested tree child order changed but it can't handle its own
2992 - // child order invalidation (e.g. because it's filtered out like host nodes),
2993 - // propagate the need to reset child order upwards to this Fiber.
2994 - shouldResetChildren = true;
2995 - }
2996 - // However we also keep track if the order of the children matches
2997 - // the previous order. They are always different referentially, but
2998 - // if the instances line up conceptually we'll want to know that.
2999 - if (prevChild !== prevChildAtSameIndex) {
3034 shouldResetChildren = true;
3035 }
3036 } else {
3037 + // It's possible for a FiberInstance to be reparented when virtual parents
3038 + // get their sequence split or change structure with the same render result.
3039 + // In this case we unmount the and remount the FiberInstances.
3040 + // This might cause us to lose the selection but it's an edge case.
3041 +
3042 + // We let the previous instance remain in the "remaining queue" it is
3043 + // in to be deleted at the end since it'll have no match.
3044 +
3045 mountFiberRecursively(nextChild, traceNearestHostComponentUpdate);
3046 + // Need to mark the parent set to remount the new instance.
3047 shouldResetChildren = true;
3048 }
3049 }
@@ -3059,6 +3102,7 @@ export function attach(
3102
3103 // Returns whether closest unfiltered fiber parent needs to reset its child list.
3104 function updateFiberRecursively(
3105 + fiberInstance: null | FiberInstance, // null if this should be filtered
3106 nextFiber: Fiber,
3107 prevFiber: Fiber,
3108 traceNearestHostComponentUpdate: boolean,
@@ -3092,34 +3136,10 @@ export function attach(
3136 }
3137 }
3138
3095 - let fiberInstance: null | FiberInstance = null;
3096 - const shouldIncludeInTree = !shouldFilterFiber(nextFiber);
3097 - if (shouldIncludeInTree) {
3098 - const entry = fiberToFiberInstanceMap.get(prevFiber);
3099 - if (entry !== undefined && entry.parent === reconcilingParent) {
3100 - // Common case. Match in the same parent.
3101 - fiberInstance = entry;
3102 - // Register the new alternate in case it's not already in.
3103 - fiberToFiberInstanceMap.set(nextFiber, fiberInstance);
3104 -
3105 - // Update the Fiber so we that we always keep the current Fiber on the data.
3106 - fiberInstance.data = nextFiber;
3107 - moveChild(fiberInstance);
3108 - } else {
3109 - // It's possible for a FiberInstance to be reparented when virtual parents
3110 - // get their sequence split or change structure with the same render result.
3111 - // In this case we unmount the and remount the FiberInstances.
3112 - // This might cause us to lose the selection but it's an edge case.
3113 -
3114 - // We let the previous instance remain in the "remaining queue" it is
3115 - // in to be deleted at the end since it'll have no match.
3116 -
3117 - mountFiberRecursively(nextFiber, traceNearestHostComponentUpdate);
3118 -
3119 - // Need to mark the parent set to remount the new instance.
3120 - return true;
3121 - }
3122 -
3139 + const stashedParent = reconcilingParent;
3140 + const stashedPrevious = previouslyReconciledSibling;
3141 + const stashedRemaining = remainingReconcilingChildren;
3142 + if (fiberInstance !== null) {
3143 if (
3144 mostRecentlyInspectedElement !== null &&
3145 mostRecentlyInspectedElement.id === fiberInstance.id &&
@@ -3129,12 +3149,6 @@ export function attach(
3149 // If it is inspected again, it may need to be re-run to obtain updated hooks values.
3150 hasElementUpdatedSinceLastInspected = true;
3151 }
3132 - }
3133 -
3134 - const stashedParent = reconcilingParent;
3135 - const stashedPrevious = previouslyReconciledSibling;
3136 - const stashedRemaining = remainingReconcilingChildren;
3137 - if (fiberInstance !== null) {
3152 // Push a new DevTools instance parent while reconciling this subtree.
3153 reconcilingParent = fiberInstance;
3154 previouslyReconciledSibling = null;
@@ -3189,7 +3203,7 @@ export function attach(
3203 if (
3204 nextFallbackChildSet != null &&
3205 prevFallbackChildSet != null &&
3192 - updateFiberRecursively(
3206 + updateChildrenRecursively(
3207 nextFallbackChildSet,
3208 prevFallbackChildSet,
3209 traceNearestHostComponentUpdate,
@@ -3284,10 +3298,8 @@ export function attach(
3298 if (shouldResetChildren) {
3299 // We need to crawl the subtree for closest non-filtered Fibers
3300 // so that we can display them in a flat children set.
3287 - if (shouldIncludeInTree) {
3288 - if (reconcilingParent !== null) {
3289 - recordResetChildren(reconcilingParent);
3290 - }
3301 + if (fiberInstance !== null) {
3302 + recordResetChildren(fiberInstance);
3303 // We've handled the child order change for this Fiber.
3304 // Since it's included, there's no need to invalidate parent child order.
3305 return false;
@@ -3299,7 +3311,7 @@ export function attach(
3311 return false;
3312 }
3313 } finally {
3302 - if (shouldIncludeInTree) {
3314 + if (fiberInstance !== null) {
3315 unmountRemainingChildren();
3316 reconcilingParent = stashedParent;
3317 previouslyReconciledSibling = stashedPrevious;
@@ -3489,7 +3501,7 @@ export function attach(
3501 mountFiberRecursively(current, false);
3502 } else if (wasMounted && isMounted) {
3503 // Update an existing root.
3492 - updateFiberRecursively(current, alternate, false);
3504 + updateFiberRecursively(rootInstance, current, alternate, false);
3505 } else if (wasMounted && !isMounted) {
3506 // Unmount an existing root.
3507 removeRootPseudoKey(currentRootID);