Measure what a doorbell and a namespace request cost

.github/workflows/ci.yml

@@ -266,6 +266,45 @@ jobs:
       - name: probe magnitudes (topology)
         if: '!cancelled()'
         run: cargo run -p windows-platform-probes --bin probe-topology --locked
+      # `--release` on the next two, and ONLY on those two, because they are the
+      # only probes here that report nanoseconds. Everything above measures
+      # BEHAVIOUR -- what an API refuses, what a handle reports, which errors
+      # appear -- and that does not change with the optimisation level, so those
+      # keep the dev profile and the build time it saves.
+      #
+      # These two do not have that luxury. They compare operations a few tens of
+      # nanoseconds apart, and in an unoptimized build the loop, the closure
+      # indirection, and the request construction around each measured call
+      # carry overhead that does not shrink uniformly across them. The RATIOS
+      # are what the design reads -- "a doorbell is x% of a submit", "a captured
+      # handle costs Nx a built request" -- and a ratio of two figures each
+      # inflated by a different amount is not the shipping one.
+      #
+      # Decides how much machinery the two-layer ring's doorbell needs. Its
+      # park-and-wake handshake is bounded rather than INFINITE on purpose: the
+      # first version of it deadlocked, because an auto-reset event does not
+      # count signals and the waiter's count never caught up. A probe that can
+      # hang is a probe that can hang a build.
+      #
+      # `if: '!cancelled()'` for the same reason the topology step above carries
+      # it, and the reason is not specific to topology: a probe step exists to
+      # produce diagnostic output, so skipping it because an earlier step failed
+      # suppresses it in precisely the run that wanted it. The test step above
+      # covers both of these probes, and a host where those tests fail is a host
+      # whose timings are worth reading.
+      #
+      # It matters more for these two than for a pass/fail probe. Both now assert
+      # every status they take and panic on a missing NDJSON label, and both go
+      # through `emit_report`, which prints what was already established before a
+      # panic -- machinery that only reaches a reader if the step runs at all.
+      - name: probe magnitudes (doorbell cost)
+        if: '!cancelled()'
+        run: cargo run -p windows-platform-probes --bin probe-doorbell-cost --locked --release
+      # Read with the doorbell probe above: together they say whether the
+      # queue's mechanics or the request's own cost deserves the attention.
+      - name: probe magnitudes (request cost)
+        if: '!cancelled()'
+        run: cargo run -p windows-platform-probes --bin probe-request-cost --locked --release
       # Both halves of the long-path pair, deliberately. Either alone says
       # nothing: the finding is the *difference* between two executables that
       # differ only in whether `build.rs` embedded the `longPathAware` manifest,

crates/windows-namespace-request-sys/RESOLVED-TEST-FAILURES.md

@@ -0,0 +1,8 @@
+# Resolved test failures: windows-namespace-request-sys
+
+Entries moved here from [UNRESOLVED-TEST-FAILURES.md](UNRESOLVED-TEST-FAILURES.md)
+once diagnosed, with what the cause turned out to be. Append-only: entries are
+never deleted, so a failure that recurs can be matched against one already
+understood.
+
+*No failures have been resolved yet.*

crates/windows-namespace-request-sys/UNRESOLVED-TEST-FAILURES.md

