refactor(hm-exec): Model parallelism as NonZeroUsize instead of usize coerced with .max(1)#116
Merged
markovejnovic merged 2 commits intoJun 10, 2026
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Smell
LocalBackend::new(parallelism: usize, ..)(crates/hm-exec/src/local/backend.rs:39) stored a rawusize, and the scheduler later defended against zero withlet parallelism = parallelism.max(1);beforeSemaphore::new(parallelism)(crates/hm-exec/src/local/scheduler.rs:136).A
tokio::sync::Semaphore::new(0)would deadlock every step, so the invariantparallelism >= 1is real — but it was enforced only by a runtime coercion buried at the call site, and the doc comment (`0` is coerced to `1`) documented the footgun rather than removing it.Change
LocalBackend::newnow takesparallelism: NonZeroUsize, stored as-is on the struct.scheduler::runtakesparallelism: NonZeroUsizeand constructs the semaphore asSemaphore::new(parallelism.get())— panic-/deadlock-free by construction. The.max(1)coercion and its comment are gone.crates/hm/src/commands/run/mod.rs,resolve_parallelism) now produces theNonZeroUsizeonce: an explicit--parallelism 0is clamped viaNonZeroUsize::new(n).unwrap_or(NonZeroUsize::MIN), and the auto-detected fallback usesavailable_parallelism()(alreadyNonZeroUsize). Behavior is preserved exactly:0/absent still becomes1/CPU-count.NonZeroUsize::new(4).unwrap().This is behavior-preserving: the same effective parallelism is selected, but zero is now unrepresentable past the CLI boundary instead of being silently corrected at the use site.
Type-safety pattern
This applies the
NonZeroUsize-for-counts pattern used byfd: a count semantically required to be>= 1should be made unrepresentable as zero at the type level rather than guarded with a runtime check at every use site. The invariant moves into the type, the semaphore construction is provably safe, and the silent coercion disappears.CI note
Only
cargo check -p hm-exec --testswas run locally (it passes). Full CI runs on this PR — in particular thehmcrate's build and the TS/Python DSL build were not exercised locally.