nandi/rustnim · 71ee2a9

⑂ 71ee2a9

Run the cases in parallel, and report every failure in a body

Two things were serial that had no reason to be.

The differential suite ran its 73 cases one after another, so the wall
clock was spent waiting on rustc and nim while seven of eight cores sat
idle: 130s, against 4m34 of user time. The cases are independent -- each
has its own scratch directory and reads nothing another writes -- so they
now run on `available_parallelism()` threads, results collected in case
order so the output does not depend on scheduling. 130s -> 28s.

Each case also got a *fresh* `--nimcache`, which meant recompiling the
same standard-library modules 73 times; measured at 1.00s cold against
0.53s warm for a trivial module. The cache is now per worker rather than
per case: warm, and with no writers racing the way one shared cache would
have. `RUSTNIM_JOBS` overrides the thread count.

And `--report` still stopped at the first failing statement inside an
item, so a body with three unsupported calls named one of them per run --
the queue that flag exists to avoid, just one level down. It now records
the failure and carries on to the next statement. Four of the six passes
were likewise fail-fast even under `--report`; they and the two that were
not now share one `note()` that decides accumulate-or-stop in a single
place. Outside `--report` the first failure still stops the run: a partial
translation must never reach a file.

`cargo test --release`: 36.5s, all green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

nandithebull committed 2026-09-20T10:22:28-07:00 Browse files

71ee2a9 parent: 67f537f

modified src/lower.rs +64 -28 Viewed

