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

Optimize createElement #27271

Closed
wants to merge 1 commit into from
Closed

Optimize createElement #27271

wants to merge 1 commit into from

Conversation

zackasaurus
Copy link

@zackasaurus zackasaurus commented Aug 22, 2023

Summary

I created this PR in response to the #27216 issue I opened.

How did you test this change?

I created a sandbox which renders a large React DOM tree with a configurable number of elements and props. These are broken up by LIST, DEPTH, and PROPS. I used the settings LIST = 500, DEPTH = 20, PROPS = 30 to emulate a complex react app and saw a 30-40% reduction in script.

For benchmarking, I used the production version of both the current 18.2.0 createElement and the production-built custom version of the modified createElement as seen in the PR.

Sandbox link:

https://codesandbox.io/s/react-create-element-performance-comparison-tpp5cl?file=/src/App.js

What the UI looks like:

image

What the F12 Performance Trace looks like:

image

@zackasaurus zackasaurus changed the title add react element changes Optimize createElement Aug 22, 2023
@V01D-NULL
Copy link

any updates on this?

@zackasaurus
Copy link
Author

See: reactjs/rfcs#107

@V01D-NULL
Copy link

thank you

@sophiebits
Copy link
Collaborator

Wow! This is a big improvement. It is hard to know how this will translate to a real app since the engine performance characteristics will be different for this case with only a few component types and props shapes vs a real app with many types of components and many shapes of props. But hopefully we can ship all the changes in reactjs/rfcs#107 which would have even a bigger impact than this.

Copy link

github-actions bot commented Apr 7, 2024

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Apr 7, 2024
@kurtextrem
Copy link

this still looks like something worth merging, so "bump" from my side.

@github-actions github-actions bot removed the Resolution: Stale Automatically closed due to inactivity label Apr 10, 2024
@kurtextrem
Copy link

actually, looks like this could be closed because of #28768

@sophiebits
Copy link
Collaborator

@zackasaurus Thanks for sending this in! This helped kick-start the work to get #28768 and friends landed for real. Hopefully you will see even larger gains from that set of changes once they’re released (note they’re still behind a feature flag on main for now).

@sophiebits sophiebits closed this Apr 11, 2024
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