Repository navigation
Performance regression in 1.84 for inline-able function in closure #137273
Description
Code
I tried this code:
#[macro_use]
extern crate criterion;
use criterion::Criterion;
use rand::Rng;
// #[inline(never)]
pub fn sum_all(v: &[i32]) -> i32 {
v.iter().fold(0i32, |a, b| curried_add(a)(*b))
}
fn curried_add(a: i32) -> Box<dyn Fn(i32) -> i32> {
Box::new(move |b| a + b)
}
fn criterion_benchmark(c: &mut Criterion) {
let mut group = c.benchmark_group("bg");
let vals = generate_array(100_000);
group.bench_function("sum", |b| {
b.iter(|| {
let sum = sum_all(criterion::black_box(&vals));
criterion::black_box(sum);
})
});
}
fn generate_array(len: usize) -> Vec<i32> {
let mut rng = rand::rng();
(0..len).map(|_| rng.random_range(0..100)).collect()
}
criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);I expected to see this happen: ~26 µs reported by cargo bench
Instead, this happened: ~1.4 ms reported by cargo bench
Version it worked on
It most recently worked on: 1.83.0
$ cargo clean; caffeinate -sd cargo +1.84.1 bench -q -- --save-baseline 1.84.1 && caffeinate -sd cargo +1.83.0 bench -q -- --save-baseline 1.83.0
Removed 1043 files, 228.6MiB total
Benchmarking bg/sum: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 6.7s, enable flat sampling, or reduce sample count to 60.
bg/sum time: [1.3761 ms 1.4096 ms 1.4473 ms]
Found 17 outliers among 100 measurements (17.00%)
1 (1.00%) low severe
16 (16.00%) high severe
bg/sum time: [26.609 µs 26.640 µs 26.679 µs]
Found 10 outliers among 100 measurements (10.00%)
5 (5.00%) high mild
5 (5.00%) high severe
Version with regression
rustc --version --verbose:
rustc 1.84.1 (e71f9a9a9 2025-01-27)
binary: rustc
commit-hash: e71f9a9a98b0faf423844bf0ba7438f29dc27d58
commit-date: 2025-01-27
host: aarch64-apple-darwin
release: 1.84.1
LLVM version: 19.1.5
Backtrace
no compiler crash
#inline
If I uncomment #[inline(never)] line, the performance is back to normal under 1.84.
bisecting output
searched nightlies: from nightly-2024-10-13 to nightly-2024-11-22
regressed nightly: nightly-2024-11-04
searched commit range: b3f75cc...b8c8287
regressed commit: e3a918e
bisected with cargo-bisect-rustc v0.6.9
Host triple: aarch64-apple-darwin
Reproduce with:
cargo bisect-rustc --start 1.83.0 --end 1.84.1 --prompt -- bench -q
Activity
apiraino commentedon Feb 19, 2025
cc @RalfJung
(but I'm not sure about the bisection - I wonder how #132542 can impact inlining)
IIUC the code that regresses performance is:
RalfJung commentedon Feb 19, 2025
I guess it could pass some threshold in some heuristic somewhere? That can happen with any tiny change though. It is very odd though that adding
inline(never)improves performance, or did I misunderstand the report?Note that the inlining handling for
const_panic!changed a bit after that PR so the diff you see there does not reflect the latest rustc any more.Cc @saethlin @scottmcm
RalfJung commentedon Feb 19, 2025
That PR only changes very few functions that are not already
inline(never):clamp,encode_utf16_raw,encode_utf8_raw. I doubt any of them is used in the example above. So I have no explanation how that PR could possibly affect this code.saethlin commentedon Feb 19, 2025
Yes,
inline(never)is faster here. With asterisks. Allinlineattributes perform the same withRUSTFLAGS=-Ccodegen-units=1orI don't think a bisection is informative here, all that PR did was probably perturb CGU partitioning to cause the bad partitioning.
This kind of behavior smells like IPSCCP https://llvm.org/docs/Passes.html#ipsccp-interprocedural-sparse-conditional-constant-propagation
saethlin commentedon Feb 19, 2025
Also, it looks like adding
#[inline]to bothcurried_addandsum_allseems to produce the fast benchmark result. No idea if that's stable wrt other build config perturbations, though I'd expect it is.apiraino commentedon Feb 24, 2025
WG-prioritization assigning priority (Zulip discussion).
@rustbot label -I-prioritize +P-medium
dianqk commentedon Feb 26, 2025
It seems that
criterion::bencher::Bencher<M>::iteris not inliningmy_benchmark::curried_add::{{closure}}. Could this be related to the CGU partitioning? But bothsum_allandcurried_addare inliningmy_benchmark::curried_add::{{closure}}.saethlin commentedon Feb 26, 2025
Closures should always get
InstantiationMode::LocalCopy, so even in a debug build they should getinternaland a copy should be placed in every CGU that calls them directly.So normally CGU partitioning doesn't block inlining of a closure. Perhaps the
Box+dynmakes that not true?