@@ -0,0 +1,98 @@
+# Unresolved test failures: windows-namespace-request-sys
+
+Observed failures that are real but not yet diagnosed. Recorded here rather than
+left in a CI log, so a later intermittent failure is recognised as a known one
+instead of being re-investigated from scratch or dismissed as noise.
+
+When an entry is resolved, move it to a sibling
+[RESOLVED-TEST-FAILURES.md](RESOLVED-TEST-FAILURES.md) under a
+`## Resolved <YYYY-MM-DD HH:MM:SS +hh:mm> -- <description>` heading in the same
+change that removes it from this file. Do not delete entries.
+
+## Observed 2026-09-09 15:35:14 -04:00 -- `close::tests` handle-value reuse poisons the allocation lock
+
+**Symptom.** One CI run of `cargo test --workspace` reported **96 failures** in
+this crate: `close::tests::a_caller_supplied_routine_is_carried` failed with
+`assertion failed: !was_still_open(raw)`, and the other 95 all failed with
+`the lock is not poisoned: PoisonError { .. }`. The 95 are collateral -- the
+first test panicked while holding the write guard on `handle_allocation()`,
+which poisons the lock for every test that takes it afterwards. **The count is
+alarming and the defect is singular.**
+
+**It is intermittent, and that is established rather than assumed.** The same
+commit (`b90d897`) was re-run with no change and passed. The crate passes 5/5
+locally. The two commits before it passed the identical job. The commit that
+"failed" was comment-only, in a different crate.
+
+**Mechanism, as far as it is understood.** `was_still_open` probes a raw value
+by attempting to close it:
+
+```rust
+fn was_still_open(handle: HANDLE) -> bool {
+    unsafe { CloseHandle(handle) != FALSE }
+}
+```
+
+Its own comment states the precondition: "A stale value fails with
+`ERROR_INVALID_HANDLE` rather than closing something else, **because these tests
+hold the allocation lock**." That precondition does not hold for the whole
+process. Measured on this revision: **218 tests in the crate, and 11 of them open
+handles without taking `handle_allocation()`** -- among them
+`open::tests::a_missing_path_reports_the_raw_code_unaltered`,
+`open::tests::the_overlapped_flag_is_carried_rather_than_decided`, and
+`watch::tests::a_missing_directory_reports_the_raw_code`.
+
+`cargo test` runs tests as threads in one process (deliberately -- see the root
+[DESIGN-NOTES.md](../../DESIGN-NOTES.md)), so a handle value freed by a
+lock-holding test can be immediately reallocated by one of those 11 running
+concurrently. The probe then finds the value open and the assertion fails.
+
+**The failing assertion is not the worst of it.** `was_still_open` *closes* the
+handle when the probe succeeds. So in the losing interleaving this test does not
+merely mis-report -- it closes a live handle belonging to another test, which
+can surface later as an unrelated failure somewhere else entirely. The visible
+assertion is the benign outcome.
+
+**Not caused by the change that observed it.** The branch that hit this
+(`mikegrier/probes-cost-pair`, PR #83) makes **no source change to this crate**:
+its only file here is this record. Everything else it touches is
+`crates/windows-platform-probes/`, `.github/workflows/ci.yml` and `Cargo.lock`,
+and it takes `windows-namespace-request-sys` as a new *dependency* without
+altering it.
+
+(An earlier revision of this paragraph said the branch touched only those three
+paths, which was untrue the moment it was written -- the file stating it lives
+under `crates/windows-namespace-request-sys/`. Corrected so a later reader
+checking the claim against the diff finds it holds.)
+
+**Directions for whoever picks this up**, in rough order of directness:
+
+1. Take the allocation lock in the 11 tests that open handles without it. This is
+   the smallest change and closes the measured hole, but it leaves the invariant
+   resting on every future test author remembering -- the same "a flat rule beats
+   a rule someone must remember to apply" problem recorded in the root
+   [DESIGN-NOTES.md](../../DESIGN-NOTES.md) for status checking.
+2. Make the hazard structural rather than remembered: have `Fixture` /
+   `captured_duplicate` take the lock themselves, so opening a handle *without*
+   it is not something a test can do by omission. This is the same move as
+   preferring a type that discharges a rule over a rule each author must apply.
+3. Make the lock unnecessary by not probing a raw value at all. `was_still_open`
+   exists to answer "did the close routine actually run?", and asking the routine
+   rather than the OS cannot race. **Note this is a bigger change than it sounds**
+   -- the two routines here are the real `CloseHandle` and
+   `FindCloseChangeNotification`, called directly with no shim (a deliberate
+   property, recorded in [src/close.rs](src/close.rs)), so there is nothing
+   currently observable to ask. It would mean introducing a test-only routine
+   that records into a static, which is the pattern `windows-threadpool-sys`
+   already uses for its wait targets -- see the root
+   [DESIGN-NOTES.md](../../DESIGN-NOTES.md) -> "Testing it needs per-test statics,
+   not one global counter", which also documents why those statics must be
+   per-test rather than at module scope, for exactly this concurrency reason.
+
+Direction 1 stops the bleeding today; direction 2 is the smallest change that
+stops it recurring. Direction 3 is the most thorough and touches the most.
+
+(An earlier revision of this list claimed the close routines "already have
+observation statics". They do not -- that is `windows-threadpool-sys`'s pattern,
+imported here by mistake. `was_still_open`, used at seven sites in
+`close/tests.rs`, is the only mechanism this crate has for the question.)

crates/windows-platform-probes/CHECKLIST.md

@@ -137,3 +137,72 @@ speculative list to extend by imagination -- a fourth is added when a fourth con
   than noise: across two rounds one reader raised this twice while two others cleared it, one of them
   explicitly after being pointed at the question. Nothing in the suite decides it either way, which is
   itself the argument for the oracle.
+
+- [ ] **M2.6** -- Say precisely what `GetFullPathNameW` does, in the crate that owns it, and decide
+  whether it is still the call `prepare` wants. Two successive descriptions in the cost probe were
+  each wrong in the same direction: *a syscall cost*, which a timing loop cannot establish, and then
+  *lexical*, which it also is not. The probe now states the cost and declines the mechanism, which is
+  honest but leaves the question open one layer down.
+
+  [../windows-namespace-request-sys/src/full_path.rs](../windows-namespace-request-sys/src/full_path.rs)
+  carries the same imprecision, and is the crate that owns the answer: its module doc says "This call
+  is **lexical**. It resolves relative components and `.`/`..` against the process current
+  directory". Those two sentences disagree -- consulting the current directory is process state, and
+  for a drive-relative path (`C:foo`) it also reads the per-drive current directory held in the
+  `=C:` environment variables. "Touches no filesystem" is the claim that holds; "lexical" is not.
+
+  **The mono-repo rule says fix the layer, so the correction belongs in
+  `windows-namespace-request-sys`, not in the probe that consumes it.** It is queued rather than
+  taken because that crate is outside this peel and is release-managed, so a docs change there is its
+  own commit with its own scope.
+
+  The decision half is the part worth an engineer's attention rather than a sweep. A genuinely
+  lexical canonicalizer exists -- `PathCchCanonicalizeEx`, or `PathAllocCanonicalize` -- and would be
+  cheaper, with no process state read at all. **It is very likely the wrong call anyway**, because
+  resolving against the current directory *at submission* is the property the namespace design is
+  buying: the CWD is shared mutable state, so a relative path means something different depending on
+  when it is resolved, and pinning that on the submitting thread is the whole point. Record that
+  conclusion explicitly, with the alternative named, so the next reader does not re-derive it -- and
+  if it is wrong, the cheaper call is sitting there.
+
+  Also worth settling while the question is open: whether `GetFullPathNameW` can enter the kernel at
+  all on any path this crate takes. The probe measured ~212 ns for a build on x86_64 and declines to
+  say what that is made of; the owning crate could say, and a reader of either would then stop
+  guessing.
+
+- [ ] **M2.7** -- Decide whether the other nine probe steps in CI should carry `if: '!cancelled()'`,
+  and apply or record the decision.
+
+  **Measured 2026-09-09:** twelve probe steps in [ci.yml](../../.github/workflows/ci.yml), of which
+  three are guarded -- topology, and the doorbell/request pair added with this note. The other nine
+  (`error mode`, `handle state`, `worker context`, `pool growth`, `device map`, `IoRing`,
+  `completion port`, and both halves of the long-path pair) are skipped whenever an earlier step in
+  the job fails, because Actions defaults to `if: success()`.
+
+  The argument for guarding is already written at the topology step and is not specific to it: a
+  probe step exists to emit diagnostics, so skipping it on failure suppresses it in exactly the run
+  that wanted it. **The long-path pair is the sharpest case** -- its own comment says either half
+  alone "says nothing", since the finding is the difference between two executables, so a partial
+  run of that pair is worse than useless.
+
+  **It is queued rather than done because there is a real tradeoff, and it is an operational call.**
+  `!cancelled()` also runs the step when the *build* failed, where `cargo run` cannot compile and
+  the step turns from skipped (grey) into failed (red). That trades quieter broken-build output for
+  better broken-test output. The topology step already took that trade; whether all twelve should is
+  a judgement about how the CI log is read, not something to settle by consistency alone.
+
+- [ ] **M2.8** -- Carry the OS error in the remaining Win32 assertion messages.
+
+  `last_os_error()` (or a raw `GetLastError`) is in the messages in `doorbell_cost`, `request_cost`
+  and `handle_state`, and missing from four sites in probes this peel did not touch:
+  `completion_port.rs:224` and `:234` ("create a completion port"), `ioring.rs:320` ("create the
+  probe pipe"), and `pool_growth.rs:62` ("create the gate event"). Each says what was being attempted
+  and not why it failed, which is the whole of what a CI log can offer someone who cannot rerun under
+  a debugger.
+
+  Two rules worth carrying over, both learned the expensive way in this peel. Read the error
+  **immediately after the single call whose failure is reported** -- a code attached to a condition
+  spanning two calls belongs to whichever ran last, not whichever failed, and can print "The
+  operation completed successfully" under a message saying something failed. And attach it only to a
+  condition that is genuinely an OS failure: a call that returned a size rather than an error should
+  not carry one, since `GetLastError` says nothing about it.

crates/windows-platform-probes/Cargo.toml

@@ -51,6 +51,14 @@ path = "src/bin/pool_growth.rs"
 name = "probe-topology"
 path = "src/bin/topology.rs"
 
+[[bin]]
+name = "probe-doorbell-cost"
+path = "src/bin/doorbell_cost.rs"
+
+[[bin]]
+name = "probe-request-cost"
+path = "src/bin/request_cost.rs"
+
 # These two are the same code, and that is the measurement: they differ only in
 # whether `build.rs` embeds the `longPathAware` manifest, which is not a runtime
 # switch and so cannot be a flag on one binary.
@@ -87,6 +95,10 @@ windows-topology-sys = { path = "../windows-topology-sys" }
 # somewhere and compared against something it does not describe. That banner is
 # `windows-placement-probe`'s to render, not a second copy here.
 windows-placement-probe = { path = "../windows-placement-probe" }
+# The request-cost probe measures the real request type the design would put on
+# a queue, not a stand-in, for the same reason the topology probe reads the
+# shipping parse: a reimplementation would measure the reimplementation.
+windows-namespace-request-sys = { path = "../windows-namespace-request-sys" }
 # The long-path probe measures a length against `MAX_PATH`, and `MAX_PATH` counts
 # UTF-16 code units. `OsStr::len` counts Rust's platform encoding -- WTF-8 here --
 # so the two disagree the moment a non-ASCII character appears in `%TEMP%`, which
@@ -115,6 +127,9 @@ features = [
 	"Win32_System_Diagnostics_Debug",
 	"Win32_System_IO",
 	"Win32_System_Pipes",
+	# GetSystemDirectoryW, so the request probe measures the real system
+	# directory instead of assuming Windows is installed on C:.
+	"Win32_System_SystemInformation",
 	"Win32_System_SystemServices",
 	"Win32_System_Threading",
 ]