Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce a faster version of the addProperties function #28969

Merged
merged 3 commits into from
May 2, 2024

Conversation

dmytrorykun
Copy link
Contributor

Summary

This PR introduces a faster version of the addProperties function. This new function is basically the diffProperties with prevProps set to null, propagated constants, and all the unreachable code paths collapsed.

How did you test this change?

I've tested this change with the benchmark app and got ~4.4% improvement in the view creation time.

@react-sizebot
Copy link

react-sizebot commented May 2, 2024

Comparing: 4508873...ab21f13

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.66 kB 6.66 kB = 1.82 kB 1.82 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB = 1.83 kB 1.83 kB
facebook-www/ReactDOM-prod.classic.js = 591.11 kB 591.11 kB = 103.94 kB 103.94 kB
facebook-www/ReactDOM-prod.modern.js = 567.33 kB 567.33 kB = 100.34 kB 100.34 kB
test_utils/ReactAllWarnings.js Deleted 64.26 kB 0.00 kB Deleted 16.02 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
react-native/implementations/ReactFabric-prod.fb.js +0.37% 366.41 kB 367.77 kB +0.23% 64.05 kB 64.20 kB
react-native/implementations/ReactFabric-profiling.fb.js +0.34% 393.71 kB 395.06 kB +0.21% 68.28 kB 68.42 kB
react-native/implementations/ReactNativeRenderer-prod.fb.js +0.34% 373.83 kB 375.10 kB +0.17% 65.39 kB 65.50 kB
react-native/implementations/ReactNativeRenderer-profiling.fb.js +0.32% 401.07 kB 402.35 kB +0.22% 69.62 kB 69.77 kB
test_utils/ReactAllWarnings.js Deleted 64.26 kB 0.00 kB Deleted 16.02 kB 0.00 kB

Generated by 🚫 dangerJS against ab21f13

@dmytrorykun dmytrorykun merged commit 73bcdfb into facebook:main May 2, 2024
37 of 38 checks passed
github-actions bot pushed a commit that referenced this pull request May 2, 2024
## Summary

This PR introduces a faster version of the `addProperties` function.
This new function is basically the `diffProperties` with `prevProps` set
to `null`, propagated constants, and all the unreachable code paths
collapsed.

## How did you test this change?

I've tested this change with [the benchmark
app](https://github.com/react-native-community/RNNewArchitectureApp/tree/new-architecture-benchmarks)
and got ~4.4% improvement in the view creation time.

DiffTrain build for [73bcdfb](73bcdfb)
github-actions bot pushed a commit that referenced this pull request May 2, 2024
## Summary

This PR introduces a faster version of the `addProperties` function.
This new function is basically the `diffProperties` with `prevProps` set
to `null`, propagated constants, and all the unreachable code paths
collapsed.

## How did you test this change?

I've tested this change with [the benchmark
app](https://github.com/react-native-community/RNNewArchitectureApp/tree/new-architecture-benchmarks)
and got ~4.4% improvement in the view creation time.

DiffTrain build for commit 73bcdfb.
@@ -35,6 +35,7 @@ export const {
favorSafetyOverHydrationPerf,
disableDefaultPropsExceptForClasses,
enableNoCloningMemoCache,
enableAddPropertiesFastPath,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you mean to do this? don’t think this code path is used on www

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. I just wanted to make it symmetric to ReactFeatureFlags.native-fb-dynamic.js - ReactFeatureFlags.native-fb.js. I'm not sure if it's actually necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants