-
Notifications
You must be signed in to change notification settings - Fork 257
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
Use a Set
for referencers
#3056
Conversation
When working with elements with many referencers, `addReferencer()` became very hot. If you have eg. 100 referencers, each one is added one by one, and each new addition iterates the entire existing referencers list. This could cause quadratic performance. This changes the `_referencers` values to be JS `Set`s. This maintains the same order for algorithms that rely on order.
|
✅ Deploy Preview for apollo-federation-docs canceled.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
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.
Approved.
Follow up question though -> should we also convert other array usages to sets? unionsWhereMember
/allImplementations
etc?
It actually might be good to convert |
When working with elements with many referencers,
addReferencer()
became very hot.If you have eg. 100 referencers, each one is added one by one, and each new addition iterates the entire existing referencers list. This could cause quadratic performance.
This changes the
_referencers
values to be JSSet
s. This maintains the same order for algorithms that rely on order.