feat(prefetch): rank candidates by cost and urgency, cap weak sources (#616, #617) - #640
feat(prefetch): rank candidates by cost and urgency, cap weak sources (#616, #617)#640jleni wants to merge 3 commits into
Conversation
|
Pushed f9f6686: workspace bump to v0.13.0.
Packaging resolves That is not restructurable. The ranking metadata originates in the bin's data sources (store index rows, shard entries) and has to reach the planner through the shared 0.13.0 rather than 0.12.1 because kache-core gains public API. With 0.13.0 unpublished the packaging step takes its documented "not on crates.io yet" branch and skips, which is the state the tree normally sits in between a bump and its release. All four manifests plus the dep pin move together so Flagging one thing for review: this folds a release-number bump into a feature PR, whereas the convention has been standalone |
27cebb5 to
bcea4af
Compare
…#616, #617) Closes #616. Closes #617. Stage 1 (#633) bounded a plan's RESOURCE use. This bounds its composition and decides what order the survivors are fetched in. monolithic-manifest path sorted by compile_time_ms and filtered cheap crates. Candidates now carry the metadata to rank by, and every source tags what it produced: - PrefetchCandidate gains compile_time_ms, size_bytes, source, and demand_index, all Option/defaulted so an old planner's candidates still parse and a new plan still reaches an old daemon. - ShardEntry gains compile_time_ms and artifact_size; save-manifest already had both from ManifestEntry and simply was not persisting them. Old shards in S3 parse as None. - keys_for_crates selects compile_time_ms and size, returning a named CrateHistoryEntry instead of a bare tuple. Unknown is never zero. Both store columns default to 0 for rows written before their migrations, so a 0 is mapped to None: read as a measurement it would rank an un-backfilled entry as free to fetch and worthless to have. Dispatch order is lexicographic, not a weighted score, because a weighted sum needs coefficients and nothing can calibrate them until #618 makes "arrived before it was demanded" measurable: 1. urgency bucket, because prefetch races the build and a costly artifact needed at minute eight loses to a cheap one needed at second five 2. source confidence 3. value descending, unknown cost last Demand position is bucketed (width 16, about one download wave) rather than used exactly: it comes from a guppy traversal, which only approximates when cargo asks, so position 40 vs 45 is false precision while 40 vs 400 is real. different job from the daemon's byte/key/time budgets. Those bound resource use and are the trust boundary; these stop the weakest source crowding out better candidates before the budget is even reached. The key cache maps a crate NAME to every key in the bucket with no target, toolchain, profile, or feature dimension, so of n variants at most one can be right: default 2 per crate, 64 per plan, plus 2 history entries per crate. Shards get no per-crate cap, because one crate legitimately has several compile units. Drops are counted per class and logged, so a trimmed plan is distinguishable from one that had nothing more to offer. Mutation-driven test work: the first pass left 9 surviving mutants in the new code, including the per-crate history cap, which had no test at all. cargo-mutants now reports 52 caught, 0 missed for kache-core. Notably cap_to's "-" mutated to "/" survived because the original test used 4 items capped to 2, where both give the same answer. The staging in notes/design/prefetch-budgets-and-ranking.md is revised there: stages 2 to 4 all rewrite build_prefetch_plan, the PlannerDataSource surface, and fallback_planner.rs, so splitting them meant changing the same functions three times, and the metadata stage alone would have landed wire fields no code reads.
Required by the API this PR adds, not cosmetic. `Check (Linux)` runs `cargo package -p kache --locked` whenever the manifest's kache-core version is already on crates.io. Packaging resolves kache-core from the REGISTRY rather than the workspace path, so the bin is compiled against published kache-core 0.12.0 — which has none of CandidateSource, PlanLimits, build_prefetch_plan_with_limits, or the new PrefetchCandidate fields. Five E0433/E0425/E0560 errors, reproduced locally. There is no way to restructure around it: the ranking metadata originates in the bin's data sources (the store's index rows, shard entries) and has to reach the planner through the shared PrefetchCandidate type, so the bin genuinely requires the newer kache-core. 0.13.0 rather than 0.12.1 because kache-core gains public API, which is a minor-version event. With 0.13.0 unpublished, the packaging step takes its documented "not on crates.io yet" branch and skips, which is the same state the tree sits in between a bump and its release. Moves all four manifests plus the kache-core dep pin together, so check-version-consistency.sh stays green (it gates kache, kache-core, and the pin as one value).
The mutation gate runs two lanes. The kache-core lane passed, which the local run had predicted, but only that lane was run locally: CI also mutates changed Rust lines across the rest of the workspace, and this change touches five files there. Nine mutants survived. Six were in `positive_or_none`, the four-line helper that decides a non-positive SQLite column means "not recorded" rather than a measured zero. It is the function implementing the "unknown is never zero" rule this PR argues for at length, and it had no direct test, so replacing its whole body with `Some(0)` — precisely the bug it exists to prevent — went unnoticed. Now covered over 0, negative, 1, a real value, and i64::MAX. The other three were the `dropped_total() > 0` guard on the composition log. Extracted to a named `should_report_composition` predicate and tested: whether to report is a real decision (do not log when nothing was dropped, never hide a truncation), so it deserves a name rather than an untestable branch inside a tracing call. Local diff lane: 14 caught, 0 missed, 4 unviable.
bcea4af to
cc0cf0e
Compare
Closes #616. Closes #617.
Stage 1 (#633) bounded a plan's resource use: keys, bytes, deadline. This bounds its
composition, and decides what order the survivors are fetched in.
#617: ranking
The plan path applied no cost weighting at all, while the older monolithic-manifest path
sorted by
compile_time_msand filtered crates cheaper to rebuild than to fetch. The newerpath had simply dropped a signal the older one already had.
Candidates now carry what ranking needs, and every source tags what it produced:
PrefetchCandidategainscompile_time_ms,size_bytes,source,demand_index. AllOption/defaulted, so an old planner's candidates still parse and a new plan still reachesan old daemon.
ShardEntrygainscompile_time_msandartifact_size.save-manifestalready had bothfrom
ManifestEntryand was discarding them at write time. Old shards in S3 parse asNone.keys_for_cratesselects the two columns it was ignoring and returns a namedCrateHistoryEntryinstead of a bare(String, String, PathBuf).Unknown is never zero. Both store columns default to
0for rows written before theirmigrations. Read as a measurement, a
0would rank an un-backfilled entry as free to fetchand worthless to have, so
0maps toNone.Dispatch order is lexicographic, not a weighted score: a weighted sum needs coefficients,
and nothing can calibrate them until #618 makes "arrived before it was demanded" measurable.
loses to a cheap one needed at second five.
later bucket).
Demand position is bucketed (width 16, about one download wave) rather than used exactly. It
comes from a guppy traversal, which only approximates when cargo actually asks, so treating
position 40 and 45 as different is false precision while 40 versus 400 is real.
#616: composition caps
PlanLimitsbounds what each source may contribute. That is a different job from stage 1'sbyte/key/time budgets: those bound resource use and are the trust boundary, these stop the
weakest source crowding out better candidates before the budget is even reached.
The key cache maps a crate name to every key in the bucket, with no target, toolchain,
profile, or feature dimension, so of
nvariants at most one can be right. Defaults: 2 percrate, 64 per plan, plus 2 history entries per crate. Shards get no per-crate cap, because one
crate legitimately has several compile units.
Capping is not a fix for that source; dimensioning the remote layout is, and that needs its own
index version and migration. The design note says so rather than implying this closes it.
Drops are counted per class and logged, so a trimmed plan stays distinguishable from one that
had nothing more to offer.
Why this is one PR, not three
The design note staged this as 2, 3, 4. I revised that in the note. All three rewrite
build_prefetch_plan, thePlannerDataSourcesurface, andfallback_planner.rs, so splittingmeant changing the same functions three times over two rebases. And the metadata stage alone
would have landed wire fields no code reads, which a reviewer cannot judge without seeing the
ranking that consumes them. The separation that carried its weight was enforcement from
selection; going finer than that did not.
The mutation gate found real gaps
kache-coreis under complete mutation coverage as of #634. My first test pass looked fine at24 green, but 9 mutants survived, including the per-crate history cap, which had no test
at all.
The instructive one:
cap_to's-mutated to/survived because the test used 4 itemscapped to 2, where subtraction and division both give 2. The test asserted the right value for
the wrong reason.
After targeted tests:
cargo-mutantsreports 52 caught, 0 missed forkache-core(was43 caught, 9 missed). Run locally with cargo-mutants 27.0.0; CI pins 27.1.0.
Verification
cargo clippy --workspace --all-targets -- -D warnings(the realjust lintgate): cleancargo fmt --all --check: cleanOne local e2e failure,
clone_worktrees_pull_makes_both_commits_checkoutable, is my own gitcommit-signing config breaking that test's temp-repo commits. It passes with
GIT_CONFIG_GLOBAL=/dev/null, touches no prefetch code, and CI runners have no signing key.