-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
feat(reactivity): ref-specific track/trigger and miscellaneous optimizations #3995
Conversation
I noticed this PR no longer use a class/prototype implementation for effects - does this mean this PR doesn't affect memory consumption like #2345 did? |
Ran the benchmarks myself and indeed seeing same improvements - the watch creation seems to have to do with the ref read. Although I'm still amazed that simple |
I don't think it got faster. In theory, the changes in this PR shouldn't affect it at all. That particular benchmark just gives very different result every run for some reason..
That's correct. I left them out, because the memory consumption changes require a breaking API change for the reactivity module. These were roughly the optimizations that could be done:
Notice that most gain can be obtained from 1-3, and these only affect the arguments in the Are you prepared to change the reactivity API at all? And to what extent? |
I actually tried to avoid set add/removes and sync them after the effect (https://github.com/Planning-nl/vue-next/commit/f8cc160aed8c581662629904457f4a87437e62d5). In most tests, performance is boosted by ~10 to 20%. However, some tests fail because it seems that, in some cases, synchronized behavior from within the effect require the dependencies (Set) to be up to date at any time. When postponing dep registration until after the track op has finished, it seems that some use cases aren't properly covered. Any way, imho the additional complexity is too much and the performance gains too minimal to actually implement this. |
New PR created, see #2345.