Repository navigation
Expose the pool, require the ps-st3 that has the wake fixes - #2
Merged
Merged
Conversation
added 8 commits
September 10, 2026 14:43
Selecting an idle policy is a nagoya-level decision: a caller picks it when it builds a runtime, through nagoya's own API. But `Tuning` is `ps-st3`'s type and nagoya did not re-export it, so every such caller had to add `ps-st3` as a direct dependency to spell one struct, in a graph that already contains it transitively. WorkTable did exactly that.
`Runtime::new` is the only way to get threads that call `task::mark_current`, and that marker is the whole mechanism `local_wakes` depends on: without it a wake takes the injector no matter what the tuning says. `mark_current` is private, so a caller wanting a pool at a non-default tuning had to build the `Pool` itself, and got a pool that silently ignores the half of the tuning it was chosen for. That is not a small difference. The tuning is set, the pool runs, and the behaviour it asks for never happens, so a comparison between two such pools would attribute to the scheduler a difference that was only ever a missing thread-local. WorkTable has been running every flavor but its default this way. `with_tuning` takes the count, the tuning and a thread-name label, and `new` becomes it at the default tuning. Two tests, because one would not be evidence. A locality-tuned `Runtime` polls a self-waking task on exactly one thread. A hand-built pool at the same tuning, differing only in the missing `mark_current`, polls it on more than one. The second is what makes the first mean something.
`par_for` takes an `Arc<Pool>` because it schedules closures and never builds a task, so a caller holding only a `Runtime` could not use it. Every data-parallel benchmark worked around that by building its own `Pool`, which meant its threads were never marked as pool workers, and that marker is the only thing that makes `local_wakes` do anything. Thirteen benchmarks were therefore running the default tuning on unmarked threads and could not see a flavor at all.
0.6.1 carries three fixes 0.6.0 does not: a burst of submits all naming the same sleeper, private work waking a worker that cannot run it, and a nudge whose condition could not be true at the default injector batch size. The requirement was ^0.6, which accepts 0.6.0. A lockfile pinning it would build a pool without any of them, and nothing would say so: the crate compiles either way. Every scheduler measurement made against this crate would quietly stop being reproducible. Measured on the fixes this pins, YCSB C at 16 threads: cores kept busy went from 7.5 to 12.9 against tokio's 12.8.
This crate has no repository-level CARGO_REGISTRY_TOKEN, and every publish run so far short-circuited on "version unchanged, nothing to publish", so the upload path has never run here. 0.1.1 would be the first time the token is exercised. An absent token surfaces as an authentication error from cargo publish after the package is already built, which reads like a registry problem rather than a missing secret. The guard says which it is, and where to set it. This is the crate the release order starts at: worktable requires nagoya ^0.1.1 and nagoya requires ps-st3 ^0.6.1, so a publish that fails obscurely here blocks everything and looks like it came from somewhere else. Whether an organisation-level secret already covers this repository is not readable from a repository-level secret listing, so the guard reports rather than assumes.
The requirement was ^0.6.1 to stop a lockfile resolving 0.6.0, which lacks the three wake fixes. Carets for crates we control is the rule, and a floor is a pin wearing a different hat. Cargo takes the highest compatible version anyway, so ^0.6 gets 0.6.1 once it publishes. It also means this branch resolves against the registry today rather than being red until then.
Applied from the four-patch series in ~/code/patch, whose author states plainly that nothing was compiled or run. It is compiled and run now: the crate builds, all 65 tests pass including the 15 new regression tests the series brings, `--no-default-features` still checks, and WorkTable's 999 tests pass against this tree, which is what its `[patch.crates-io]` points at. What the series covers, from the review: Cancelled and stale semaphore waiters no longer consume wakes; multiple notifications no longer collapse into one; registrations are removed on completion and on drop, for borrowed and owned RwLock waits too. Cancelled timers no longer sit in the heap until their deadlines. The unindexed heap is now an indexed one, so removal is O(log n) with slot indices maintained under the registry mutex. A panic no longer removes a worker or strands a parallel completion. Panic propagation is enabled through the existing `std` feature rather than a new dependency, parallel-piece bodies catch and rethrow at the awaiting future, and an RAII token retires a piece on execution, cancellation, unwind, and on dropping an unrun queue entry. Queued jobs no longer own the pool that owns them. Scheduling closures and parallel pieces hold `Weak` references, and `FallibleTask` lets a cancelled runnable produce the `None` join result the signature already declared. `no_std` and custom hosts can activate locality: `WorkerContext` and `with_worker_context` without a `std` dependency, plus `run_worker` for a caller-owned thread. The automatic worker count is clamped to `usize::BITS`, and an explicitly oversized count is rejected before host resources are created. Reversed empty `par_for` ranges complete before queue publication rather than underflowing. Whole-buffer I/O retries `Interrupted` while preserving progress. The only manifest change is `async-task/std` inside the existing `std` feature. Nothing became unconditionally std. Two caveats the series carries and I have not verified beyond compiling: panic isolation requires std and unwinding, so abort-on-panic builds still abort, and timers are not interrupt-safe. A host following the old tick-interrupt documentation has to move registry processing to deferred execution.
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
1 participant
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.
Three commits: a pool accessor the runtime work needed, the
ps-st3requirement that keeps the wake fixes from being silently dropped, and a guard on the publish token.Runtime::pool()with_tuningbuilds a marked pool, andnagoya::task::mark_currentis private, so a hand-builtPoolsilently runs locality tunings as spread: the tuning is set, the pool works, and the behaviour it asks for never happens. Exposing the pool an executor runs on is what lets a consumer route work per class without rebuilding that marking by hand.ps-st3 ^0.6.1, not^0.60.6.1 carries three wake fixes that 0.6.0 does not (pathscale/ps-st3#5): a burst of submits all naming the same sleeper, private work waking a worker that cannot run it, and a nudge whose condition could not be true at the default injector batch size.
^0.6accepts 0.6.0. A lockfile pinning it would build a pool without any of them, and nothing would say so, because the crate compiles either way. Every scheduler measurement made against this crate would quietly stop being reproducible.Measured on the fixes this pins, YCSB C at 16 threads through WorkTable: cores kept busy went from 7.5 to 12.9, against tokio's 12.8.
This makes the branch red until ps-st3 0.6.1 publishes. That is the requirement working, not a break:
nagoyacannot resolve from the registry until the crate it depends on is there. Publish order is ps-st3 0.6.1, then nagoya 0.1.1, then worktable 1.9.0-alpha1.A guard on the publish token
This crate has no repository-level
CARGO_REGISTRY_TOKEN, and every publish run so far short-circuited on "version unchanged at 0.1.0, nothing to publish", so the upload path has never actually run here. 0.1.1 would be the first time the token is exercised.An absent token surfaces as an authentication error from
cargo publishafter the package is built, which reads like a registry problem rather than a missing secret, on the crate the whole release order starts at. The guard says which it is and where to set it.