@@ -614,7 +614,8 @@ impl Lowerer {
614	614	for (m, f) in files {
615	615	self.cur_mod = m.clone();
616	616	for item in &f.items {
617		- self.collect_aliases(item).map_err(\|e\| self.where_(&e))?;
	617	+ let r = self.collect_aliases(item);
	618	+ self.note(r)?;
618	619	}
619	620	}
620	621
@@ -637,7 +638,8 @@ impl Lowerer {
637	638	for (m, f) in files {
638	639	self.cur_mod = m.clone();
639	640	for item in &f.items {
640		- self.collect(item).map_err(\|e\| self.where_(&e))?;
	641	+ let r = self.collect(item);
	642	+ self.note(r)?;
641	643	}
642	644	}
643	645
@@ -662,7 +664,8 @@ impl Lowerer {
662	664	for (m, f) in files {
663	665	self.cur_mod = m.clone();
664	666	for item in &f.items {
665		- self.emit_assoc_consts(item).map_err(\|e\| self.where_(&e))?;
	667	+ let r = self.emit_assoc_consts(item);
	668	+ self.note(r)?;
666	669	}
667	670	}
668	671
@@ -670,14 +673,8 @@ impl Lowerer {
670	673	for (m, f) in files {
671	674	self.cur_mod = m.clone();
672	675	for item in &f.items {
673		- if let Err(e) = self.item_types(item) {
674		- let e = self.where_(&e);
675		- if self.report {
676		- self.errors.push(e);
677		- } else {
678		- return Err(e);
679		- }
680		- }
	676	+ let r = self.item_types(item);
	677	+ self.note(r)?;
681	678	}
682	679	}
683	680
@@ -710,14 +707,8 @@ impl Lowerer {
710	707	for (m, f) in files {
711	708	self.cur_mod = m.clone();
712	709	for item in &f.items {
713		- if let Err(e) = self.item(item) {
714		- let e = self.where_(&e);
715		- if self.report {
716		- self.errors.push(e);
717		- } else {
718		- return Err(e);
719		- }
720		- }
	710	+ let r = self.item(item);
	711	+ self.note(r)?;
721	712	}
722	713	}
723	714
@@ -757,7 +748,7 @@ impl Lowerer {
757	748	}
758	749	let mut hist: Vec<(String, usize)> = counts.into_iter().collect();
759	750	hist.sort_by(\|a, b\| b.1.cmp(&a.1).then(a.0.cmp(&b.0)));
760		- let mut out = format!("{} item(s) could not be translated.\n\nby cause:\n", self.errors.len());
	751	+ let mut out = format!("{} failure(s); the run translated nothing.\n\nby cause:\n", self.errors.len());
761	752	for (w, n) in hist.iter().take(40) {
762	753	out.push_str(&format!(" {n:>5} {w}\n"));
763	754	}
@@ -3782,9 +3773,9 @@ impl Lowerer {
3782	3773	for (i, st) in b.stmts.iter().enumerate() {
3783	3774	let last = i + 1 == n;
3784	3775	let rest = &b.stmts[i + 1..];
3785		- match st {
	3776	+ let r: Result<(), String> = match st {
3786	3777	Stmt::Expr(e, None) if last && expressible(e) => {
3787		- tail = Some(self.expr_at(e, expect)?)
	3778	+ self.expr_at(e, expect).map(\|v\| tail = Some(v))
3788	3779	}
3789	3780	Stmt::Expr(e, None) if last => {
3790	3781	// A trailing `if`/`match` with statement arms, or a loop.
@@ -3793,18 +3784,41 @@ impl Lowerer {
3793	3784	match &target {
3794	3785	Some((t, ty)) => {
3795	3786	let (t, ty) = (t.clone(), ty.clone());
3796		- self.assign_from(e, &t, ty.as_ref())?;
	3787	+ self.assign_from(e, &t, ty.as_ref())
3797	3788	}
3798		- None => self.stmt(st)?,
	3789	+ None => self.stmt(st),
3799	3790	}
3800	3791	}
3801	3792	Stmt::Local(l) => {
3802		- self.local(l, rest)?;
3803		- for lk in std::mem::take(&mut self.pending_locks) {
3804		- self.line(&format!("release({})", lk));
	3793	+ let r = self.local(l, rest);
	3794	+ if r.is_ok() {
	3795	+ for lk in std::mem::take(&mut self.pending_locks) {
	3796	+ self.line(&format!("release({})", lk));
	3797	+ }
3805	3798	}
	3799	+ r
	3800	+ }
	3801	+ _ => self.stmt(st),
	3802	+ };
	3803	+ if let Err(e) = r {
	3804	+ // Outside `--report` the first failing statement stops the
	3805	+ // run, as it always has.
	3806	+ if !self.report {
	3807	+ return Err(e);
3806	3808	}
3807		- _ => self.stmt(st)?,
	3809	+ // Under `--report`, carry on to the next statement. A body
	3810	+ // with three unsupported calls in it named one of them per
	3811	+ // run, so finding out what a crate actually needs took three
	3812	+ // runs instead of one. Nothing is written in report mode, so
	3813	+ // the half-lowered body this leaves behind goes nowhere; what
	3814	+ // it buys is the other two names.
	3815	+ //
	3816	+ // Later statements may now fail for want of a binding the
	3817	+ // failed one would have introduced. That noise is the price,
	3818	+ // and the histogram this feeds already folds repeats together.
	3819	+ let e = self.where_(&e);
	3820	+ self.errors.push(e);
	3821	+ self.line("discard \"rustnim: statement not translated\"");
3808	3822	}
3809	3823	}
3810	3824	self.target = target;
@@ -3827,6 +3841,28 @@ impl Lowerer {
3827	3841	Ok(())
3828	3842	}
3829	3843
	3844	+ /// Fail-fast, or record and carry on.
	3845	+ ///
	3846	+ /// Outside `--report` the first failure stops the run: a partial
	3847	+ /// translation must never reach a file. Under `--report` the failure is
	3848	+ /// recorded and the pass continues, so one run names every item that
	3849	+ /// could not be translated instead of turning a list into a queue --
	3850	+ /// fix the first, run again, meet the second.
	3851	+ fn note(&mut self, r: Result<(), String>) -> Result<(), String> {
	3852	+ match r {
	3853	+ Ok(()) => Ok(()),
	3854	+ Err(e) => {
	3855	+ let e = self.where_(&e);
	3856	+ if self.report {
	3857	+ self.errors.push(e);
	3858	+ Ok(())
	3859	+ } else {
	3860	+ Err(e)
	3861	+ }
	3862	+ }
	3863	+ }
	3864	+ }
	3865	+
3830	3866	fn stmt(&mut self, s: &Stmt) -> Result<(), String> {
3831	3867	let r = self.stmt_inner(s);
3832	3868	// Any lock still queued was taken by a temporary guard, which Rust

          for (m, f) in files {
              self.cur_mod = m.clone();
              for item in &f.items {
--                self.collect_aliases(item).map_err(|e| self.where_(&e))?;
++                let r = self.collect_aliases(item);
++                self.note(r)?;
              }
          }
          for (m, f) in files {
              self.cur_mod = m.clone();
              for item in &f.items {
--                self.collect(item).map_err(|e| self.where_(&e))?;
++                let r = self.collect(item);
++                self.note(r)?;
              }
          }
          for (m, f) in files {
              self.cur_mod = m.clone();
              for item in &f.items {
--                self.emit_assoc_consts(item).map_err(|e| self.where_(&e))?;
++                let r = self.emit_assoc_consts(item);
++                self.note(r)?;
              }
          }
          for (m, f) in files {
              self.cur_mod = m.clone();
              for item in &f.items {
--                if let Err(e) = self.item_types(item) {
++                let r = self.item_types(item);
--                    let e = self.where_(&e);
++                self.note(r)?;
--                    if self.report {
--                        self.errors.push(e);
--                    } else {
--                        return Err(e);
--                    }
--                }
              }
          }
          for (m, f) in files {
              self.cur_mod = m.clone();
              for item in &f.items {
--                if let Err(e) = self.item(item) {
++                let r = self.item(item);
--                    let e = self.where_(&e);
++                self.note(r)?;
--                    if self.report {
--                        self.errors.push(e);
--                    } else {
--                        return Err(e);
--                    }
--                }
              }
          }
              }
              let mut hist: Vec<(String, usize)> = counts.into_iter().collect();
              hist.sort_by(|a, b| b.1.cmp(&a.1).then(a.0.cmp(&b.0)));
--            let mut out = format!("{} item(s) could not be translated.\n\nby cause:\n", self.errors.len());
++            let mut out = format!("{} failure(s); the run translated nothing.\n\nby cause:\n", self.errors.len());
              for (w, n) in hist.iter().take(40) {
                  out.push_str(&format!("  {n:>5}  {w}\n"));
              }
          for (i, st) in b.stmts.iter().enumerate() {
              let last = i + 1 == n;
              let rest = &b.stmts[i + 1..];
--            match st {
++            let r: Result<(), String> = match st {
                  Stmt::Expr(e, None) if last && expressible(e) => {
--                    tail = Some(self.expr_at(e, expect)?)
++                    self.expr_at(e, expect).map(|v| tail = Some(v))
                  }
                  Stmt::Expr(e, None) if last => {
                      // A trailing `if`/`match` with statement arms, or a loop.
                      match &target {
                          Some((t, ty)) => {
                              let (t, ty) = (t.clone(), ty.clone());
--                            self.assign_from(e, &t, ty.as_ref())?;
++                            self.assign_from(e, &t, ty.as_ref())
                          }
--                        None => self.stmt(st)?,
++                        None => self.stmt(st),
                      }
                  }
                  Stmt::Local(l) => {
--                    self.local(l, rest)?;
++                    let r = self.local(l, rest);
--                    for lk in std::mem::take(&mut self.pending_locks) {
++                    if r.is_ok() {
--                        self.line(&format!("release({})", lk));
++                        for lk in std::mem::take(&mut self.pending_locks) {
++                            self.line(&format!("release({})", lk));
++                        }
                      }
++                    r
++                }
++                _ => self.stmt(st),
++            };
++            if let Err(e) = r {
++                // Outside `--report` the first failing statement stops the
++                // run, as it always has.
++                if !self.report {
++                    return Err(e);
                  }
--                _ => self.stmt(st)?,
++                // Under `--report`, carry on to the next statement. A body
++                // with three unsupported calls in it named one of them per
++                // run, so finding out what a crate actually needs took three
++                // runs instead of one. Nothing is written in report mode, so
++                // the half-lowered body this leaves behind goes nowhere; what
++                // it buys is the other two names.
++                //
++                // Later statements may now fail for want of a binding the
++                // failed one would have introduced. That noise is the price,
++                // and the histogram this feeds already folds repeats together.
++                let e = self.where_(&e);
++                self.errors.push(e);
++                self.line("discard \"rustnim: statement not translated\"");
              }
          }
          self.target = target;
          Ok(())
      }
++    /// Fail-fast, or record and carry on.
++    ///
++    /// Outside `--report` the first failure stops the run: a partial
++    /// translation must never reach a file. Under `--report` the failure is
++    /// recorded and the pass continues, so one run names every item that
++    /// could not be translated instead of turning a list into a queue --
++    /// fix the first, run again, meet the second.
++    fn note(&mut self, r: Result<(), String>) -> Result<(), String> {
++        match r {
++            Ok(()) => Ok(()),
++            Err(e) => {
++                let e = self.where_(&e);
++                if self.report {
++                    self.errors.push(e);
++                    Ok(())
++                } else {
++                    Err(e)
++                }
++            }
++        }
++    }
++
      fn stmt(&mut self, s: &Stmt) -> Result<(), String> {
          let r = self.stmt_inner(s);
          // Any lock still queued was taken by a temporary guard, which Rust

modified tests/differential.rs +57 -17 Viewed

@@ -249,7 +249,7 @@ fn tail(s: &str, lines: usize) -> String {
249	249
250	250	// --------------------------------------------------------------- one case
251	251
252		-fn run_case(case: &Path, work: &Path, nim: &Path) -> Outcome {
	252	+fn run_case(case: &Path, work: &Path, nim: &Path, cache: &Path) -> Outcome {
253	253	let name = case.file_stem().unwrap().to_string_lossy().into_owned();
254	254	// For a directory case, `main.rs` is the crate root and carries the
255	255	// directives; every `.rs` beside it is passed to rustnim as well.
@@ -390,7 +390,7 @@ fn run_case(case: &Path, work: &Path, nim: &Path) -> Outcome {
390	390	.arg("--hints:off")
391	391	.arg("--warnings:off")
392	392	.arg("--colors:off")
393		- .arg(format!("--nimcache:{}", dir.join("nimcache").display()))
	393	+ .arg(format!("--nimcache:{}", cache.display()))
394	394	.arg(format!("-o:{}", nim_bin.display()))
395	395	.arg(&nim_src)
396	396	.env("TMPDIR", &dir),
@@ -480,22 +480,62 @@ fn differential() {
480	480	assert!(!cases.is_empty(), "no cases in {}", cases_dir.display());
481	481
482	482	let filter = std::env::var("RUSTNIM_CASE").ok();
483		- let mut results: BTreeMap<String, Outcome> = BTreeMap::new();
484		- for case in &cases {
485		- let name = case.file_stem().unwrap().to_string_lossy().into_owned();
486		- if let Some(f) = &filter {
487		- if !name.contains(f.as_str()) {
488		- continue;
489		- }
490		- }
491		- let outcome = run_case(case, &work, &nim);
492		- match &outcome {
493		- Outcome::Pass => eprintln!("ok {name}"),
494		- Outcome::Skip(why) => eprintln!("skip {name} ({why})"),
495		- Outcome::Fail { .. } => eprintln!("FAIL {name}"),
	483	+ let cases: Vec<PathBuf> = cases
	484	+ .into_iter()
	485	+ .filter(\|c\| match &filter {
	486	+ None => true,
	487	+ Some(f) => c
	488	+ .file_stem()
	489	+ .unwrap()
	490	+ .to_string_lossy()
	491	+ .contains(f.as_str()),
	492	+ })
	493	+ .collect();
	494	+
	495	+ // The cases are independent -- each one gets its own scratch directory and
	496	+ // reads nothing the others write -- so they are run on several threads.
	497	+ // Serially this suite spends its wall clock waiting on rustc and nim while
	498	+ // seven of eight cores sit idle.
	499	+ //
	500	+ // Each worker keeps its own Nim cache, rather than each case getting a
	501	+ // fresh one. A cold cache costs about half a second per case, all of it
	502	+ // recompiling the same standard-library modules; one cache per worker is
	503	+ // both warm and free of the write races a single shared cache would have.
	504	+ let jobs: usize = std::env::var("RUSTNIM_JOBS")
	505	+ .ok()
	506	+ .and_then(\|v\| v.parse().ok())
	507	+ .filter(\|n\| *n > 0)
	508	+ .unwrap_or_else(\|\| {
	509	+ std::thread::available_parallelism().map(\|n\| n.get()).unwrap_or(1)
	510	+ })
	511	+ .min(cases.len().max(1));
	512	+
	513	+ let next = std::sync::atomic::AtomicUsize::new(0);
	514	+ let results: std::sync::Mutex<BTreeMap<String, Outcome>> =
	515	+ std::sync::Mutex::new(BTreeMap::new());
	516	+ std::thread::scope(\|scope\| {
	517	+ for w in 0..jobs {
	518	+ let (next, results, cases, work, nim) = (&next, &results, &cases, &work, &nim);
	519	+ scope.spawn(move \|\| {
	520	+ let cache = work.join(format!("nimcache-{w}"));
	521	+ loop {
	522	+ let i = next.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
	523	+ let Some(case) = cases.get(i) else { break };
	524	+ let name = case.file_stem().unwrap().to_string_lossy().into_owned();
	525	+ let outcome = run_case(case, work, nim, &cache);
	526	+ match &outcome {
	527	+ Outcome::Pass => eprintln!("ok {name}"),
	528	+ Outcome::Skip(why) => eprintln!("skip {name} ({why})"),
	529	+ Outcome::Fail { .. } => eprintln!("FAIL {name}"),
	530	+ }
	531	+ results.lock().unwrap().insert(name, outcome);
	532	+ }
	533	+ });
496	534	}
497		- results.insert(name, outcome);
498		- }
	535	+ });
	536	+ // Reported in case order regardless of which thread finished when: a test
	537	+ // whose output depends on scheduling is a test you cannot diff.
	538	+ let results = results.into_inner().unwrap();
499	539
500	540	let mut failed = Vec::new();
501	541	let (mut pass, mut skip) = (0, 0);

  // --------------------------------------------------------------- one case
--fn run_case(case: &Path, work: &Path, nim: &Path) -> Outcome {
++fn run_case(case: &Path, work: &Path, nim: &Path, cache: &Path) -> Outcome {
      let name = case.file_stem().unwrap().to_string_lossy().into_owned();
      // For a directory case, `main.rs` is the crate root and carries the
      // directives; every `.rs` beside it is passed to rustnim as well.
              .arg("--hints:off")
              .arg("--warnings:off")
              .arg("--colors:off")
--            .arg(format!("--nimcache:{}", dir.join("nimcache").display()))
++            .arg(format!("--nimcache:{}", cache.display()))
              .arg(format!("-o:{}", nim_bin.display()))
              .arg(&nim_src)
              .env("TMPDIR", &dir),
      assert!(!cases.is_empty(), "no cases in {}", cases_dir.display());
      let filter = std::env::var("RUSTNIM_CASE").ok();
--    let mut results: BTreeMap<String, Outcome> = BTreeMap::new();
++    let cases: Vec<PathBuf> = cases
--    for case in &cases {
++        .into_iter()
--        let name = case.file_stem().unwrap().to_string_lossy().into_owned();
++        .filter(|c| match &filter {
--        if let Some(f) = &filter {
++            None => true,
--            if !name.contains(f.as_str()) {
++            Some(f) => c
--                continue;
++                .file_stem()
--            }
++                .unwrap()
--        }
++                .to_string_lossy()
--        let outcome = run_case(case, &work, &nim);
++                .contains(f.as_str()),
--        match &outcome {
++        })
--            Outcome::Pass => eprintln!("ok   {name}"),
++        .collect();
--            Outcome::Skip(why) => eprintln!("skip {name} ({why})"),
++
--            Outcome::Fail { .. } => eprintln!("FAIL {name}"),
++    // The cases are independent -- each one gets its own scratch directory and
++    // reads nothing the others write -- so they are run on several threads.
++    // Serially this suite spends its wall clock waiting on rustc and nim while
++    // seven of eight cores sit idle.
++    //
++    // Each worker keeps its *own* Nim cache, rather than each case getting a
++    // fresh one. A cold cache costs about half a second per case, all of it
++    // recompiling the same standard-library modules; one cache per worker is
++    // both warm and free of the write races a single shared cache would have.
++    let jobs: usize = std::env::var("RUSTNIM_JOBS")
++        .ok()
++        .and_then(|v| v.parse().ok())
++        .filter(|n| *n > 0)
++        .unwrap_or_else(|| {
++            std::thread::available_parallelism().map(|n| n.get()).unwrap_or(1)
++        })
++        .min(cases.len().max(1));
++
++    let next = std::sync::atomic::AtomicUsize::new(0);
++    let results: std::sync::Mutex<BTreeMap<String, Outcome>> =
++        std::sync::Mutex::new(BTreeMap::new());
++    std::thread::scope(|scope| {
++        for w in 0..jobs {
++            let (next, results, cases, work, nim) = (&next, &results, &cases, &work, &nim);
++            scope.spawn(move || {
++                let cache = work.join(format!("nimcache-{w}"));
++                loop {
++                    let i = next.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
++                    let Some(case) = cases.get(i) else { break };
++                    let name = case.file_stem().unwrap().to_string_lossy().into_owned();
++                    let outcome = run_case(case, work, nim, &cache);
++                    match &outcome {
++                        Outcome::Pass => eprintln!("ok   {name}"),
++                        Outcome::Skip(why) => eprintln!("skip {name} ({why})"),
++                        Outcome::Fail { .. } => eprintln!("FAIL {name}"),
++                    }
++                    results.lock().unwrap().insert(name, outcome);
++                }
++            });
          }
--        results.insert(name, outcome);
++    });
--    }
++    // Reported in case order regardless of which thread finished when: a test
++    // whose output depends on scheduling is a test you cannot diff.
++    let results = results.into_inner().unwrap();
      let mut failed = Vec::new();
      let (mut pass, mut skip) = (0, 0);