[IMP] reactivity: replace sets with small arrays for performance #1581
Conversation
This file contains 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
While Sets have better lookup complexity than arrays, because of the large constant factors, small arrays can perform better than small sets when checking for inclusion. In practice, replacing both of the raw types sets with arrays can improve performance of reactive-heavy workloads by as much as 30%. Considering the reactivity code is very hot when rendering data-heavy components, and the low impact on readability of the fix, the cost-benefit analysis is clearly in favour of making the fix.
sdegueldre
force-pushed
the
master-reactivity-perf-imp-sad
branch
from
7852758
to
489f1a3
Compare
aab-odoo
approved these changes
Jan 12, 2024
ged-odoo
reviewed
Jan 14, 2024
| @@ -45,7 +46,7 @@ function canBeMadeReactive(value: any): boolean { | |||
| if (typeof value !== "object") { | |||
| return false; | |||
| } | |||
| return SUPPORTED_RAW_TYPES.has(rawType(value)); | |||
| return SUPPORTED_RAW_TYPES.includes(rawType(value)); | |||
There was a problem hiding this comment.
at this point, maybe it's even faster to simply check for equality:
const type = rawType(value);
return type === "Object" || type === "Array" || type === "Set" || type === "Map" || type === "WeakMap";Same for collections. what do you think? @sdegueldre
There was a problem hiding this comment.
I tried with an inline switch previously when experimenting but perf was about the same, I'm guessing this is already very easy to JIT efficiently but may require more exploration.
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.
While Sets have better lookup complexity than arrays, because of the large constant factors, small arrays can perform better than small sets when checking for inclusion.
In practice, replacing both of the raw types sets with arrays can improve performance of reactive-heavy workloads by as much as 30%.
Considering the reactivity code is very hot when rendering data-heavy components, and the low impact on readability of the fix, the cost-benefit analysis is clearly in favour of making the fix.