perf(test): cheap bypass read; fix(perf): identical arms only suspect on a fresh base - #1114
Conversation
`bypass_env_vars` ran `batten::config::load` over the committed authority to collect `[[rule]].bypass_env`, and `common::batten()` calls it for every fixture command. nextest runs a process per case, so the memo saved nothing across the suite: under callgrind, the load was 242M of `cli::help_leads_with_the_crate_description`'s 251M instructions at the test binary's opt-level 0. `bypass_env_vars_in` still reads the names out of the file, through a two-field serde view the deserializer skips everything else for. The same case is now 59M instructions. `bypass_scrub::the_cheap_bypass_read_names_what_the_full_load_names` holds the cheap read to the full load, on a fixture that declares a hatch (the committed config declares none, which would make the comparison vacuous) and on the committed file. `bypass-read-drops-rows` is killed by hand. Refs: CLOUD-2106
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 25 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe bypass environment reader now extracts names from rule entries without loading and validating the full configuration. It sorts and deduplicates the names, and returns an empty list if reading or parsing fails. The memoized reader uses this function for the committed configuration. A test compares its results with those from the validated configuration load. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to This change only speeds up test helper reads of bypass environment names. It adds a comparison test and shows no concrete merge risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
CLOUD-2060's guard refuses a pair whose arms are byte-identical, for one mechanism: both arms built into the shared directory in the same run, the second inheriting the first's `batten` units through cargo's mtime fingerprint. CLOUD-2097 then made the previous lap's head the next lap's base, built from the same checkout path, so a change that leaves the binary alone now yields a head identical to its kept base, legitimately. CLOUD-2106's test-only lap was refused at `perf-gate` on exactly that, after a green suite. `arms_suspect` keeps the refusal for a base built this run and lets a cached base equal to the head through to the measurement, which reads ~1.0. `perf::tests::identical_arms_are_suspect_only_when_the_base_was_built_this_run`; `identical-arms-always-suspect` is killed by hand. Refs: CLOUD-2107
wenzowski
changed the title
|
/fast-forward |
1 participant
Closes CLOUD-2106
Closes CLOUD-2107
CLOUD-2106 —
perf(test).common::batten()ran the full validatedbatten::config::loadto collect[[rule]].bypass_env. nextest runs one process per test, so every spawning case paid for that load: under callgrind, 242M of the 251M instructions incli::help_leads_with_the_crate_description.bypass_env_vars_instill reads the names from the committed file, through a two-field serde view. The same case now takes 59M instructions (−76%).bypass_scrub::the_cheap_bypass_read_names_what_the_full_load_nameschecks the cheap read against the full load, on a fixture that declares a hatch and on the committed file.bypass-read-drops-rowswas killed by hand.CLOUD-2107 —
fix(perf). This fixes a regression from CLOUD-2097. The perf gate's identical-arms guard (CLOUD-2060) refused CLOUD-2106's test-only lap after a green suite. The base arm was now the previous lap's kept head, built from the same checkout path, and the binary had legitimately not changed.arms_suspectkeeps the refusal only when this run built the base, which is the CLOUD-2060 mechanism's precondition. A cached base that matches the head goes through to the measurement, which reads about 1.0.identical_arms_are_suspect_only_when_the_base_was_built_this_run; mutantidentical-arms-always-suspect, killed by hand.All 29 perf unit tests and all 12
perf_paircases pass.🤖 Generated with Claude Code
https://claude.ai/code/session_01SFeJfGnT67NjjLWQJ1SgXB