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
Perf set changing prop #32574
Perf set changing prop #32574
Conversation
|
f1cca3a
to
db8701e
Compare
While determining a property name to bind to we were checking a mapping object resulting in the megamorphic read. Replacing such read with a series of if checks speeds up rproprty update benchmark ~30% (~1400ms down to ~1000ms).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* | ||
* Performance note: this function is written as a series of if checks (instead of, say, a property | ||
* object lookup) for performance reasons - the series of `if` checks seems to be the fastest way of | ||
* mapping property names. Do NOT change without benchmarking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
'tabindex': 'tabIndex', | ||
}; | ||
function mapPropName(name: string): string { | ||
if (name === 'class') return 'className'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could make this even faster if we did something like this:
const ch = name.charCodeAt(0);
if (ch === 99 /*c*/ || ch === 102 /*f*/' || ch === 105 /*i*/ || ch === 116 /*t*/ || ch === 't') {
...
}
While determining a property name to bind to we were checking a mapping object resulting in the megamorphic read. Replacing such read with a series of if checks speeds up rproprty update benchmark ~30% (~1400ms down to ~1000ms). PR Close #32574
…2574) While determining a property name to bind to we were checking a mapping object resulting in the megamorphic read. Replacing such read with a series of if checks speeds up rproprty update benchmark ~30% (~1400ms down to ~1000ms). PR Close angular#32574
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This PR introduces a new benchmark that focuses on performance of property updates. It also contain a fix where by removing a megamorphic access we can speed up this new benchmark by ~30%.
Review commit by commit and check individual commit message for more details.