ENH: benchmark the in-memory lookups on hot paths - #782
Conversation
The existing benchmarks are end-to-end and disk-bound, so a change worth microseconds per lookup disappears into their noise: a regression that made resolving an already-cached remote file 12us slower moved test_scan by about 0.4%, well inside run-to-run variation. Adds small in-memory benchmarks for the lookups those paths repeat: format resolution once plugins are loaded, resolving a cached remote file, IO handle lookup within one operation, and repeat spool access. Each repeats enough times to do a few milliseconds of work. They resolve changes at the intended scale: across the registry synchronization in #779, test_load_plugins_already_loaded moves from 0.02s to under the 0.005s reporting floor.
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #782 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 164 164
Lines 17589 17589
=========================================
Hits 17589 17589
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will not alter performance
Performance Changes
Comparing |
Description
Adds micro-benchmarks for the in-memory lookups DASCore repeats on its hot paths.
The existing benchmarks are all end-to-end and disk-bound, which makes them blind at this scale. A concrete example from the free-threading series: replacing the memo on
_materialize_remote_filewith locking alone made resolving an already-cached remote file 12.2 µs → 35.1 µs, a 2.9x regression on every remote file open. Intest_scan, which opens about 40 files, that is roughly 1 ms on a 240 ms benchmark — 0.4%, comfortably inside run-to-run noise. It was found by hand, and only because someone asked the right question.Three regressions in that series had the same shape (a cache removed in favor of a lock), and none of them were visible to CI.
What is added
benchmarks/test_lookup_benchmarks.py, covering the lookups those paths make:load_plugins()no-op, resolution by extension, and resolution by known format/version.ensure_local_fileon a path which is already local.Each repeats its lookup enough times to do a few milliseconds of work, so the measurement is not dominated by setup. The counts differ per benchmark because the operations differ in cost by two orders of magnitude — from about 0.05 µs for the
load_pluginsno-op to about 13 µs for the remote-cache resolution.Do they actually resolve changes at this scale?
Yes — checked against a change already merged. Running this file at
1183dbcc(before #779) and at currentdev:test_load_plugins_already_loadedtest_yield_fiberio_by_formatThose match the 4x and 1.3x improvements measured by hand when #779 landed. CodSpeed counts instructions rather than wall time, so its resolution is better still.
Notes
benchmarks/changes.benchmarklabel to run CodSpeed on a PR; that gate is unchanged.set_configtier, matching the existing convention for class- and module-scoped benchmark fixtures.Changelog
none
Checklist
I have (if applicable):