Skip to content

Commit f65277c

Browse files
committed
Simplify parallelization in test-float-parse
Currently, test case generators are launched in parallel and their test cases also run in parallel, all within the same pool. I originally implemented this with the assumption that there would be an advantage in parallelizing the generators themselves, but this turns out to not really have any benefit. Simplify things by running generators in series while keeping their test cases parallelized. This makes the code easier to follow, and there is no longer a need for MPSC or multiprogress bars. Additionally, the UI output can be made cleaner.
1 parent 9af8985 commit f65277c

File tree

7 files changed

+142
-221
lines changed

7 files changed

+142
-221
lines changed

src/etc/test-float-parse/src/gen/exhaustive.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,12 @@ impl<F: Float> Generator<F> for Exhaustive<F>
1313
where
1414
RangeInclusive<F::Int>: Iterator<Item = F::Int>,
1515
{
16-
const NAME: &'static str = "exhaustive";
1716
const SHORT_NAME: &'static str = "exhaustive";
1817

1918
type WriteCtx = F;
2019

2120
fn total_tests() -> u64 {
22-
F::Int::MAX.try_into().unwrap_or(u64::MAX)
21+
1u64.checked_shl(F::Int::BITS).expect("More than u64::MAX tests")
2322
}
2423

2524
fn new() -> Self {

src/etc/test-float-parse/src/gen/fuzz.rs

-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ impl<F: Float> Generator<F> for Fuzz<F>
4949
where
5050
Standard: Distribution<<F as Float>::Int>,
5151
{
52-
const NAME: &'static str = "fuzz";
5352
const SHORT_NAME: &'static str = "fuzz";
5453

5554
type WriteCtx = F;

src/etc/test-float-parse/src/gen/sparse.rs

-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ impl<F: Float> Generator<F> for FewOnesInt<F>
3535
where
3636
<F::Int as TryFrom<u128>>::Error: std::fmt::Debug,
3737
{
38-
const NAME: &'static str = "few ones int";
3938
const SHORT_NAME: &'static str = "few ones int";
4039

4140
type WriteCtx = F::Int;

src/etc/test-float-parse/src/lib.rs

+46-167
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,19 @@ mod traits;
22
mod ui;
33
mod validate;
44

5-
use std::any::{TypeId, type_name};
5+
use std::any::type_name;
66
use std::cmp::min;
77
use std::ops::RangeInclusive;
88
use std::process::ExitCode;
9+
use std::sync::OnceLock;
910
use std::sync::atomic::{AtomicU64, Ordering};
10-
use std::sync::{OnceLock, mpsc};
1111
use std::{fmt, time};
1212

13-
use indicatif::{MultiProgress, ProgressBar};
1413
use rand::distributions::{Distribution, Standard};
1514
use rayon::prelude::*;
1615
use time::{Duration, Instant};
1716
use traits::{Float, Generator, Int};
17+
use validate::CheckError;
1818

1919
/// Test generators.
2020
mod gen {
@@ -43,7 +43,7 @@ const HUGE_TEST_CUTOFF: u64 = 5_000_000;
4343
/// Seed for tests that use a deterministic RNG.
4444
const SEED: [u8; 32] = *b"3.141592653589793238462643383279";
4545

46-
/// Global configuration
46+
/// Global configuration.
4747
#[derive(Debug)]
4848
pub struct Config {
4949
pub timeout: Duration,
@@ -106,7 +106,7 @@ pub fn run(cfg: Config, include: &[String], exclude: &[String]) -> ExitCode {
106106

107107
println!("launching");
108108
let elapsed = launch_tests(&mut tests, &cfg);
109-
ui::finish(&tests, elapsed, &cfg)
109+
ui::finish_all(&tests, elapsed, &cfg)
110110
}
111111

112112
/// Enumerate tests to run but don't actually run them.
@@ -160,18 +160,18 @@ where
160160
#[derive(Debug)]
161161
pub struct TestInfo {
162162
pub name: String,
163-
/// Tests are identified by the type ID of `(F, G)` (tuple of the float and generator type).
164-
/// This gives an easy way to associate messages with tests.
165-
id: TypeId,
166163
float_name: &'static str,
164+
float_bits: u32,
167165
gen_name: &'static str,
168166
/// Name for display in the progress bar.
169167
short_name: String,
168+
/// Pad the short name to a common width for progress bar use.
169+
short_name_padded: String,
170170
total_tests: u64,
171171
/// Function to launch this test.
172-
launch: fn(&mpsc::Sender<Msg>, &TestInfo, &Config),
172+
launch: fn(&TestInfo, &Config),
173173
/// Progress bar to be updated.
174-
pb: Option<ProgressBar>,
174+
progress: Option<ui::Progress>,
175175
/// Once completed, this will be set.
176176
completed: OnceLock<Completed>,
177177
}
@@ -187,121 +187,37 @@ impl TestInfo {
187187
let f_name = type_name::<F>();
188188
let gen_name = G::NAME;
189189
let gen_short_name = G::SHORT_NAME;
190+
let name = format!("{f_name} {gen_name}");
191+
let short_name = format!("{f_name} {gen_short_name}");
192+
let short_name_padded = format!("{short_name:18}");
190193

191194
let info = TestInfo {
192-
id: TypeId::of::<(F, G)>(),
193195
float_name: f_name,
196+
float_bits: F::BITS,
194197
gen_name,
195-
pb: None,
196-
name: format!("{f_name} {gen_name}"),
197-
short_name: format!("{f_name} {gen_short_name}"),
198+
progress: None,
199+
name,
200+
short_name_padded,
201+
short_name,
198202
launch: test_runner::<F, G>,
199203
total_tests: G::total_tests(),
200204
completed: OnceLock::new(),
201205
};
202206
v.push(info);
203207
}
204208

205-
/// Pad the short name to a common width for progress bar use.
206-
fn short_name_padded(&self) -> String {
207-
format!("{:18}", self.short_name)
208-
}
209-
210-
/// Create a progress bar for this test within a multiprogress bar.
211-
fn register_pb(&mut self, mp: &MultiProgress, drop_bars: &mut Vec<ProgressBar>) {
212-
self.pb = Some(ui::create_pb(mp, self.total_tests, &self.short_name_padded(), drop_bars));
213-
}
214-
215-
/// When the test is finished, update progress bar messages and finalize.
216-
fn finalize_pb(&self, c: &Completed) {
217-
let pb = self.pb.as_ref().unwrap();
218-
ui::finalize_pb(pb, &self.short_name_padded(), c);
219-
}
220-
221209
/// True if this should be run after all others.
222210
fn is_huge_test(&self) -> bool {
223211
self.total_tests >= HUGE_TEST_CUTOFF
224212
}
225-
}
226-
227-
/// A message sent from test runner threads to the UI/log thread.
228-
#[derive(Clone, Debug)]
229-
struct Msg {
230-
id: TypeId,
231-
update: Update,
232-
}
233213

234-
impl Msg {
235-
/// Wrap an `Update` into a message for the specified type. We use the `TypeId` of `(F, G)` to
236-
/// identify which test a message in the channel came from.
237-
fn new<F: Float, G: Generator<F>>(u: Update) -> Self {
238-
Self { id: TypeId::of::<(F, G)>(), update: u }
239-
}
240-
241-
/// Get the matching test from a list. Panics if not found.
242-
fn find_test<'a>(&self, tests: &'a [TestInfo]) -> &'a TestInfo {
243-
tests.iter().find(|t| t.id == self.id).unwrap()
244-
}
245-
246-
/// Update UI as needed for a single message received from the test runners.
247-
fn handle(self, tests: &[TestInfo], mp: &MultiProgress) {
248-
let test = self.find_test(tests);
249-
let pb = test.pb.as_ref().unwrap();
250-
251-
match self.update {
252-
Update::Started => {
253-
mp.println(format!("Testing '{}'", test.name)).unwrap();
254-
}
255-
Update::Progress { executed, failures } => {
256-
pb.set_message(format! {"{failures}"});
257-
pb.set_position(executed);
258-
}
259-
Update::Failure { fail, input, float_res } => {
260-
mp.println(format!(
261-
"Failure in '{}': {fail}. parsing '{input}'. Parsed as: {float_res}",
262-
test.name
263-
))
264-
.unwrap();
265-
}
266-
Update::Completed(c) => {
267-
test.finalize_pb(&c);
268-
269-
let prefix = match c.result {
270-
Ok(FinishedAll) => "Completed tests for",
271-
Err(EarlyExit::Timeout) => "Timed out",
272-
Err(EarlyExit::MaxFailures) => "Max failures reached for",
273-
};
274-
275-
mp.println(format!(
276-
"{prefix} generator '{}' in {:?}. {} tests run, {} failures",
277-
test.name, c.elapsed, c.executed, c.failures
278-
))
279-
.unwrap();
280-
test.completed.set(c).unwrap();
281-
}
282-
};
214+
/// When the test is finished, update progress bar messages and finalize.
215+
fn complete(&self, c: Completed) {
216+
self.progress.as_ref().unwrap().complete(&c, 0);
217+
self.completed.set(c).unwrap();
283218
}
284219
}
285220

286-
/// Status sent with a message.
287-
#[derive(Clone, Debug)]
288-
enum Update {
289-
/// Starting a new test runner.
290-
Started,
291-
/// Completed a out of b tests.
292-
Progress { executed: u64, failures: u64 },
293-
/// Received a failed test.
294-
Failure {
295-
fail: CheckFailure,
296-
/// String for which parsing was attempted.
297-
input: Box<str>,
298-
/// The parsed & decomposed `FloatRes`, already stringified so we don't need generics here.
299-
float_res: Box<str>,
300-
},
301-
/// Exited with an unexpected condition.
302-
Completed(Completed),
303-
}
304-
305221
/// Result of an input did not parsing successfully.
306222
#[derive(Clone, Debug)]
307223
enum CheckFailure {
@@ -398,71 +314,34 @@ enum EarlyExit {
398314
/// This launches a main thread that receives messages and handlees UI updates, and uses the
399315
/// rest of the thread pool to execute the tests.
400316
fn launch_tests(tests: &mut [TestInfo], cfg: &Config) -> Duration {
401-
// Run shorter tests first
402-
tests.sort_unstable_by_key(|test| test.total_tests);
317+
// Run shorter tests and smaller float types first.
318+
tests.sort_unstable_by_key(|test| (test.total_tests, test.float_bits));
403319

404320
for test in tests.iter() {
405321
println!("Launching test '{}'", test.name);
406322
}
407323

408-
// Configure progress bars
409324
let mut all_progress_bars = Vec::new();
410-
let mp = MultiProgress::new();
411-
mp.set_move_cursor(true);
412-
for test in tests.iter_mut() {
413-
test.register_pb(&mp, &mut all_progress_bars);
414-
}
415-
416-
ui::set_panic_hook(all_progress_bars);
417-
418-
let (tx, rx) = mpsc::channel::<Msg>();
419325
let start = Instant::now();
420326

421-
rayon::scope(|scope| {
422-
// Thread that updates the UI
423-
scope.spawn(|_scope| {
424-
let rx = rx; // move rx
425-
426-
loop {
427-
if tests.iter().all(|t| t.completed.get().is_some()) {
428-
break;
429-
}
430-
431-
let msg = rx.recv().unwrap();
432-
msg.handle(tests, &mp);
433-
}
434-
435-
// All tests completed; finish things up
436-
drop(mp);
437-
assert_eq!(rx.try_recv().unwrap_err(), mpsc::TryRecvError::Empty);
438-
});
439-
440-
// Don't let the thread pool be starved by huge tests. Run faster tests first in parallel,
441-
// then parallelize only within the rest of the tests.
442-
let (huge_tests, normal_tests): (Vec<_>, Vec<_>) =
443-
tests.iter().partition(|t| t.is_huge_test());
444-
445-
// Run the actual tests
446-
normal_tests.par_iter().for_each(|test| ((test.launch)(&tx, test, cfg)));
447-
448-
huge_tests.par_iter().for_each(|test| ((test.launch)(&tx, test, cfg)));
449-
});
327+
for test in tests.iter_mut() {
328+
test.progress = Some(ui::Progress::new(test, &mut all_progress_bars));
329+
ui::set_panic_hook(&all_progress_bars);
330+
((test.launch)(test, cfg));
331+
}
450332

451333
start.elapsed()
452334
}
453335

454336
/// Test runer for a single generator.
455337
///
456338
/// This calls the generator's iterator multiple times (in parallel) and validates each output.
457-
fn test_runner<F: Float, G: Generator<F>>(tx: &mpsc::Sender<Msg>, _info: &TestInfo, cfg: &Config) {
458-
tx.send(Msg::new::<F, G>(Update::Started)).unwrap();
459-
460-
let total = G::total_tests();
339+
fn test_runner<F: Float, G: Generator<F>>(test: &TestInfo, cfg: &Config) {
461340
let gen = G::new();
462341
let executed = AtomicU64::new(0);
463342
let failures = AtomicU64::new(0);
464343

465-
let checks_per_update = min(total, 1000);
344+
let checks_per_update = min(test.total_tests, 1000);
466345
let started = Instant::now();
467346

468347
// Function to execute for a single test iteration.
@@ -474,7 +353,12 @@ fn test_runner<F: Float, G: Generator<F>>(tx: &mpsc::Sender<Msg>, _info: &TestIn
474353
match validate::validate::<F>(buf) {
475354
Ok(()) => (),
476355
Err(e) => {
477-
tx.send(Msg::new::<F, G>(e)).unwrap();
356+
let CheckError { fail, input, float_res } = e;
357+
test.progress.as_ref().unwrap().println(&format!(
358+
"Failure in '{}': {fail}. parsing '{input}'. Parsed as: {float_res}",
359+
test.name
360+
));
361+
478362
let f = failures.fetch_add(1, Ordering::Relaxed);
479363
// End early if the limit is exceeded.
480364
if f >= cfg.max_failures {
@@ -486,9 +370,7 @@ fn test_runner<F: Float, G: Generator<F>>(tx: &mpsc::Sender<Msg>, _info: &TestIn
486370
// Send periodic updates
487371
if executed % checks_per_update == 0 {
488372
let failures = failures.load(Ordering::Relaxed);
489-
490-
tx.send(Msg::new::<F, G>(Update::Progress { executed, failures })).unwrap();
491-
373+
test.progress.as_ref().unwrap().update(executed, failures);
492374
if started.elapsed() > cfg.timeout {
493375
return Err(EarlyExit::Timeout);
494376
}
@@ -499,28 +381,25 @@ fn test_runner<F: Float, G: Generator<F>>(tx: &mpsc::Sender<Msg>, _info: &TestIn
499381

500382
// Run the test iterations in parallel. Each thread gets a string buffer to write
501383
// its check values to.
502-
let res = gen.par_bridge().try_for_each_init(|| String::with_capacity(100), check_one);
384+
let res = gen.par_bridge().try_for_each_init(String::new, check_one);
503385

504386
let elapsed = started.elapsed();
505387
let executed = executed.into_inner();
506388
let failures = failures.into_inner();
507389

508390
// Warn about bad estimates if relevant.
509-
let warning = if executed != total && res.is_ok() {
510-
let msg = format!("executed tests != estimated ({executed} != {total}) for {}", G::NAME);
391+
let warning = if executed != test.total_tests && res.is_ok() {
392+
let msg = format!(
393+
"executed tests != estimated ({executed} != {}) for {}",
394+
test.total_tests,
395+
G::NAME
396+
);
511397

512398
Some(msg.into())
513399
} else {
514400
None
515401
};
516402

517403
let result = res.map(|()| FinishedAll);
518-
tx.send(Msg::new::<F, G>(Update::Completed(Completed {
519-
executed,
520-
failures,
521-
result,
522-
warning,
523-
elapsed,
524-
})))
525-
.unwrap();
404+
test.complete(Completed { executed, failures, result, warning, elapsed });
526405
}

src/etc/test-float-parse/src/traits.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ impl_float!(f32, u32, 32; f64, u64, 64);
177177
/// allocations (which otherwise turn out to be a pretty expensive part of these tests).
178178
pub trait Generator<F: Float>: Iterator<Item = Self::WriteCtx> + Send + 'static {
179179
/// Full display and filtering name
180-
const NAME: &'static str;
180+
const NAME: &'static str = Self::SHORT_NAME;
181181

182182
/// Name for display with the progress bar
183183
const SHORT_NAME: &'static str;

0 commit comments

Comments
 (0)