mirror of
https://github.com/rust-lang/rust.git
synced 2026-07-11 02:01:14 -04:00
Jacob Pratt
5d9e984267
[perf] Add explicit `Iterator::count` impl for `ChunkBy`. Prior to this PR, `.chunk_by(..).count()` constructs were using the default `Iterator::count` implementation for `ChunkBy`, which was based on `Iterator::next()`. We can do noticeably better by writing a dedicated implementation, as measured [at this real-world call site in `omnibor-rs`](https://github.com/omnibor/omnibor-rs/blob/116c5c2281219dece9da4eddc7d2e2c47e4c81c4/omnibor/src/gitoid/internal.rs#L186). A local benchmark on my macOS laptop shows that using `omnibor-rs`'s public API to invoke `ArtifactId::sha256()` in a tight loop, with appropriate black-boxing, experiences a 5-7% CPU time win with the dedicated implementation. Of course the benchmark is sensitive to the exact length and contents of the file, hence the 5-7% range. But the 95% confidence interval for the amount of improvement in every benchmark I ran is <1% wide, so I am quite confident this is an improvement. I also extracted the relevant `omnibor-rs` use of `.chunk_by(..).count()` into a microbenchmark, which shows a 2.1-2.4x throughput improvement (-51.5-57.7% CPU time) with the new implementation. Disassembly of the baseline vs new implementation shows that the optimizer was able to vectorize the new implementation, while the baseline was not vectorized. **AI disclosure:** The optimization opportunity here was discovered as part of a systematic probe for missed optimizations utilizing both traditional and AI tools. The code here was initially prototyped and vetted by AI tools, followed by additional manual work. I secured approval in advance from the reviewer I pinged. I stand behind the quality of the code I'm submitting, and I vouch it's as good or better compared to if I had written every line by my own hand.