Skip to content

Commit 6c60835

Browse files
ByronByron
committed
feat!: Progress::counter() is now mandatory.
This should simplify downstream code and we just accept that we are dealing with a threaded world. This also comes with performance improvements as increments are now 250% faster.
1 parent e1e282a commit 6c60835

File tree

5 files changed

+76
-22
lines changed

5 files changed

+76
-22
lines changed

‎benches/usage.rs

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use prodash::{
44
tree::{root::Options as TreeOptions, Root as Tree},
55
BoxedDynNestedProgress, Count,
66
};
7+
use std::sync::atomic::Ordering;
78

89
fn usage(c: &mut Criterion) {
910
fn small_tree() -> std::sync::Arc<Tree> {
@@ -14,7 +15,35 @@ fn usage(c: &mut Criterion) {
1415
.create()
1516
.into()
1617
}
17-
c.benchmark_group("BoxedDynProgress::set")
18+
c.benchmark_group("Shared Counter")
19+
.throughput(Throughput::Elements(5))
20+
.bench_function("inc counter 5 times", |b| {
21+
let root = small_tree();
22+
let progress = root.add_child("the one");
23+
progress.init(Some(20), Some("element".into()));
24+
let counter = progress.counter();
25+
b.iter(|| {
26+
counter.fetch_add(1, Ordering::Relaxed);
27+
counter.fetch_add(1, Ordering::Relaxed);
28+
counter.fetch_add(1, Ordering::Relaxed);
29+
counter.fetch_add(1, Ordering::Relaxed);
30+
counter.fetch_add(1, Ordering::Relaxed);
31+
});
32+
})
33+
.bench_function("set counter 5 times", |b| {
34+
let root = small_tree();
35+
let progress = root.add_child("the one");
36+
progress.init(Some(20), Some("element".into()));
37+
let counter = progress.counter();
38+
b.iter(|| {
39+
counter.store(1, Ordering::SeqCst);
40+
counter.store(2, Ordering::SeqCst);
41+
counter.store(3, Ordering::SeqCst);
42+
counter.store(4, Ordering::SeqCst);
43+
counter.store(5, Ordering::SeqCst);
44+
});
45+
});
46+
c.benchmark_group("BoxedDynProgress")
1847
.throughput(Throughput::Elements(5))
1948
.bench_function("set tree 5 times", |b| {
2049
let root = small_tree();
@@ -28,8 +57,21 @@ fn usage(c: &mut Criterion) {
2857
progress.set(4);
2958
progress.set(5);
3059
});
60+
})
61+
.bench_function("inc tree 5 times", |b| {
62+
let root = small_tree();
63+
let progress = root.add_child("the one");
64+
progress.init(Some(20), Some("element".into()));
65+
let progress = BoxedDynNestedProgress::new(progress);
66+
b.iter(|| {
67+
progress.inc();
68+
progress.inc();
69+
progress.inc();
70+
progress.inc();
71+
progress.inc();
72+
});
3173
});
32-
c.benchmark_group("tree::Item::set")
74+
c.benchmark_group("tree::Item")
3375
.throughput(Throughput::Elements(5))
3476
.bench_function("set tree 5 times", |b| {
3577
let root = small_tree();
@@ -42,6 +84,18 @@ fn usage(c: &mut Criterion) {
4284
progress.set(4);
4385
progress.set(5);
4486
});
87+
})
88+
.bench_function("inc tree 5 times", |b| {
89+
let root = small_tree();
90+
let progress = root.add_child("the one");
91+
progress.init(Some(20), Some("element".into()));
92+
b.iter(|| {
93+
progress.inc();
94+
progress.inc();
95+
progress.inc();
96+
progress.inc();
97+
progress.inc();
98+
});
4599
});
46100
c.benchmark_group("Tree::add_child")
47101
.throughput(Throughput::Elements(4))

‎src/progress/log.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,12 @@ impl Count for Log {
8484
}
8585

8686
fn inc_by(&self, step: Step) {
87-
self.step.fetch_add(step, Ordering::SeqCst);
87+
self.step.fetch_add(step, Ordering::Relaxed);
8888
self.maybe_log()
8989
}
9090

91-
fn counter(&self) -> Option<StepShared> {
92-
Some(self.step.clone())
91+
fn counter(&self) -> StepShared {
92+
self.step.clone()
9393
}
9494
}
9595

