-
-
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
perf: replace Map/Set with WeakMap/WeakSet #8549
Conversation
TBR |
❌ Deploy Preview for vue-sfc-playground failed.
|
❌ Deploy Preview for vue-next-template-explorer failed.
|
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
Size ReportBundles
Usages
|
This PR solves the same problem as #7827, but I think #7827 should be preferred as it would also clean up deps that are primitive types (strings). But maybe the WeakMap approach is more perfomant? Not sure. either way the pros and cons should be weighted before merging this. Also, johnson seems to already have included the fix from #7827 in his larger reactivity refactor of #5912 according to #9419 (comment) |
@LinusBorg #7827 addresses memory leaks in reactivity. This PR aims to replace as many Maps in the project with WeakMaps as possible. It doesn't conflict with the code in #7827, and I believe WeakMap is the best solution for the in this PR. |
Okay, seems I was too quick here. Thanks for getting back to me quickly. :) |
For the keys of object properties, we should use
WeakMap
andWeakSet
, because they will not prevent the garbage collector from reclaiming the key.