Send only changed props on Insert of preallocated views with accumulated raw props - #58770
Open
bartlomiejbloniarz wants to merge 1 commit into
Open
bartlomiejbloniarz wants to merge 1 commit into
bartlomiejbloniarz wants to merge 1 commit into
Conversation
…ted raw props
With enableAccumulatedUpdatesInRawPropsAndroid, every Insert emits
UpdateProps({}, new) and sends the view's full props again, although the view
already received them at preallocation or from its Create item. So every
inserted view's props are serialized and applied twice. Under the pull model
both happen on the UI thread. With Props 2.0 the second payload is diffed
against default props, so a prop reset to its default between preallocation
and mount is never sent.
Behind the new enablePreallocatedPropsDiffOnInsertAndroid flag, the allocated
view registry keeps the props a view was preallocated with until its first
Insert. That Insert sends the difference between the preallocated and inserted
props, and nothing for views created by a Create item.
|
@bartlomiejbloniarz has imported this pull request. If you are a Meta employee, you can view this in D122571252. |
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
With
enableAccumulatedUpdatesInRawPropsAndroid,FabricMountingManagerhandles every Insert by emittingUpdateProps({}, new), which sends the view's full props again. The view already received those props at preallocation or from its Create item. So every inserted view's props are serialized and applied twice. The pull model (enableMountingCoordinatorPullModelAndroid) requires accumulated raw props and preallocates new views before the UI thread pulls the transaction, so under the pull model both happen on the UI thread.For components with a Props 2.0 diff (
View, withenablePropsUpdateReconciliationAndroid), this is also a correctness bug. The second payload is diffed against default props. If a prop goes back to its default between preallocation and mount, it is never sent, and the view keeps the preallocated value.This PR adds
enablePreallocatedPropsDiffOnInsertAndroid. It is off by default and only takes effect together with accumulated raw props.In debug builds, Inserts of preallocated views now go through the Props 1.5 vs Props 2.0 validation in
getProps. It can log props that Props 2.0 does not cover yet.The flag keeps the change separately testable while
enableAccumulatedUpdatesInRawPropsAndroidis being evaluated. The intent is to fold it into that flag once validated. I'm happy to make it unconditional now if that is preferred.Changelog:
[Internal] - Add
enablePreallocatedPropsDiffOnInsertAndroid, which stops re-sending all props on Insert of preallocated views whenenableAccumulatedUpdatesInRawPropsAndroidis onTest Plan:
yarn featureflags --verify-unchangedandyarn test packages/react-native/src/private/featureflagspass.yarn format-check-cpppasses.FabricMountingManager.cppcompiles with the ReactAndroid arm64-v8a NDK flags. I did not run a full RNTester Android build of this exact revision.The runtime checks below used an earlier revision of this change with identical behaviour. The build also contained unrelated local fixes, present in both arms.
Emulator, pull model and Props 2.0 flags on: a view whose
backgroundColorandopacityare reset to their defaults between preallocation and mount keeps the old values with the flag off. With the flag on, it shows the defaults.OPPO A16 (low-end Android), release build, pull model and Props 2.0 flags on. The benchmark is a native screen that opens a React Native surface with placeholder cards, then a list of 24 cards. "First render" is the first React Native frame drawn. "Content drawn" is the first frame with the list. Flag on vs off in the same build:
Paired within-round differences, 35 pairs: first render −20.6 ms [95% CI −26.8, −15.0], content drawn −22.0 ms [−43.4, −18.4]. Arms were interleaved.