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

perf(utils): use WeakSet to track object references #5791

Conversation

vanyaxk
Copy link
Contributor

@vanyaxk vanyaxk commented Oct 23, 2023

This change removes Set in favour of WeakSet to track seen objects while cloning for performance reasons

From MDN:
The number of objects or their traversal order is immaterial, so a WeakSet is more suitable (and performant) than a Set for tracking object references, especially if a very large number of objects is involved.

@avelad avelad added type: performance A performance issue priority: P1 Big impact or workaround impractical; resolve before feature release labels Oct 23, 2023
@avelad avelad added this to the v4.6 milestone Oct 23, 2023
@avelad avelad requested review from avelad and theodab October 23, 2023 08:05
@shaka-bot
Copy link
Collaborator

Incremental code coverage: 100.00%

@avelad avelad merged commit daa9d1f into shaka-project:main Oct 23, 2023
14 of 15 checks passed
Robloche pushed a commit to Robloche/shaka-player that referenced this pull request Nov 30, 2023
This change removes `Set` in favour of `WeakSet` to track seen objects
while cloning for performance reasons

From MDN:
`
The number of objects or their traversal order is immaterial, so a
WeakSet is more suitable (and performant) than a Set for tracking object
references, especially if a very large number of objects is involved.
`

---------

Co-authored-by: Ivan Kohut <ivan.kohut@lamin.ar>
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Dec 22, 2023
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Dec 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: P1 Big impact or workaround impractical; resolve before feature release status: archived Archived and locked; will not be updated type: performance A performance issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants