[pull] canary from vercel:canary - #1351
Merged
Merged
Conversation
## What Key blocks for short keys (≤ 32 bytes) don't store a per-entry hash, but were still sorted by `(hash, key)`. That forced every binary-search probe to recompute the entry's xxh3 hash just to compare it — roughly ten hashes per lookup. These blocks now store their entries in **key order**, so the search compares key bytes directly and hashes nothing. All the families `turbo-tasks-backend` produces hit this case, so this is the common path in practice. ## Why this is safe Hash-based routing is unchanged. Files are still assigned by hash, the index block still routes to a key block by hash, and `min_hash`/`max_hash`, the AMQF, and compaction's coverage model all work exactly as before. Thus only the order *within* a hash-less block changes. This adds a small cost to writing, which now must re-sort blocks of keys and a double cost to compaction which now must re-sort blocks in hash order to iterate them, and then sort them back to key order when writing the new file. ## Benchmarks Measured against `canary`, baseline and comparison run back to back on the same machine. **Lookups ** All 16 `static_sorted_file_lookup` configurations improved | entries | hit/uncached | hit/cached | miss/uncached | miss/cached | | --- | --- | --- | --- | --- | | 1 Ki | -30.4% | -24.8% | -29.1% | -29.4% | | 10 Ki | -25.2% | -19.7% | -22.7% | -23.7% | | 100 Ki | -17.3% | -19.3% | -15.2% | -16.8% | | 1000 Ki | -7.0% | -25.7% | -7.0% | -10.9% | **Commits ** Short keys pay for the added per-block sort. | config | change | | --- | --- | | `key_8` 85Ki entries | +3.3% | | `key_8` 853Ki entries | +3.1% | | `key_8` 8.33Mi entries | +8.5% | | `key_32Ki` / `key_4` (large-key configs) | -2.6% to -19.1% | Six of thirteen write configurations came out as noise (p >= 0.05) and are omitted. **Compaction** `StaticSortedFileIter` must yield `(hash, key)` order because `MergeIter` merges on it, so it reorders each hash-less block back into hash order. Plus the additional cost of the commit (above) | config | change | | --- | --- | | 4Mi entries / 8 commits | +6.0% (reproduced at +10.2%) | | 16Mi entries / 8 commits | +7.5% | | 4Mi entries / 32 commits | +7.8% | | 16Mi entries / 32 commits | +5.2% | | 16Mi entries / 128 commits | -9.5% |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )