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

Change proposal for over 20x speedup in findManyByIds #84

Open
gamebak opened this issue Oct 4, 2021 · 3 comments
Open

Change proposal for over 20x speedup in findManyByIds #84

gamebak opened this issue Oct 4, 2021 · 3 comments
Labels
help wanted Extra attention is needed needs reproduction

Comments

@gamebak
Copy link

gamebak commented Oct 4, 2021

Speed up proposal for method findManyByIds.

return Promise.all(ids.map(id => methods.findOneById(id, { ttl })))

As-Is right now, it's opening a lot of findOneById with a promise.all, but it's really inefficient once you have over 100 keys and it's slowing the UI.

While I don't exactly know how to implement this in the library exactly, the solution will look similar to this at the mongo level:

 this.collection.find({ _id: { $in: arrIds } })

This will result in a single call to the server-side, speeding up for bigger queries.

@0xChqrles
Copy link

@gamebak In the end Dataloader only makes a single request with all the ids passed

@gamebak
Copy link
Author

gamebak commented Feb 15, 2022

It doesn't, I've tested this, unless I'm missing something.

@lorensr
Copy link
Member

lorensr commented Mar 31, 2022

It's certainly the intent of the DataLoader function to use $in:

if (!filter[fieldName]) filter[fieldName] = { $in: [] }

If it's not working, would be happy to look at a PR that fixes it.

@lorensr lorensr added help wanted Extra attention is needed needs reproduction labels Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed needs reproduction
Projects
None yet
Development

No branches or pull requests

3 participants