‎src/progress/utils.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
use crate::{messages::MessageLevel, progress::Id, Count, NestedProgress, Progress, Unit};
2+
use std::sync::atomic::AtomicUsize;
3+
use std::sync::Arc;
24

35
/// An implementation of [`NestedProgress`] which discards all calls.
46
pub struct Discard;
@@ -12,8 +14,8 @@ impl Count for Discard {
1214

1315
fn inc_by(&self, _step: usize) {}
1416

15-
fn counter(&self) -> Option<StepShared> {
16-
None
17+
fn counter(&self) -> StepShared {
18+
Arc::new(AtomicUsize::default())
1719
}
1820
}
1921

@@ -82,7 +84,7 @@ where
8284
Either::Right(r) => r.inc_by(step),
8385
}
8486
}
85-
fn counter(&self) -> Option<StepShared> {
87+
fn counter(&self) -> StepShared {
8688
match self {
8789
Either::Left(l) => l.counter(),
8890
Either::Right(r) => r.counter(),
@@ -223,7 +225,7 @@ where
223225
self.0.inc_by(step)
224226
}
225227

226-
fn counter(&self) -> Option<StepShared> {
228+
fn counter(&self) -> StepShared {
227229
self.0.counter()
228230
}
229231
}
@@ -307,7 +309,7 @@ impl<T: NestedProgress> Count for ThroughputOnDrop<T> {
307309
self.0.inc_by(step)
308310
}
309311

310-
fn counter(&self) -> Option<StepShared> {
312+
fn counter(&self) -> StepShared {
311313
self.0.counter()
312314
}
313315
}

‎src/traits.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,11 @@ pub trait Count {
4242
self.inc_by(1)
4343
}
4444

45-
/// If available, return an atomic counter for direct access to the underlying state.
45+
/// Return an atomic counter for direct access to the underlying state.
4646
///
4747
/// This is useful if multiple threads want to access the same progress, without the need
4848
/// for provide each their own progress and aggregating the result.
49-
fn counter(&self) -> Option<StepShared> {
50-
None
51-
}
49+
fn counter(&self) -> StepShared;
5250
}
5351

5452
/// An object-safe trait for describing hierarchical progress.
@@ -262,7 +260,7 @@ mod impls {
262260
(*self).inc()
263261
}
264262

265-
fn counter(&self) -> Option<StepShared> {
263+
fn counter(&self) -> StepShared {
266264
(*self).counter()
267265
}
268266
}
@@ -287,7 +285,7 @@ mod impls {
287285
self.deref().inc()
288286
}
289287

290-
fn counter(&self) -> Option<StepShared> {
288+
fn counter(&self) -> StepShared {
291289
self.deref().counter()
292290
}
293291
}
@@ -426,7 +424,7 @@ mod impls {
426424
self.0.inc()
427425
}
428426

429-
fn counter(&self) -> Option<StepShared> {
427+
fn counter(&self) -> StepShared {
430428
self.0.counter()
431429
}
432430
}
@@ -488,7 +486,7 @@ mod impls {
488486
self.0.inc()
489487
}
490488

491-
fn counter(&self) -> Option<StepShared> {
489+
fn counter(&self) -> StepShared {
492490
self.0.counter()
493491
}
494492
}

‎src/tree/item.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,14 +213,14 @@ impl Item {
213213
///
214214
/// **Note**: that this call has no effect unless `init(…)` was called before.
215215
pub fn inc_by(&self, step: Step) {
216-
self.value.fetch_add(step, Ordering::SeqCst);
216+
self.value.fetch_add(step, Ordering::Relaxed);
217217
}
218218

219219
/// Increment the current progress by one.
220220
///
221221
/// **Note**: that this call has no effect unless `init(…)` was called before.
222222
pub fn inc(&self) {
223-
self.value.fetch_add(1, Ordering::SeqCst);
223+
self.value.fetch_add(1, Ordering::Relaxed);
224224
}
225225

226226
/// Call to indicate that progress cannot be indicated, and that the task cannot be interrupted.
@@ -358,8 +358,8 @@ impl crate::Count for Item {
358358
self.inc_by(step)
359359
}
360360

361-
fn counter(&self) -> Option<StepShared> {
362-
Some(Arc::clone(&self.value))
361+
fn counter(&self) -> StepShared {
362+
Arc::clone(&self.value)
363363
}
364364
}
365365

0 commit comments

Comments
 (0)