perf: make dataloader doc placement O(1) with a key-index map#17469
Open
yoo-minho wants to merge 1 commit into
Open
perf: make dataloader doc placement O(1) with a key-index map#17469yoo-minho wants to merge 1 commit into
yoo-minho wants to merge 1 commit into
Conversation
The dataloader placed each returned doc by scanning the full keys array with findIndex, once per doc — O(n²) over the batch size. Build a key->index Map once (keys is fixed for the batch) and look up in O(1), making it O(n). First occurrence wins, matching findIndex.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Replaces the per-doc
keys.findIndex(...)scan in the collections dataloader with a singleMap<string, number>lookup, turning the doc-placement loop from O(n²) into O(n) over the batch size.Why?
batchLoadFnreceives all cache keys requested in one tick (keys: readonly string[]). After the batchedfindreturns, the current code places each doc back into the result array by scanning the entirekeysarray withfindIndex, once per doc:Total docs across all batches ≈
keys.length, and each placement scans all ofkeys, so the work isO(keys.length²).keysgrows with the number of documents loaded in a single tick — which, with depth-populated relations, can be hundreds or thousands — so this is quadratic on a hot batching path.How?
keysis fixed for the batch, so I build akey → indexmap once up front and look up in O(1):Behaviour is unchanged:
findIndexreturns the first match, and the map is built first-occurrence-wins to preserve that. Keys are unique per load in practice, so the guard is just defensive.Targeting
main(v4) as an enhancement, per the PR template.Fixes #