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.

Sign up for GitHub

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

Jump to bottom

Initial working example of lending iterator for combinations. #682

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Initial working example of lending iterator for combinations. #682

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0eefe16
Initial working example of lending iterator for combinations.
Easyoakland Mar 9, 2023
94857f8
Fixed minor whitespace and comment issues.
Easyoakland Mar 10, 2023
7839cd3
Moved common logic for out.
Easyoakland Mar 22, 2023
ce1e22a
Changed parity test to quickcheck.
Easyoakland Mar 22, 2023
cdf8507
Comment fix.
Easyoakland Mar 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ test = false

[dependencies]
either = { version = "1.0", default-features = false }
lending-iterator = { version = "0.1.6", optional = true}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielhenrymantilla This feature relies heavily on your lending-iterator crate. Do you think this would be appropriate functionality to lending-iterator? I think the performance story presented by the PR is compelling enough that this functionality should be somewhere, but I'm not yet sure where.

I share @phimuemue's hesitation for adding a new public dependency to itertools. The bar for adding dependencies, especially public ones, is very high. Itertools only has one dependency (either), and it's maintained by a maintainer of itertools

First, we need to be sure that new dependencies will not pose a risk of breakage for our users. At minimum, new dependencies must adhere to the itertools MSRV policy. As a foundational crate in the Rust ecosystem, we treat MSRV increases as breaking changes; our dependencies should do so too. We also maintain a very conservative MSRV of 1.36; adding lending-iterator as a dependency would require increasing that to 1.57.

Second, we need to be confident that new dependencies (and features, really) will not increase our maintenance burden too much. If we accept this PR, I think it would be reasonable to expect that we will eventually provide lending alternatives of many of our other iterator adapters. I don't know about @phimuemue, but I don't have any experience with lending-iterator. We'll need to consider whether this is a road we want itertools to go down.


[dev-dependencies]
rand = "0.7"
Expand All @@ -39,6 +40,7 @@ quickcheck = { version = "0.9", default_features = false }
default = ["use_std"]
use_std = ["use_alloc", "either/use_std"]
use_alloc = []
lending_iters = ["dep:lending-iterator"]

[profile]
bench = { debug = true }
Expand Down Expand Up @@ -71,6 +73,11 @@ harness = false
name = "combinations"
harness = false

[[bench]]
name = "combinations_lending"
harness = false
required-features = ["default", "lending_iters"]

[[bench]]
name = "powerset"
harness = false
94 changes: 94 additions & 0 deletions benches/combinations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::collections::{HashSet, VecDeque};

use criterion::{black_box, criterion_group, criterion_main, Criterion};
use itertools::Itertools;

Expand Down Expand Up @@ -110,6 +112,91 @@ fn comb_c14(c: &mut Criterion) {
});
}

fn comb_single_use(c: &mut Criterion) {
c.bench_function("comb single use", move |b| {
b.iter(|| {
let mut combination_bitmask = 0usize;
(0..N14).combinations(14).for_each(|combo| {
let compared_bitmask = 0b101010101010101011110000usize;
combo.into_iter().for_each(|bit_pos| {
combination_bitmask |= 1 << bit_pos;
});
black_box((combination_bitmask & compared_bitmask).count_ones());
});
})
});
}

fn comb_into_hash_set(c: &mut Criterion) {
c.bench_function("comb into hash set", move |b| {
b.iter(|| {
(0..N14).combinations(14).for_each(|combo| {
black_box({
let mut out = HashSet::with_capacity(14);
out.extend(combo);
out
});
});
})
});
}

fn comb_into_vec_deque(c: &mut Criterion) {
c.bench_function("comb into vec deque", move |b| {
b.iter(|| {
(0..N14).combinations(14).for_each(|combo| {
black_box(VecDeque::from(combo));
});
})
});
}

fn comb_into_slice(c: &mut Criterion) {
c.bench_function("comb into slice", move |b| {
b.iter(|| {
(0..N14).combinations(14).for_each(|combo| {
black_box({
let mut out = [0; 14];
let mut combo_iter = combo.into_iter();
out.fill_with(|| combo_iter.next().unwrap_or_default());
out
});
});
})
});
}

fn comb_into_slice_unchecked(c: &mut Criterion) {
c.bench_function("comb into slice unchecked", move |b| {
b.iter(|| {
(0..N14).combinations(14).for_each(|combo| {
black_box({
let mut out = [0; 14];
let mut combo_iter = combo.into_iter();
out.fill_with(|| combo_iter.next().unwrap());
out
});
});
})
});
}

fn comb_into_slice_for_loop(c: &mut Criterion) {
c.bench_function("comb into slice for loop", move |b| {
b.iter(|| {
(0..N14).combinations(14).for_each(|combo| {
black_box({
let mut out = [0; 14];
for (i, elem) in combo.into_iter().enumerate() {
out[i] = elem;
}
out
});
});
})
});
}

criterion_group!(
benches,
comb_for1,
Expand All @@ -121,5 +208,12 @@ criterion_group!(
comb_c3,
comb_c4,
comb_c14,
comb_single_use,
comb_into_hash_set,
comb_into_vec_deque,
comb_into_slice,
comb_into_slice_unchecked,
comb_into_slice_for_loop,
);

criterion_main!(benches);
193 changes: 193 additions & 0 deletions benches/combinations_lending.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
#![cfg(feature = "lending_iters")]

use std::collections::{HashSet, VecDeque};

use criterion::{black_box, criterion_group, criterion_main, Criterion};
use itertools::Itertools;
use itertools::LendingIterator;

// approximate 100_000 iterations for each combination
const N1: usize = 100_000;
const N2: usize = 448;
const N3: usize = 86;
const N4: usize = 41;
const N14: usize = 21;

fn comb_lending_c1(c: &mut Criterion) {
c.bench_function("comb lending c1", move |b| {
b.iter(|| {
(0..N1).combinations_lending(1).for_each(|combo| {
black_box({
let mut out = Vec::with_capacity(1);
out.extend(combo);
out
});
});
})
});
}

fn comb_lending_c2(c: &mut Criterion) {
c.bench_function("comb lending c2", move |b| {
b.iter(|| {
(0..N2).combinations_lending(2).for_each(|combo| {
black_box({
let mut out = Vec::with_capacity(2);
out.extend(combo);
out
});
});
})
});
}

fn comb_lending_c3(c: &mut Criterion) {
c.bench_function("comb lending c3", move |b| {
b.iter(|| {
(0..N3).combinations_lending(3).for_each(|combo| {
black_box({
let mut out = Vec::with_capacity(3);
out.extend(combo);
out
});
});
})
});
}

fn comb_lending_c4(c: &mut Criterion) {
c.bench_function("comb lending c4", move |b| {
b.iter(|| {
(0..N4).combinations_lending(4).for_each(|combo| {
black_box({
let mut out = Vec::with_capacity(4);
out.extend(combo);
out
});
});
})
});
}

fn comb_lending_c14(c: &mut Criterion) {
c.bench_function("comb lending c14", move |b| {
b.iter(|| {
(0..N14).combinations_lending(14).for_each(|combo| {
black_box({
let mut out = Vec::with_capacity(14);
out.extend(combo);
out
});
});
})
});
}

fn comb_lending_single_use(c: &mut Criterion) {
c.bench_function("comb lending single use", move |b| {
b.iter(|| {
let mut combination_bitmask = 0usize;
(0..N14).combinations_lending(14).for_each(|combo| {
let compared_bitmask = 0b101010101010101011110000usize;
combo.for_each(|bit_pos| {
combination_bitmask |= 1 << bit_pos;
});
black_box((combination_bitmask & compared_bitmask).count_ones());
});
})
});
}

fn comb_lending_into_hash_set_from_collect(c: &mut Criterion) {
c.bench_function("comb lending into hash set from collect", move |b| {
b.iter(|| {
(0..N14).combinations_lending(14).for_each(|combo| {
black_box(combo.collect::<HashSet<_>>());
});
})
});
}

fn comb_lending_into_hash_set_from_extend(c: &mut Criterion) {
c.bench_function("comb lending into hash set from extend", move |b| {
b.iter(|| {
(0..N14).combinations_lending(14).for_each(|combo| {
black_box({
let mut out = HashSet::with_capacity(14);
out.extend(combo);
out
});
});
})
});
}

fn comb_lending_into_vec_deque_from_collect(c: &mut Criterion) {
c.bench_function("comb lending into vec deque from collect", move |b| {
b.iter(|| {
(0..N14).combinations_lending(14).for_each(|combo| {
black_box(combo.collect::<VecDeque<_>>());
});
})
});
}

fn comb_lending_into_vec_deque_from_extend(c: &mut Criterion) {
c.bench_function("comb lending into vec deque from extend", move |b| {
b.iter(|| {
(0..N14).combinations_lending(14).for_each(|combo| {
black_box({
let mut out = VecDeque::with_capacity(14);
out.extend(combo);
out
});
});
})
});
}

fn comb_lending_into_slice(c: &mut Criterion) {
c.bench_function("comb lending into slice", move |b| {
b.iter(|| {
(0..N14).combinations_lending(14).for_each(|mut combo| {
black_box({
let mut out = [0; 14];
out.fill_with(|| combo.next().unwrap_or_default());
out
});
});
})
});
}

fn comb_lending_into_slice_unchecked(c: &mut Criterion) {
c.bench_function("comb lending into slice unchecked", move |b| {
b.iter(|| {
(0..N14).combinations_lending(14).for_each(|mut combo| {
black_box({
let mut out = [0; 14];
out.fill_with(|| combo.next().unwrap());
out
});
});
})
});
}

criterion_group!(
benches,
comb_lending_c1,
comb_lending_c2,
comb_lending_c3,
comb_lending_c4,
comb_lending_c14,
comb_lending_single_use,
comb_lending_into_hash_set_from_collect,
comb_lending_into_hash_set_from_extend,
comb_lending_into_vec_deque_from_collect,
comb_lending_into_vec_deque_from_extend,
comb_lending_into_slice,
comb_lending_into_slice_unchecked,
);

criterion_main!(benches);