Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggest faster .contains() instead of .iter().any() for [u8] and [i8] slices #13353

Open
nyurik opened this issue Sep 6, 2024 · 3 comments · May be fixed by #13817
Open

Suggest faster .contains() instead of .iter().any() for [u8] and [i8] slices #13353

nyurik opened this issue Sep 6, 2024 · 3 comments · May be fixed by #13817
Labels
A-lint Area: New lints

Comments

@nyurik
Copy link
Contributor

nyurik commented Sep 6, 2024

What it does

Suggest to replace values.iter().any(|&x| x == 10) with values.contains(&10) as shown in the example for x being u8 or an i8. Contains uses specialization, thus gains 8-10x in performance.

The generated assembly is significantly different too (see here).

Advantage

  • 8x faster code

Drawbacks

  • none(?)

Example

#[inline(never)]
pub fn has_any(values: &[u8]) -> bool {
    values.iter().any(|&x| x == 10)
}

Could be written as:

#[inline(never)]
pub fn ref_zero(values: &[u8]) -> bool {
    values.contains(&10)
}
@nyurik nyurik added the A-lint Area: New lints label Sep 6, 2024
@nyurik
Copy link
Contributor Author

nyurik commented Sep 6, 2024

If anyone wants to try it, run this with cargo bench:

[dev-dependencies]
criterion = { version = "0.5", features = ["html_reports"] }

[[bench]]
name = "my_benchmark"
harness = false
// benches/my_benchmark.rs
use std::hint::black_box;
use criterion::{criterion_group, criterion_main, Criterion};

#[inline(never)]
pub fn contains(values: &[u8]) -> bool {
    values.contains(&10)
}

#[inline(never)]
pub fn has_any(values: &[u8]) -> bool {
    values.iter().any(|x| *x == 10)
}

fn criterion_benchmark(c: &mut Criterion) {
    let data = vec![20u8; 1000];
    let slice = &data[..];
    c.bench_function("ref_zero", |b| b.iter(|| contains(black_box(slice))));
    c.bench_function("has_any", |b| b.iter(|| has_any(black_box(slice))));
}

criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);

@Jarcho
Copy link
Contributor

Jarcho commented Sep 6, 2024

@nyurik
Copy link
Contributor Author

nyurik commented Sep 6, 2024

@Jarcho ah, thx, i missed the specialization. Thus, it makes perfect sense to do this as a lint for u8 and i8

@nyurik nyurik changed the title Significantly improve perf by replacing .iter().any() with .contains() Optimize u8 & i8 slice searching by replacing .iter().any() with .contains() Sep 6, 2024
@nyurik nyurik changed the title Optimize u8 & i8 slice searching by replacing .iter().any() with .contains() Suggest faster .contains() instead of .iter().any() for [u8] and [i8] slices Sep 6, 2024
@lapla-cogito lapla-cogito linked a pull request Dec 12, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants