Skip to content

refactor(hm-exec): Model parallelism as NonZeroUsize instead of usize coerced with .max(1)#116

Merged
markovejnovic merged 2 commits into
mainfrom
cq/hm-exec-model-parallelism-as-nonzerousize-instea-5
Jun 10, 2026
Merged

refactor(hm-exec): Model parallelism as NonZeroUsize instead of usize coerced with .max(1)#116
markovejnovic merged 2 commits into
mainfrom
cq/hm-exec-model-parallelism-as-nonzerousize-instea-5

Conversation

@markovejnovic

Copy link
Copy Markdown
Contributor

Smell

LocalBackend::new(parallelism: usize, ..) (crates/hm-exec/src/local/backend.rs:39) stored a raw usize, and the scheduler later defended against zero with let parallelism = parallelism.max(1); before Semaphore::new(parallelism) (crates/hm-exec/src/local/scheduler.rs:136).

A tokio::sync::Semaphore::new(0) would deadlock every step, so the invariant parallelism >= 1 is 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::new now takes parallelism: NonZeroUsize, stored as-is on the struct.
  • scheduler::run takes parallelism: NonZeroUsize and constructs the semaphore as Semaphore::new(parallelism.get()) — panic-/deadlock-free by construction. The .max(1) coercion and its comment are gone.
  • The CLI boundary (crates/hm/src/commands/run/mod.rs, resolve_parallelism) now produces the NonZeroUsize once: an explicit --parallelism 0 is clamped via NonZeroUsize::new(n).unwrap_or(NonZeroUsize::MIN), and the auto-detected fallback uses available_parallelism() (already NonZeroUsize). Behavior is preserved exactly: 0/absent still becomes 1/CPU-count.
  • The crate's contract test was updated to pass 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 by fd: a count semantically required to be >= 1 should 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 --tests was run locally (it passes). Full CI runs on this PR — in particular the hm crate's build and the TS/Python DSL build were not exercised locally.

@markovejnovic markovejnovic marked this pull request as ready for review June 10, 2026 19:51
@markovejnovic markovejnovic merged commit 5c26d82 into main Jun 10, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant