This repository was archived by the owner on May 14, 2026. It is now read-only.
Conversation
…ssion Replaces the per-key SELECT loop in `prefetch_cas_paths` with a single batched `SELECT key, data FROM package_index WHERE key IN (?, ?, …)` chunked at 999 placeholders. SQLite walks the PK B-tree once per chunk instead of once per key, collapsing N round-trips across the SQLite mutex into one and dropping the ~50 ms of all-miss query overhead the empty-store install paid after #292. Closes #294.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #298 +/- ##
==========================================
+ Coverage 88.76% 88.94% +0.17%
==========================================
Files 61 61
Lines 5698 5806 +108
==========================================
+ Hits 5058 5164 +106
- Misses 640 642 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Micro-Benchmark ResultsLinux |
Integrated-Benchmark Report (Linux)Scenario: Frozen Lockfile
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 3.5108760668000003,
"stddev": 0.8147367971723012,
"median": 3.1775408147,
"user": 1.9872251199999997,
"system": 2.70889966,
"min": 2.9938078621999997,
"max": 5.6620812162,
"times": [
3.3057781872,
3.0174286572,
3.1228755212,
3.1295406002,
3.8776073132,
3.7413318481999998,
5.6620812162,
3.0327684332,
3.2255410292,
2.9938078621999997
]
},
{
"command": "pacquet@main",
"mean": 3.1728979311999996,
"stddev": 0.30058582109790166,
"median": 3.0448236322,
"user": 2.0012098199999997,
"system": 2.6668003600000008,
"min": 2.8463012892,
"max": 3.7313833652,
"times": [
2.9952961652,
3.6766630542,
2.9888498971999997,
2.8463012892,
3.1489100592,
3.0670753522,
2.9906450502,
3.2612831671999998,
3.0225719122,
3.7313833652
]
},
{
"command": "pnpm",
"mean": 5.749281026599999,
"stddev": 0.2806855154925967,
"median": 5.6476566922,
"user": 6.686728219999999,
"system": 3.31096196,
"min": 5.4585686042,
"max": 6.2274436832,
"times": [
5.9138554492,
5.5170335462,
5.4719221512,
5.6894636752,
6.0882047172,
6.2274436832,
5.9823768112,
5.6058497092,
5.4585686042,
5.5380919192
]
}
]
}Scenario: Frozen Lockfile (Hot Cache)
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.0386760451400001,
"stddev": 0.16057293996373895,
"median": 0.97284898474,
"user": 0.20299773999999998,
"system": 1.23595898,
"min": 0.92613907774,
"max": 1.45293447674,
"times": [
1.09305889674,
0.9921977107400001,
1.13358053374,
0.9418974517400001,
0.9790742317400001,
1.45293447674,
0.95502006374,
0.92613907774,
0.96662373774,
0.94623427074
]
},
{
"command": "pacquet@main",
"mean": 1.16996249704,
"stddev": 0.2865548191899628,
"median": 1.0595974207399999,
"user": 0.20226704,
"system": 1.2412922800000001,
"min": 0.9467806857400001,
"max": 1.84874050274,
"times": [
1.03727110674,
1.16499519974,
1.4608752817400001,
0.99262435774,
0.97283908174,
0.95995748274,
1.23361753674,
1.84874050274,
1.08192373474,
0.9467806857400001
]
},
{
"command": "pnpm",
"mean": 2.93641609054,
"stddev": 0.6255359854453874,
"median": 2.77788871624,
"user": 2.21510464,
"system": 1.6282113799999998,
"min": 2.2618145227400004,
"max": 3.89070011574,
"times": [
2.77249293274,
3.65401254674,
3.89070011574,
2.47868372974,
2.32433542874,
2.2618145227400004,
2.2713350127400003,
3.4005507457400004,
3.5269513707400004,
2.78328449974
]
}
]
} |
Contributor
There was a problem hiding this comment.
Pull request overview
Improves cold-cache install performance by batching store-index prefetch reads into chunked IN (...) queries to avoid per-key SQLite round-trips and mutex contention.
Changes:
- Added
StoreIndex::get_manyto fetch many index rows via chunkedSELECT key, data ... WHERE key IN (...). - Updated
prefetch_cas_pathsto useget_manyinstead of loopinggetper key. - Added unit tests for
get_manycovering empty/miss/hit/mixed/undecodable/chunking scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/tarball/src/lib.rs | Switches prefetch Phase 1 from per-key reads to a batched get_many call with fallback on error. |
| crates/store-dir/src/store_index.rs | Implements StoreIndex::get_many with chunking and adds targeted unit tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- prefetch_cas_paths: drop the `into_iter().collect()` that ran under the SQLite mutex; return the `HashMap` directly from the guard scope and let Phase 2 iterate it. Saves an O(n) copy inside the lock. - Test doc: clarify that `get_many` skips undecodable rows but emits a `debug!` log for them — earlier wording said "silently".
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
anonrig
approved these changes
Apr 25, 2026
KSXGitHub
reviewed
Apr 26, 2026
KSXGitHub flagged the `format!("… IN ({placeholders})")` in #298 as
visually scary. It's safe — only the `?,?,…?` placeholder string is
interpolated, key values flow through `params_from_iter` — but a
comment makes the invariant explicit so future readers don't have to
re-derive it.
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.
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.
Summary
Closes #294.
Replaces the per-key SELECT loop in
prefetch_cas_pathswith a single batchedSELECT key, data FROM package_index WHERE key IN (?, ?, …), chunked at 999 placeholders. SQLite walks thepackage_indexPK B-tree once per chunk instead of once per key, collapsing N round-trips across the SQLite mutex into one query.This drops the ~50 ms of all-miss query overhead the empty-store install paid after #292 introduced the install-wide prefetch — the cold-cache regression the issue reports.
Changes
StoreIndex::get_many(&[String]) -> Result<HashMap<String, PackageFilesIndex>, StoreIndexError>— batched lookup, chunked atGET_MANY_CHUNK = 999(safe vs. legacySQLITE_MAX_VARIABLE_NUMBERbuilds; rusqlite's bundled SQLite caps at 32766).prefetch_cas_pathsPhase 1 now callsguard.get_many(&cache_keys)instead of loopingguard.get(&cache_key)per key.Behaviour notes
debug!and skipped, matchingload_cached_cas_paths's.ok()?stance on the per-key path — a malformed row is a cache miss, not a batch failure.StoreIndexError::Read;prefetch_cas_pathsalready turns that into a soft fallback (per-snapshot lookups).Test plan
just readyclean (220 tests pass, 2 skipped; lint clean).crates/store-dir/src/store_index.rs:.ok()?)GET_MANY_CHUNKto exercise chunking