feat(cost): add cache evidence to physical plan estimates - #353
Conversation
|
Review and fixes pushed: 701ab8b + ff1a99e, after merging main in 8067f7f and adapting to the #357 shared resource API. Findings: (1) cache hit subtraction and floating-point ceil could erase genuine misses or miscount bytes, changing final plan selection. Regressions include a nearly resident cache incorrectly reading 0 instead of 1 byte, full-width integer loss, and ordinary scan=15 / working-set=3 / capacity=2 incorrectly reading 6 instead of 5 bytes. Miss fractions now use integer uncovered bytes and integer-execution byte accounting uses checked full-width rational arithmetic. (2) missing base calibration provenance was accepted; blank versions now fail before ranking. Six new regressions were observed failing under the original logic and passing after fixes, including actual global selection. Validation: cargo test --workspace --locked (371 mapping tests); strict workspace/all-target/all-feature clippy; 25 viewer tests; format/diff checks. Normal push, no PR merge, no human-review attestation. |
|
Updated in 9e5cba5: truly rebased onto main c3410d1 (linear history), preserving all previous arithmetic and ranking regressions. CacheProfile, CacheEvidence, and CacheCapacityEvidence now have one canonical definition in asap_types::resources, with legacy mapping type paths re-exported. Cache assumptions remain distinct from additive resource consumption. Numerical validation and cost calculations stay in mapping; callers now use cache_hit_ratios(&profile, count, arrival), documented in the migration notes. Shared JSON compatibility and type-identity tests were added. Validation passed: full workspace tests --locked (372 mapping, 167 types, 20 dag_export), strict workspace Clippy across all targets/features, viewer tests (25), and formatting. Updated with an explicit force-with-lease; not merged. |
ff1a99e to
9e5cba5
Compare
|
Follow-up: split the remaining shared definitions into resources/cpu.rs (ModeledCpu and MeasuredCpu), resources/measurement.rs (Measurement), and resources/physical.rs (PhysicalResources and MeasuredResources). resources.rs retains module declarations, re-exports, and cross-resource contract tests. Existing root import paths, fields, units, defaults, and JSON formats are unchanged. Full workspace tests --locked and strict workspace Clippy across all targets/features pass. |
Why
Repeated-query caching changes the baseline cost and can change whether retained summaries are worthwhile. Addresses the physical-plan portion of #349; #345 and #346 remain separate PRs.
What and how
Add versioned result-cache and buffer-cache evidence to physical comparisons. Capacity/working-set residency and declared streaming invalidation determine result hits; result hits reduce CPU and reads, while buffer hits reduce reads only (including build-once scans). Both alternatives share immutable cache assumptions. Export demand, capacities, hit ratios, and provenance; reject aggregation across incompatible profiles. Legacy JSON defaults to named no-cache.
Before this PR
Six evaluations of a 100-row, 1,000-byte scan cost 600 CPU operations and 6,000 scan bytes, regardless of identical repeats.
After this PR
Two distinct evaluations plus four identical repeats with a resident result cache cost 200 CPU operations and 2,000 scan bytes. With 50% buffer residency, reads fall to 1,000 bytes while CPU stays at 200. These hand-computed estimates are executable tests, not runtime measurements.
Verification
cargo test --workspace --locked, including current main's test(promql): verify TopK planning and numeric reference results #351 tests.cargo clippy --workspace --all-targets --all-features --locked -- -D warnings, formatting, and diff checks.Architectural decisions
Cache evidence belongs to the immutable deployment snapshot, not logical operators. Shared annotations carry profile identity without depending on estimator types. Legacy non-cache models retain their existing aggregation; known and missing profiles cannot be mixed.
Limitations and follow-up
This is ready for human review of the scoped physical-plan implementation; it does not close all of #349. The summary-maintenance lifecycle adapter still uses the compatibility no-cache estimator. Residency is a steady-state heuristic without cold-start ordering, replacement policy, or lookup/fill overhead. Streaming invalidation is supplied, not derived from arrival rate and query schedule. Rust snapshot/request constructors require the new fields. Storage request counts and boundary bytes are separate work.
Screenshots and empirical runtime benchmarks: not applicable to the claims here; none were produced.
Human review — do not complete with an agent