From 17e638958f2d565af7d140baf869f68ab43bf60c Mon Sep 17 00:00:00 2001 From: Mike Grier Date: Tue, 8 Sep 2026 23:37:23 -0400 Subject: [PATCH 1/7] feat(placement-probe): render a banner from a fingerprint already discovered `banner_line` discovers the host and renders it in one step, so a caller that must BRACKET the discovery -- reading the host before and after a measurement to detect one that changed under it -- had nowhere to put readings it already held. Formatting `host: {fingerprint}` at the call site would put a second copy of this line's shape in another crate, and a probe's banner is comparable with every other probe's only while exactly one place produces it. `banner_line_for` is that one place, and `banner_line` is now expressed in terms of it. It takes the `std::io::Result` rather than the `Fingerprint`, which is the point rather than a convenience: `banner_line` renders both outcomes into one string, so a caller comparing two of those strings cannot tell a host that changed from a discovery that failed -- and two failures whose `io::Error` text differs compare unequal while establishing nothing about the machine at all. --- .../src/fingerprint.rs | 27 +++++++++++- .../src/fingerprint/tests.rs | 41 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/crates/windows-placement-probe/src/fingerprint.rs b/crates/windows-placement-probe/src/fingerprint.rs index ab9a1b719..7eb0bd6f4 100644 --- a/crates/windows-placement-probe/src/fingerprint.rs +++ b/crates/windows-placement-probe/src/fingerprint.rs @@ -868,7 +868,32 @@ pub fn places_from_topology( /// to rest on a human having read the format string. #[must_use] pub fn banner_line() -> String { - match Fingerprint::discover() { + banner_line_for(&Fingerprint::discover()) +} + +/// The banner line for a fingerprint that has already been discovered. +/// +/// Split out of [`banner_line`] for a caller that must BRACKET the discovery -- +/// reading the host before and after a measurement, so two readings that differ +/// are visible. Such a caller holds the readings already, and formatting +/// `host: {fingerprint}` itself would put a second copy of this line's shape +/// in another crate; a probe's banner is comparable with every other probe's +/// only while exactly one place produces it. +/// +/// Bracketing establishes that the two readings DIFFER, not that the host +/// changed: [`Fingerprint::discover`] returns `Ok` on a parse that dropped a +/// record or whose sources disagreed, so consecutive fingerprints can differ +/// because the enumeration was flaky. Naming a cause is the caller's business +/// and no caller can name that one. +/// +/// **Takes the `Result` rather than the `Fingerprint`, which is the point.** +/// [`banner_line`] renders both outcomes into one string, so a caller comparing +/// two of those strings cannot tell two differing readings from a discovery +/// that failed -- two failures carrying different `io::Error` text differ as +/// strings while establishing nothing about the machine at all. +#[must_use] +pub fn banner_line_for(discovered: &std::io::Result) -> String { + match discovered { Ok(fingerprint) => format!("host: {fingerprint}"), Err(error) => format!("host: UNKNOWN -- topology discovery failed: {error}"), } diff --git a/crates/windows-placement-probe/src/fingerprint/tests.rs b/crates/windows-placement-probe/src/fingerprint/tests.rs index 5afa32540..5031c30aa 100644 --- a/crates/windows-placement-probe/src/fingerprint/tests.rs +++ b/crates/windows-placement-probe/src/fingerprint/tests.rs @@ -1341,3 +1341,44 @@ mod multi_group_conversion { } } } + +#[test] +fn a_banner_for_an_already_discovered_fingerprint_renders_both_outcomes() { + // `banner_line_for` exists so a caller that BRACKETS the discovery -- reading + // the host before and after a measurement, to detect one that changed under + // it -- can render the readings it already holds, rather than formatting this + // line a second time somewhere else. A failed read stays distinguishable, + // which is the whole reason it takes the `Result`: a caller comparing two + // rendered lines cannot otherwise tell a host that moved from one that could + // not be read. + // The two are compared on ONE reading, not on two. Asserting + // `banner_line_for(&Ok(discovered)) == banner_line()` re-reads the host + // inside the assertion, so a machine that changed between them fails a test + // about string formatting -- in the crate whose `attribution` exists + // precisely because two consecutive discoveries can differ. The property + // under test is that one place owns the format, and that is shown by + // rendering the same value twice. + let fingerprint = Fingerprint::discover().expect("this machine must be discoverable"); + let rendered = super::banner_line_for(&Ok(fingerprint.clone())); + + assert_eq!( + rendered, + super::banner_line_for(&Ok(fingerprint.clone())), + "the rendering is a function of the reading alone" + ); + assert!( + rendered.starts_with("host: "), + "and it is the shape `banner_line` has always produced: {rendered}" + ); + assert!( + rendered.contains(&fingerprint.to_string()), + "got {rendered}" + ); + + let failed = super::banner_line_for(&Err(std::io::Error::other("no topology"))); + assert!(failed.starts_with("host: "), "got {failed}"); + assert!( + failed.contains("UNKNOWN") && failed.contains("no topology"), + "a failed read says so, and says why: {failed}" + ); +} From f14c1bce986bb4f8be6dc8f4b8ef021b2f0823a2 Mon Sep 17 00:00:00 2001 From: Mike Grier Date: Tue, 8 Sep 2026 23:37:24 -0400 Subject: [PATCH 2/7] feat(platform-probes): survey the machine topology and cross-check the shipping parse Adds `probe-topology`: a binary that prints the machine's processor, NUMA and cache shape and what each candidate partitioning policy would yield, plus one `x-probe-topology` NDJSON line so accumulated CI output can be mined without parsing the prose. CI already runs it on every build, which turns the hosted-runner fleet into a slow survey of what real machines look like -- including the negative result that cloud runners are consistently single-node. The parse comes from `windows_topology_sys::MachineMemoryTopology::discover` rather than a reimplementation here, for the same reason the pool-growth probe uses the real thread-pool crate: a reimplementation would measure the reimplementation. Three Win32 counters are then read INDEPENDENTLY -- GetActiveProcessorCount, GetActiveProcessorGroupCount and GetNumaHighestNodeNumber -- so the probe doubles as a cross-check on that crate's parsing across every machine CI ever runs on. `measure` reads the counters and `observe` reduces a topology plus those counters to an `Observation`, split so the reduction is testable without a host. The design problem this probe is mostly made of is saying only what a run established. `cross_check` returns three lists, deliberately not merged, because they are claims about different things with different owners: `disagreements` (a counter was compared and did not match -- a finding about the shipping crate), `not_compared` (a counter could not be read -- a gap in this measurement), and `parse_incomplete` (the parse is short or disputed between its two sources). `Agree` requires all three empty; only `disagreements` yields `Disagree`, because an incomplete parse is not a wrong one and reporting it as a divergence would send a reader to audit a mismatch that does not exist. `Verdict` is an enum rather than a bool, and the NDJSON carries a tri-state `"cross_check"`, so neither a renderer nor a mining pass can treat "two things checked out and the third was never established" as agreement. The same discipline shapes the rest. Zero from either processor counter is a failed read rather than a count. Node numbers are compared highest-against-highest and taken from whichever source reported them, since `NumaNodeIndex` is machine-wide from both -- a count against `highest + 1` would call a sparse-but-correct machine a regression. Every domain-policy count is clamped, and the clamp is the contract rather than defensiveness: a fleet sized at zero domains performs no I/O. The outermost partitioning cache is asked of the crate, never re-derived, because a higher cache level is not always the coarser one. `PartitioningCache` gives the absent answer four distinct forms so neither the prose nor the JSON can report "no cache level partitions this machine" -- a claim about hardware -- when the truth is that no level was reported, or that a level decoded to nothing, or that the survey lost a summary. `CrossCheck::parse_in_doubt` is the single gate on every hardware conclusion the renderer draws, and it deliberately excludes `not_compared`, because a counter this probe could not read says nothing about the parse. The host banner is bracketed the same way, and for the same reason: the fingerprint is a topology rendering rather than a name, so a machine that changed across the run would print one shape above a body describing another. It is read before and after through `Fingerprint::discover` rather than `banner_line`, so a failed read stays distinguishable from a host that moved. Tests pin the invariants that must hold on any machine, since almost every number here is host-specific and there is nothing to assert about its value. The three DESIGN-NOTES decisions record the three-list taxonomy, why the banner is bracketed, and why a verdict other than `Agree` fails CI even though a flaky hypervisor can produce one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 23 + Cargo.lock | 1 + crates/windows-platform-probes/Cargo.toml | 8 + .../windows-platform-probes/DESIGN-NOTES.md | 309 ++ .../src/bin/topology.rs | 50 + crates/windows-platform-probes/src/lib.rs | 3 + crates/windows-platform-probes/src/tests.rs | 3460 +++++++++++++++++ .../windows-platform-probes/src/topology.rs | 1430 +++++++ .../src/topology_report.rs | 584 +++ 9 files changed, 5868 insertions(+) create mode 100644 crates/windows-platform-probes/src/bin/topology.rs create mode 100644 crates/windows-platform-probes/src/topology.rs create mode 100644 crates/windows-platform-probes/src/topology_report.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 211c59e3a..67895fa08 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -243,6 +243,29 @@ jobs: run: cargo run -p windows-platform-probes --bin probe-ioring --locked - name: probe magnitudes (completion port) run: cargo run -p windows-platform-probes --bin probe-completion-port --locked + # Printed on every build on purpose. Hosted runners are a heterogeneous + # fleet, so accumulating this across builds turns ordinary CI into a slow + # survey of what real machines look like -- and the negative result, that + # cloud runners are consistently single-node, is itself evidence for how + # the execution-domain design should size itself by default. The probe + # emits one `x-probe-topology` JSON line so the results can be mined out + # of logs mechanically rather than read by eye. + # + # `if: '!cancelled()'` is what makes "on every build" true. The test step + # above asserts the cross-check reaches `Agree`, and the host where that + # assertion FAILS is the one whose report is worth the most -- yet Actions + # skips later steps in a failed job, so without this the report is + # suppressed in exactly the case it exists for. The test's own failure + # carries the `CrossCheck` and nothing else: not the domain counts, the + # enumeration anomalies, or the machine-readable JSON line. + # + # It is a SECOND measurement, not a rendering of the one that failed -- + # the test and this binary each call `measure()`. So it reproduces a + # condition the host holds persistently, which is the case worth + # investigating, and does not recover a transient one. + - name: probe magnitudes (topology) + if: '!cancelled()' + run: cargo run -p windows-platform-probes --bin probe-topology --locked # 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, diff --git a/Cargo.lock b/Cargo.lock index 55c81d5c1..140169c83 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -219,6 +219,7 @@ dependencies = [ "windows-placement-probe", "windows-sys", "windows-threadpool-sys", + "windows-topology-sys", "wtf-string", ] diff --git a/crates/windows-platform-probes/Cargo.toml b/crates/windows-platform-probes/Cargo.toml index d7f7f6e04..88cf25858 100644 --- a/crates/windows-platform-probes/Cargo.toml +++ b/crates/windows-platform-probes/Cargo.toml @@ -47,6 +47,10 @@ path = "src/bin/ioring.rs" name = "probe-pool-growth" path = "src/bin/pool_growth.rs" +[[bin]] +name = "probe-topology" +path = "src/bin/topology.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. @@ -74,6 +78,10 @@ path = "src/bin/long_path_unaware.rs" # reimplementation of the SDK's inline environment helpers, so it depends on the # real crate. windows-threadpool-sys = { path = "../windows-threadpool-sys" } +# Same reason: the topology probe measures what the shipping parse produces, not +# a second parse written here, which would only measure itself. The raw Win32 +# counters it cross-checks against are read independently through windows-sys. +windows-topology-sys = { path = "../windows-topology-sys" } # Every probe's report opens with a line naming the machine that produced it and # whether the measurement is tainted, so a captured finding cannot be pasted # somewhere and compared against something it does not describe. That banner is diff --git a/crates/windows-platform-probes/DESIGN-NOTES.md b/crates/windows-platform-probes/DESIGN-NOTES.md index b0b88b748..217c83d27 100644 --- a/crates/windows-platform-probes/DESIGN-NOTES.md +++ b/crates/windows-platform-probes/DESIGN-NOTES.md @@ -140,6 +140,27 @@ test suite for this workspace's crates: a probe answers "what does Windows do?", never "does our code work?". A probe that starts asserting our own behaviour belongs in the crate that owns that behaviour. +**One carve-out, and it is narrow.** A probe may compare a workspace crate's +reading of the platform against a *second, independent* reading this probe takes +itself -- see [the topology cross-check](#d-topology-three-lists), which reads +`GetActiveProcessorCount` and friends directly and compares them with what +`windows-topology-sys` parsed out of `GetLogicalProcessorInformationEx`. The +subject is still the machine; the crate's parse is one of two readings of it, +and a divergence is a finding about the platform-facing code precisely because +the second reading came from the platform. + +What makes it belong here rather than in the owning crate is the thing the +owning crate's own tests cannot get: CI runs these probes across a +heterogeneous hosted-runner fleet, so the comparison happens on machines nobody +enumerated in advance. A unit test in `windows-topology-sys` can only assert +against topologies someone thought to construct. + +The boundary this preserves: the probe never asserts a value it did not read +from the platform. It has no expected core count, no golden topology, no +knowledge of what the crate *should* have said -- only two readings and whether +they agree. A probe that grew a hard-coded expectation would be back on the +wrong side of the rule above. + ## The earlier probes are migrated, and two of them corrected in the move @@ -567,3 +588,291 @@ host -- Windows 11 build 26200, `cmd.exe` 10.0.26100.1 -- `cmd` carries there says nothing about the ceiling. That is a fact about that binary on that build rather than about `cmd` for all time, which is exactly why the probe rests on a binary this workspace builds and manifests itself. + +## The topology cross-check has three lists, because they have three owners + + + +The topology probe measures what `windows-topology-sys` parsed and compares it +against three Win32 counters read independently. What it may then *claim* is a +`Verdict` of `Agree`, `Disagree`, or `Incomplete`, derived from three lists that +are deliberately not merged: + +- `disagreements` -- a counter was compared and did not match. A finding about + the shipping crate's parse. +- `not_compared` -- a reading this probe could not make or could not trust. A + gap in this measurement, and nothing at all about the parse. (The causes are + not enumerated here. This bullet once named two of them, and a machine that + changed under the run -- a bracket that closed on two *different* instants, + which is neither of the two named -- had already falsified the pair.) +- `parse_incomplete` -- the parse is short, or its claims are mutually + inconsistent. Neither of the above: nothing this probe read was + contradicted, and nothing it wanted to read was missing. Established from the + parse rather than from any counter, which is why no counter agreeing can + retire an entry here. + + Note the owner is the *parse*, not "what the crate said about itself". That + narrower reading held only while every entry happened to be a crate + self-assessment; the CPU-Sets-only NUMA domain below is derived by the probe, + from provenance the crate carries but draws no conclusion about. + +Collapsing any pair of these produced a shipped defect, each caught in a +separate review round on the same branch. Merging the first two let a failed +`GetNumaHighestNodeNumber` print the report's agreement line directly below +"GetNumaHighestNodeNumber : failed". Omitting the +third let a parse the crate had *already reported as incomplete* satisfy all +three counters and reach `Agree` -- worse, because that evidence was in hand +rather than needing another call to fetch. Treating a counter's zero as a count +inverted the blame, reporting a failed read as though the crate had parsed the +machine wrongly. + +The same inversion reached the NUMA node numbers. `highest_numa_node` was taken +from the relationship walk's label alone, so a domain that only CPU Sets +described -- which `fold_memberships` pushes as its own domain, because "the +walk not describing it is a fact about the walk, not evidence the relation is +not there" -- raised the domain count while being unable to raise the highest. +The gap against the machine-wide `GetNumaHighestNodeNumber` was then filed as a +`disagreement`: an accusation against the crate for a label this probe had +discarded, with the contradiction printed in the same report. The maximum is +now taken across every label a PLATFORM source reported, which is sound because +`NumaNodeIndex` is machine-wide from both of them -- unlike `CoreIndex`, which +is group-relative and is why the labels are not interchangeable in general. + +"Every observation's label" was the first correction and went one step too far. +`Source::Description` is not a platform source, so a caller annotating a domain +the walk had already reported -- which is platform-backed, and therefore does +not close the provenance gate -- could raise the maximum above anything Windows +said and have the difference filed against the shipping parse. The filter is +what keeps the comparison a comparison of two platform readings. + +That left a real finding needing somewhere to go, and it became a +`parse_incomplete` cause: a memory domain only one source described means the +two sources group NUMA membership differently. Nothing else reaches it. +`coherence` compares PROCESSOR SETS, so two sources can name exactly the same +processors and still disagree about nodes; and the node totals can match while +the membership does not, so no counter sees it either. + +The rule is fiat rather than derived: **`Agree` requires all three lists +empty**, so anything `cross_check` pushes blocks it, whether or not a counter +noticed. Only `disagreements` yields `Disagree`, because an incomplete parse is +not a wrong one and reporting it as a divergence sends a reader to audit a +mismatch that does not exist. + +**Stated over the lists, not over their causes, and deliberately so.** +`cross_check`'s body is the single enumeration of what fills `parse_incomplete`, +and this note does not reproduce it. Read the body. + +This section is itself the worked example, twice. An earlier revision stated the +rule as "anything other than an empty anomaly list and `Coherence::Agreed`", +which was true when written; a third cause was added without sweeping the +restatements, and this note then transcribed the stale pair as settled +fiat. As a biconditional it had become false, and the danger ran the wrong way: +the guidance below tells a reader not to loosen the CI assertion, but nothing +would have stopped one *tightening the code* by deleting a branch this document +did not mention. A rule phrased over the lists cannot rot that way, because a +fourth cause satisfies it without anyone remembering to edit prose. + +The rule was then correctly restated -- and a *list of the current causes* was +left behind in both this note and `cross_check`'s own rustdoc, hedged with +"treat that as the current contents rather than the rule". A fourth cause was +added one commit later and neither list was swept, so the fix rotted inside two +rounds in the same paragraph that diagnosed the rot. The hedge did not help, +because the rustdoc carried no hedge at all. The lesson is stronger than the one +first drawn: a list of causes kept beside the rule is not a summary of the body, +it is a second copy of it that nothing checks, and the durable answer is not to +keep one. + +The `coherence` match is exhaustive for the same reason at the type level: a +variant added later is a compile error rather than a silent new path to "parsed +this machine consistently". + +`Verdict` is an enum rather than a `bool` for the same reason, and the NDJSON +carries a `"cross_check"` string rather than a boolean -- `Verdict`'s three +values, plus `not_measured` on the row emitted when discovery itself failed: +a log-mining pass must be able to tell "everything checked out" from "two +things checked out and the third was never established". `cross_check_ok:true` +said the same thing for both. + +**`cross_check == "agree"` is the one field a mining pass must read before +trusting any other.** Every count on that line comes from what decoded, so a +record Windows returned that did not fully decode leaves `caches`, `packages`, +`cores` and `outermost_partitioning_cache_level` wrong by an amount no field +states, and a query grouping by cache level has no reason to join against +`enumeration_anomalies`. The verdict closes that. + +**It closes that, and no more: `agree` means no record FAILED TO DECODE, not +that the counts are complete.** Nothing independent measures packages, cores or +caches, so a record that decoded cleanly while describing less of the machine +than exists -- a package covering half the online processors, a cache level +whose one domain covers half of them -- raises no anomaly and reaches `agree`. +That gap is deliberate and open: a coverage check would have to hold on every +machine in the runner fleet, and by the decision below any verdict other than +`Agree` fails the build, so a check this probe cannot validate beyond its own +host would fail builds for hosts that are reporting themselves correctly. +`agree` says every check this probe could make was made and matched -- never +that a check exists for every field on the line. The report's own comment above +`x-probe-topology` says the same thing, and the two are meant to be read +together. + +Note "wrong", not "short". A record that decodes to nothing is dropped and +shortens a count, but a `TruncatedArray` record is *kept* with the entries that +fit -- so a cache record with a partial affinity mask presents a processor set +smaller than the truth, which `cache_partitions_at_level` counts as its own +distinct partition and which therefore INFLATES a domain count. An anomaly does +not tell you the direction, and nothing in the report claims to. + +**The verdict closes that and no more: `agree` means the counts are not +DISTORTED, not that every field is a plain hardware fact.** `outermost_partitioning_cache_level` is +where the difference bites. `windows-topology-sys` answers `None` both when no +level partitions the machine and when two partition it incomparably -- and a +machine of the second kind has a complete parse, agrees with every counter, and +still has no outermost partitioning cache. Both emitted `null`, on a row the +verdict had already certified, so a fleet query counting nulls as "machines no +cache level partitions" -- the natural reading, and the one this crate's own +no-L3 story invites -- folded in machines where a level DOES partition. Opposite +conclusions for anything sizing itself by cache boundary. + +The prose report had refused to conflate the two from the start, on the grounds +that "naming only the first turns a reported ambiguity into a false claim about +the hardware". The NDJSON simply had no field to say it in. It now does: +`outermost_partitioning_cache` is always a string, so a consumer filters on +`== "none"` rather than on the absence of a number, and +`Observation::partitioning_cache` returns an enum so a renderer cannot emit the +absent case without having decided which absent case it is. + +**The value set is `PartitioningCache`'s variants, and is not reproduced here.** +This paragraph did list them, and went stale one round later when +`no_levels_reported` was added -- the same collapse this field exists to prevent, +moved into the docs, and with a worse failure mode than a merely-missing branch: +a consumer that maps "not one of the documented values" onto the absent-level +default folds machines whose cache survey was EMPTY back into "machines no cache +level partitions", which is a claim about hardware read off a survey that found +no cache structure. Read the enum, which the renderer matches exhaustively, so +the two cannot diverge. + +`not_unique` is deliberately not called "incomparable": +the crate reaches `None` both for two maximal candidates that are not the same +partition and for a candidate filter that left nothing, and this probe cannot +tell those apart. + +The renderer's prose conclusions -- every claim it makes about the hardware, not +only the cache ones it was written for -- are gated on a **related but +deliberately narrower** condition, `CrossCheck::parse_in_doubt`: `disagreements` or +`parse_incomplete` non-empty, but *not* `not_compared`. The verdict answers +"may this run claim agreement", where a counter that could not be read matters; +the caveats answer "may these counts be read as hardware facts", where it does +not -- every `not_compared` entry is a reading this probe could not make or +could not trust, which says nothing about the parse. (Stated over what the list +means rather than what fills it: this once enumerated "the three Win32 counters +failing to read", and two later rounds falsified it by adding the bracket +outcomes.) Caveating there would assert a doubt the +run does not have, which is the same defect as asserting a certainty it does +not have. + +The two conditions are close enough that stating them as one was tempting and +was twice wrong in the other direction. The renderer first re-derived the +condition as "anomalies non-empty", so a host with `Coherence::Disagreed` and no +anomalies claimed "this machine reports no L3 at all"; corrected to +`parse_incomplete` alone, it still missed `disagreements`, so a host whose group +count Windows contradicts printed that same hardware claim directly above +"=> DISAGREE" -- in the one case where the evidence that the parse does not +describe this machine was already in hand. Both times the accompanying comment +asserted the condition was complete. Hence a single named predicate that argues +its own membership, rather than a condition restated at the point of use. + +## An incomplete parse fails CI, and that is the point + + + +`the_shipping_parse_agrees_with_the_raw_win32_counters` asserts +`Verdict::Agree`, and by the rule above that requires **all three** lists empty. +So it goes red on anything that fills any of them -- not only a parse the crate +reported as short or disputed, but also a counter this probe simply could not +read (`not_compared`), which is a gap in the measurement and says nothing about +the parse at all. Every non-`Agree` cause is a red build; there is no subset +that is tolerated. + +That matters because the parse-side causes are legal `Ok` results -- `discover` +returns the topology and says how the run went -- so this test can go red on a +host that is merely misbehaving, or on a run where a Win32 call failed, rather +than on a defect in this repository. CI runs the probe on `windows-latest`, a +virtualized fleet, which is where a defective hypervisor would show up. + +That is deliberate. The probe exists to survey real machines, and a host whose +enumeration is losing records is exactly the finding worth interrupting a build +for; a test that passed quietly on it would be the "report asserting something +the run did not establish" failure this whole probe is built to prevent, moved +up one level into the test suite. The cost is accepted: an occasional red build +that turns out to be the runner rather than the code. + +So **if this test goes red, read the verdict before touching the assertion.** +An `Incomplete` verdict names what was not established, and the answer is to +investigate that host -- not to relax the assertion, which would discard the +only signal that would ever have surfaced it. + +For that reading to be possible, the workflow step that runs the probe carries +`if: '!cancelled()'`. It sits after this test in the same job, so without it +GitHub Actions skips the report on exactly the host the report is for, and the +only surviving evidence is the `CrossCheck` in the assertion message -- not the +domain counts, the enumeration anomalies, or the machine-readable row. The +probe's own documentation says it prints on every build; that is what makes the +claim true rather than nearly true. + +What it prints is a **second measurement**, not a rendering of the one that +failed: the test and the binary each call `measure()`. That recovers a condition +the host holds persistently -- a fleet machine whose enumeration is genuinely +losing records, which is the case worth interrupting a build for -- and does not +recover one that was transient. Rendering the failing observation itself would +mean the assertion and the report were one step, which is a different design +than the binary-plus-asserted split this crate is built around. + +No work is scheduled by this decision; it records why the strict form is +correct so a future contributor does not quietly loosen it. Revisiting it means +splitting the verdicts -- `Disagree` failing while `Incomplete` reports loudly +and passes -- which is a change to what CI is for, not a bug fix. + +## The host banner is bracketed too, because it is a topology and not a name + + + +Every probe here opens with `fingerprint::banner_line()`, and in this one that +line is a *second* topology discovery: the fingerprint renders architecture, +processor and core counts, cache domain sizes and NUMA nodes, none of which +`measure`'s bracket encloses. It was read once and defended as "attribution", +on the grounds that no conclusion in the report is drawn from it. + +That defence was weaker than it sounded. A machine that changed across the run +would print one shape in the banner and a different one in the body, and a +reader mining accumulated CI output has no way to tell which described the +measurement -- the banner states the same quantities the body cross-checks, so +the two simply contradict each other with nothing saying so. Calling the header +"attribution" does not stop a reader reading a core count off it. + +So it is bracketed like everything else: read before and after, and reduced by +`topology_report::attribution`. Equal readings render exactly as before, which +keeps every fingerprint string already recorded elsewhere comparable with this +probe's. Readings that differ print both, because which of the two is stale is +precisely what cannot be determined here. + +**It reports that the readings DIFFER, and does not name a cause.** Saying "the +host changed" was the first wording and was itself an over-claim of the kind +this decision exists to remove: `Fingerprint::discover` returns `Ok` on a parse +that dropped a record or whose two sources disagreed, so a fingerprint can +differ from the one before it because the enumeration was flaky rather than +because any hardware moved. `measure`'s own bracket may say "the machine +changed" because a counter is a simple reading with no such failure mode; a +fingerprint is a whole parse, and the same sentence is not available to it. + +The two readings are `Fingerprint::discover` results rather than rendered +lines, and that is load-bearing. `banner_line` renders success and failure into +one string, so comparing two of those cannot tell a host that moved from a +discovery that failed -- and two failures whose `io::Error` text differs compare +unequal while establishing nothing at all. Rendering still goes through +`banner_line_for`, added to `windows-placement-probe` for this, so the format +has one owner and this probe's banner stays comparable with every other +probe's. + +This is a wider window than `measure`'s own bracket rather than a duplicate of +it: it closes over the whole run including both banner reads, where `measure` +closes only over the counters. Neither subsumes the other, and no work is +scheduled by this decision. diff --git a/crates/windows-platform-probes/src/bin/topology.rs b/crates/windows-platform-probes/src/bin/topology.rs new file mode 100644 index 000000000..52e44dd6a --- /dev/null +++ b/crates/windows-platform-probes/src/bin/topology.rs @@ -0,0 +1,50 @@ +// Copyright (c) Mike Grier. + +//! Prints the machine's processor topology, and how many execution domains each +//! candidate partitioning policy would produce on it. +//! +//! **An experiment, not a component.** These probes measure platform behaviour +//! and are not for production use. Do not call them from production code, and +//! do not lift a technique out of here. See this crate's DESIGN-NOTES.md. +//! +//! Running this on every CI build is deliberate: hosted runners are a +//! heterogeneous fleet, so the accumulated output is a slow survey of what real +//! machines look like. The line tagged `x-probe-topology` is emitted as a single +//! JSON object so those results can be mined out of build logs mechanically +//! rather than read by eye. + +use windows_placement_probe::fingerprint::Fingerprint; +use windows_platform_probes::report::{Stdout, emit}; +use windows_platform_probes::topology::measure; +use windows_platform_probes::topology_report::{attribution, report, report_unmeasured}; + +fn main() { + // The only place that names the real stream, and the only place that reads + // the host. The text is composed in the library so every branch of it can + // be driven from a test -- see `topology_report`. + // + // The banner is read FIRST and passed in. It runs a topology discovery of + // its own, so leaving it to the renderer made the library half depend on a + // host it claimed not to need. + // + // It is read AGAIN afterwards, and the pair bracketed, for the same reason + // `measure` brackets its counters: the fingerprint is a topology rendering, + // not a name, so a machine that changed across the run would otherwise print + // one shape above a body describing another. `attribution` decides what that + // pair means; both readings sit outside `measure`'s own bracket, which is + // what makes them a wider window than it and worth closing separately. + // + // `Fingerprint::discover` rather than `banner_line`, so a failed read stays + // distinguishable from a host that moved. Rendering goes through + // `banner_line_for`, which keeps this probe's banner the same shape as every + // other probe's. + let before = Fingerprint::discover(); + let measured = measure(); + let after = Fingerprint::discover(); + let banner = attribution(&before, &after); + let text = match measured { + Ok(observation) => report(&banner, &observation), + Err(error) => report_unmeasured(&banner, &error), + }; + emit(&mut Stdout, &text); +} diff --git a/crates/windows-platform-probes/src/lib.rs b/crates/windows-platform-probes/src/lib.rs index c16c9ebe0..dbc16f560 100644 --- a/crates/windows-platform-probes/src/lib.rs +++ b/crates/windows-platform-probes/src/lib.rs @@ -105,6 +105,7 @@ //! | [`cancel_io::cancel_against_idle_thread`] | binary only | `CancelSynchronousIo` is point-in-time against an idle thread | //! | [`cancel_io::cancel_against_busy_thread`] | binary only | it can block indefinitely against a thread re-entering synchronous I/O | //! | [`long_path::measure`] | binary only | whether the `longPathAware` manifest opt-in lifts `MAX_PATH` for a *relative* path -- binary only because the answer is the difference between two differently-manifested executables, which no single in-process test can observe | +//! | [`topology::measure`] | asserted | that `windows-topology-sys`' parse of `GetLogicalProcessorInformationEx` agrees with `GetActiveProcessorCount`, `GetActiveProcessorGroupCount` and `GetNumaHighestNodeNumber` read independently -- asserted because the invariants hold on any machine even though every value is host-specific, and the binary prints the shape so CI doubles as a fleet survey | #![cfg(windows)] #![forbid(unsafe_op_in_unsafe_fn)] @@ -120,6 +121,8 @@ pub mod long_path; pub mod long_path_report; pub mod pool_growth; pub mod report; +pub mod topology; +pub mod topology_report; pub mod worker_context; #[cfg(test)] diff --git a/crates/windows-platform-probes/src/tests.rs b/crates/windows-platform-probes/src/tests.rs index b716b9e8b..659216f27 100644 --- a/crates/windows-platform-probes/src/tests.rs +++ b/crates/windows-platform-probes/src/tests.rs @@ -693,3 +693,3463 @@ fn the_impersonation_guard_reverts_even_while_unwinding() { "the thread must not have been left impersonating after the unwind" ); } + +// --- topology ------------------------------------------------------------- +// +// Every interesting number the topology probe prints is host-specific, so +// nothing here asserts a *value*. What is asserted is internal consistency, +// which must hold on any machine and therefore catches a parsing regression in +// `windows-topology-sys` on whatever hardware CI happens to run on -- which is +// the whole reason the probe reads the shipping crate rather than a second +// parse written here. + +#[test] +fn the_machine_reports_at_least_one_processor_one_group_and_one_core() { + let observation = crate::topology::measure().expect("topology discovery"); + + assert!( + observation.online_processors >= 1, + "a running process implies at least one online processor" + ); + assert!( + observation.groups >= 1, + "every machine has at least one processor group" + ); + // Phrased as claims about the REPORT, not the machine. The machine + // certainly has both; a zero here means the enumeration did not describe + // them, and saying "every machine has at least one package" would send a + // reader to doubt the hardware. + assert!( + !observation.cores.is_empty(), + "no cores were reported, so the enumeration did not describe a machine that has them" + ); + assert!( + observation.packages >= 1, + "no packages were reported, so the enumeration did not describe a machine that has one" + ); +} + +#[test] +fn the_shipping_parse_agrees_with_the_raw_win32_counters() { + let observation = crate::topology::measure().expect("topology discovery"); + + // This is the cross-check that makes the probe worth running everywhere: a + // disagreement means windows-topology-sys parsed + // GetLogicalProcessorInformationEx differently from what the simple + // counters report on this host. + // The VERDICT, not just an empty disagreement list. Asserting only that + // nothing disagreed would pass on a run where a counter could not be read + // and so was never compared -- which is the state this probe must never + // report as agreement. + let check = observation.cross_check(); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Agree, + // The whole `CrossCheck`, not two of its three lists. Naming the lists + // individually meant `parse_incomplete` -- added later as a third way to + // miss `Agree` -- was omitted, so the one host that would newly fail + // here got "disagreements=[] not_compared=[]": an assertion saying + // agreement was not established, beside evidence that nothing + // disagreed and nothing was skipped. The reason was in `check` all + // along and was discarded at the point of failure. + "cross-check did not establish agreement: {check:?}" + ); +} + +/// An observation with nothing to complain about, for a test to perturb one +/// field of. Every host reachable here has a single NUMA node, so the sparse +/// case below cannot be measured and has to be constructed. +/// +/// It carries one whole-machine cache level rather than an empty list, and the +/// difference is load-bearing: an empty `caches` is now itself a finding (no +/// cache level was reported at all, so nothing establishes what divides this +/// machine), and a fixture that tripped it would make every test perturbing one +/// other field assert against two complaints instead of one. That this fixture +/// was previously empty and uncomplained-about is exactly the defect -- nine +/// tests turned red the moment the finding was added, which is the fixture +/// telling the truth about what it had been relying on. +fn agreeing_observation() -> crate::topology::Observation { + crate::topology::Observation { + online_processors: 4, + groups: 1, + numa_domains: 1, + numa_domains_without_processors: 0, + numa_domains_only_in_cpu_sets: 0, + numa_domains_unreported: 0, + numa_domains_with_conflicting_labels: 0, + topology_was_measured: true, + cores_only_in_cpu_sets: 0, + cores_without_processors: 0, + packages_without_processors: 0, + overlapping_walk_relations: 0, + described_relations: 0, + unreported_relations: 0, + processor_attribute_conflicts: 0, + highest_numa_node: Some(0), + packages: 1, + // One core, for the same reason it carries one cache level: reporting + // none is itself a finding now, and a fixture that tripped it would + // make every test perturbing one other field assert against two + // complaints instead of one. + // + // SMT is `true` because the core carries four processors, and the owning + // crate defines the flag as exactly that condition. It read `false` + // while `cross_check` did not look at the pair, so this fixture -- the + // one every perturbation test starts from -- described a core that + // cannot exist. + cores: vec![crate::topology::CoreShape { + simultaneous_multithreading: true, + efficiency_class: 0, + processors: 4, + }], + caches: vec![crate::topology::CacheLevel { + level: 1, + processors_per_domain: vec![4], + }], + partitioning_cache_level: None, + enumeration_anomalies: Vec::new(), + coherence: windows_topology_sys::Coherence::Agreed, + raw_active_processors: 4, + raw_group_count: 1, + raw_highest_numa_node: Some(0), + bracket: crate::topology::BracketOutcome::HeldStill, + } +} + +#[test] +fn sparse_numa_node_numbers_are_not_reported_as_a_parsing_regression() { + // `GetNumaHighestNodeNumber` reports the highest node *number*, which + // Windows does not promise equals the node count. Nodes 0 and 2 are a valid + // sparse topology: two domains, highest number two. Comparing the count + // against `highest + 1` called that a disagreement, so the probe's asserted + // test would fail on hardware that is reporting itself correctly. + let mut observation = agreeing_observation(); + observation.numa_domains = 2; + observation.highest_numa_node = Some(2); + observation.raw_highest_numa_node = Some(2); + + assert_eq!( + observation.cross_check().verdict(), + crate::topology::Verdict::Agree, + "a sparse node numbering is a valid machine, not a parse error" + ); +} + +#[test] +fn a_numa_node_the_topology_crate_never_saw_is_still_reported() { + // The other direction, so the sparse tolerance cannot pass by never + // complaining: Windows names a node the crate's parse did not produce, and + // that is the disagreement this cross-check exists to surface. + let mut observation = agreeing_observation(); + observation.raw_highest_numa_node = Some(3); + + let check = observation.cross_check(); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Disagree, + "{check:?}" + ); + assert_eq!(check.disagreements.len(), 1, "{check:?}"); + assert!(check.disagreements[0].contains("NUMA nodes"), "{check:?}"); + assert!(check.not_compared.is_empty(), "{check:?}"); +} + +#[test] +fn a_topology_reporting_no_numa_node_at_all_disagrees_with_a_raw_one() { + // The `None` arm, which the count form could not express: Windows names a + // node and the crate's parse produced no memory domain whatsoever. + let mut observation = agreeing_observation(); + observation.numa_domains = 0; + observation.highest_numa_node = None; + + let check = observation.cross_check(); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Disagree, + "{check:?}" + ); + assert_eq!(check.disagreements.len(), 1, "{check:?}"); + assert!(check.disagreements[0].contains("none"), "{check:?}"); +} + +// A counter that could not be read must never read as agreement. Each of these +// perturbs one counter into its documented failure report and asserts the +// verdict is `Incomplete` -- not `Agree`, which would claim something the run +// did not establish, and not `Disagree`, which would blame the shipping crate +// for a measurement this probe failed to take. + +#[test] +fn a_failed_numa_read_is_incomplete_rather_than_agreement() { + // The reachable one: the renderer already has a `failed` branch for this + // counter, and the verdict beneath it used to say "agree ... parsed this + // machine consistently" on the very next line. + let mut observation = agreeing_observation(); + observation.raw_highest_numa_node = None; + + let check = observation.cross_check(); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{check:?}" + ); + assert!(check.disagreements.is_empty(), "{check:?}"); + assert_eq!(check.not_compared.len(), 1, "{check:?}"); + assert!( + check.not_compared[0].contains("GetNumaHighestNodeNumber"), + "{check:?}" + ); +} + +#[test] +fn a_failed_processor_count_is_incomplete_rather_than_a_parse_disagreement() { + // `GetActiveProcessorCount` reports failure by returning zero, and no + // machine has zero active processors. Compared as a count, that zero + // produced "topology crate says 4, GetActiveProcessorCount says 0" -- an + // accusation against the crate's parse for a read this probe fumbled. + let mut observation = agreeing_observation(); + observation.raw_active_processors = 0; + + let check = observation.cross_check(); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{check:?}" + ); + assert!(check.disagreements.is_empty(), "{check:?}"); + assert!( + check.not_compared[0].contains("GetActiveProcessorCount"), + "{check:?}" + ); +} + +#[test] +fn a_failed_group_count_is_incomplete_rather_than_a_parse_disagreement() { + let mut observation = agreeing_observation(); + observation.raw_group_count = 0; + + let check = observation.cross_check(); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{check:?}" + ); + assert!(check.disagreements.is_empty(), "{check:?}"); + assert!( + check.not_compared[0].contains("GetActiveProcessorGroupCount"), + "{check:?}" + ); +} + +#[test] +fn a_dropped_enumeration_record_blocks_agreement_even_when_every_counter_matches() { + // The counters are three scalars, and none of them is sensitive to a + // `PROCESSOR_RELATIONSHIP` record that failed to decode: `discover` records + // the anomaly, drops the record, and returns `Ok`, so the package and core + // counts are short by an amount nothing here can observe. All three + // counters therefore still agree, and the probe printed "agree ... parsed + // this machine consistently" for a parse the crate had already reported as + // incomplete. + // + // The fixture is otherwise the agreeing one on purpose: the anomaly is the + // only difference, so this cannot pass by way of some other complaint. + let mut observation = agreeing_observation(); + observation.enumeration_anomalies = vec![windows_topology_sys::EnumerationAnomaly { + source: windows_topology_sys::Source::RelationshipWalk, + offset: 64, + kind: windows_topology_sys::AnomalyKind::Undersized { + declared: 8, + minimum: 48, + }, + }]; + + let check = observation.cross_check(); + assert!( + check.disagreements.is_empty(), + "an undecodable record is not the crate parsing something wrongly: {check:?}" + ); + assert!( + check.not_compared.is_empty(), + "every counter was read, so nothing belongs in not_compared: {check:?}" + ); + assert_eq!(check.parse_incomplete.len(), 1, "{check:?}"); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "agreeing counters must not certify a parse the crate said was short: {check:?}" + ); +} + +#[test] +fn disagreeing_source_enumerations_block_agreement_even_when_every_counter_matches() { + // `Coherence::Disagreed` is the crate's *conclusion*, after exhausting its + // retries, that its two Win32 sources describe different machines -- and it + // returns `Ok`. The processors in `cpu_sets_only` are deliberately absent + // from the parsed processor list, so no count taken from that list can + // reveal them and `GetActiveProcessorCount` need not notice: it counts + // ACTIVE processors, while the parsed list carries inactive slots too, so + // the two totals can coincide while the membership differs. + let mut observation = agreeing_observation(); + observation.coherence = windows_topology_sys::Coherence::Disagreed { + walk_only: Vec::new(), + cpu_sets_only: vec![windows_topology_sys::ProcessorId { + group: 0, + number: 3, + }], + attempts: 4, + }; + + let check = observation.cross_check(); + assert!(check.disagreements.is_empty(), "{check:?}"); + assert!(check.not_compared.is_empty(), "{check:?}"); + assert_eq!(check.parse_incomplete.len(), 1, "{check:?}"); + assert!( + check.parse_incomplete[0].contains("never agreed"), + "{check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{check:?}" + ); +} + +#[test] +fn a_numa_domain_only_cpu_sets_reported_blocks_agreement_rather_than_accusing_the_parse() { + // `fold_memberships` pushes a memory domain that only CPU Sets described -- + // "the walk not describing it is a fact about the walk, not evidence the + // relation is not there". Neither `coherence` nor any counter reaches it: + // coherence compares PROCESSOR SETS, and two sources can name the same + // processors while grouping them into different nodes. + let mut observation = agreeing_observation(); + observation.numa_domains = 2; + observation.numa_domains_only_in_cpu_sets = 1; + + let check = observation.cross_check(); + assert!( + check.disagreements.is_empty(), + "the sources grouping nodes differently is not the crate contradicting a counter: {check:?}" + ); + assert_eq!(check.parse_incomplete.len(), 1, "{check:?}"); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{check:?}" + ); +} + +#[test] +fn observe_takes_the_node_number_from_whichever_source_reported_it() { + // Drives the extraction itself rather than a hand-set field, which is why + // `observe` was split out of `measure`: the defect lived in this loop and + // sat behind `MachineMemoryTopology::discover`, so no test could reach it. + // + // Two memory domains. Node 0 is what the relationship walk saw; node 1 is a + // relation only CPU Sets reported, which `fold_memberships` pushes as its + // own domain. Reading the walk's label alone made node 1 raise + // `numa_domains` to 2 while leaving the highest at 0 -- so a machine whose + // GetNumaHighestNodeNumber says 1 was accused of a parsing regression for a + // node the crate had reported correctly. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Source, + }; + + let memory = || DomainKind::Memory { + memory_bytes: windows_topology_sys::Observed::NotObserved, + }; + let processor = |number| Processor { + id: ProcessorId { group: 0, number }, + online: true, + capacity: 0, + }; + // Two online processors in one group, so the counters passed below match + // and the ONLY thing this fixture can complain about is the node number. + let topology = MachineMemoryTopology { + processors: vec![processor(0), processor(1)], + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8), (0u16, 1u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: memory(), + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: memory(), + processors: [(0u16, 1u8)].into_iter().collect(), + observations: vec![Observation::new(Source::CpuSets, 1)], + }, + ], + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 2, + 1, + Some(1), + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!(observation.numa_domains, 2); + assert_eq!( + observation.highest_numa_node, + Some(1), + "node 1 was reported by CPU Sets, and node numbers are machine-wide from \ + either source, so the highest the crate reported is 1" + ); + assert_eq!( + observation.numa_domains_only_in_cpu_sets, 1, + "and the domain the walk never described is counted, because nothing else sees it" + ); + + let check = observation.cross_check(); + assert!( + check.disagreements.is_empty(), + "the counter says 1 and so does the crate: {check:?}" + ); +} + +#[test] +fn no_partitioning_level_is_told_apart_from_no_unique_one() { + use crate::topology::{CacheLevel, PartitioningCache}; + + // The distinction the NDJSON used to collapse. Both render a level of + // `null`, and both reach `Verdict::Agree` because neither touches anything + // `cross_check` consults -- so a fleet query counting nulls as "machines no + // cache level partitions" silently folded in machines where a level DOES + // partition, which is the opposite conclusion for anything sizing itself by + // cache boundary. + let mut nothing_partitions = agreeing_observation(); + nothing_partitions.caches = vec![CacheLevel { + level: 3, + processors_per_domain: vec![4], + }]; + nothing_partitions.partitioning_cache_level = None; + assert_eq!( + nothing_partitions.partitioning_cache(), + PartitioningCache::NoLevelPartitions, + "one level covering the whole machine cannot have partitioned it" + ); + + // Same `None` from the crate, but a level plainly splits the machine. The + // crate reaches that for two incomparable maximal candidates AND for a + // candidate filter that left nothing, so this is named as "not the case + // above" and no more. + let mut something_partitions = agreeing_observation(); + something_partitions.caches = vec![CacheLevel { + level: 3, + processors_per_domain: vec![2, 2], + }]; + something_partitions.partitioning_cache_level = None; + assert_eq!( + something_partitions.partitioning_cache(), + PartitioningCache::NoUniqueOutermost, + "a level with two domains partitions it, whatever the crate could name" + ); + + // Both still agree, which is the point: the verdict was never going to + // carry this distinction, so the JSON had to. + assert_eq!( + nothing_partitions.cross_check().verdict(), + crate::topology::Verdict::Agree + ); + assert_eq!( + something_partitions.cross_check().verdict(), + crate::topology::Verdict::Agree + ); +} + +#[test] +fn a_cache_level_that_decoded_to_no_partitions_blocks_the_no_partitioning_conclusion() { + use crate::topology::{CacheLevel, PartitioningCache}; + + // `NoLevelPartitions` is read as a fact about the hardware -- no cache + // boundary divides the work -- and a level with ZERO partitions satisfies + // "not more than one" exactly like a level covering the whole machine. It + // is not the same finding: that level's records decoded to nothing. + // + // Nothing else here is sensitive to it. A well-formed GROUP_AFFINITY with a + // zero mask decodes completely and raises no anomaly, so without this the + // report printed "L3 0 domain(s)" a few lines above "no cache level + // partitions this machine", with the verdict certifying the pair as agree. + let mut observation = agreeing_observation(); + observation.caches = vec![CacheLevel { + level: 3, + processors_per_domain: Vec::new(), + }]; + observation.partitioning_cache_level = None; + + assert_eq!( + observation.partitioning_cache(), + PartitioningCache::NoLevelPartitions, + "the classification is unchanged; what changes is that it is caveated" + ); + + let check = observation.cross_check(); + assert!(check.disagreements.is_empty(), "{check:?}"); + assert_eq!(check.parse_incomplete.len(), 1, "{check:?}"); + assert!( + check.parse_in_doubt(), + "so the renderer caveats it: {check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "and a mining pass filtering on agree never sees the row: {check:?}" + ); +} + +#[test] +fn no_cache_levels_at_all_is_its_own_answer_and_is_caveated() { + use crate::topology::PartitioningCache; + + // One step past the zero-partition level. `any()` over an EMPTY list is + // vacuously false, so an empty survey reached `NoLevelPartitions` and + // printed "no cache boundary divides the work" -- a conclusion about cache + // structure from a survey that found no cache structure. Nothing else here + // notices: no record failed to decode, because there was no record. + // + // Reachable on exactly the fleet this probe surveys -- a hypervisor + // reporting group, core and NUMA relationships but no cache ones. + let mut observation = agreeing_observation(); + observation.caches = Vec::new(); + observation.partitioning_cache_level = None; + + assert_eq!( + observation.partitioning_cache(), + PartitioningCache::NoLevelsReported, + "an empty survey is not the same finding as a survey that found no partitions" + ); + + let check = observation.cross_check(); + assert!(check.disagreements.is_empty(), "{check:?}"); + assert_eq!(check.parse_incomplete.len(), 1, "{check:?}"); + assert!(check.parse_in_doubt(), "{check:?}"); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "so a mining pass filtering on agree never sees the row: {check:?}" + ); +} + +#[test] +fn a_level_that_covers_the_whole_machine_is_not_caveated() { + use crate::topology::CacheLevel; + + // The control. A genuine single-domain level is the ordinary "no cache + // boundary divides the work" machine and must stay `Agree`, or the caveat + // above would fire on every uniform host and mean nothing. + let mut observation = agreeing_observation(); + observation.caches = vec![CacheLevel { + level: 3, + processors_per_domain: vec![4], + }]; + observation.partitioning_cache_level = None; + + let check = observation.cross_check(); + assert!(!check.parse_in_doubt(), "{check:?}"); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Agree, + "{check:?}" + ); +} + +#[test] +fn partitioning_cache_returns_the_summary_for_the_named_level_among_several() { + use crate::topology::{CacheLevel, PartitioningCache}; + + // Several levels, so the lookup has something to get WRONG. With one entry + // a mutated `==` finds nothing and yields `SummaryMissing`, which no test + // distinguished from the right answer; with three it returns a different + // level's summary, which this catches. + let mut observation = agreeing_observation(); + observation.caches = vec![ + CacheLevel { + level: 1, + processors_per_domain: vec![1, 1, 1, 1], + }, + CacheLevel { + level: 2, + processors_per_domain: vec![2, 2], + }, + CacheLevel { + level: 3, + processors_per_domain: vec![4], + }, + ]; + observation.partitioning_cache_level = Some(2); + + match observation.partitioning_cache() { + PartitioningCache::Level(cache) => { + assert_eq!( + cache.level, 2, + "the summary must be the one the crate named" + ); + assert_eq!(cache.domains(), 2); + } + other => panic!("expected the named level's summary, got {other:?}"), + } +} + +#[test] +fn observe_counts_a_numa_domain_with_no_processors() { + // Drives the extraction. The consequence was pinned by a hand-set field, so + // a mutation sweep found the increment itself replaceable. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Observed, Processor, ProcessorId, + Source, + }; + + let memory = || DomainKind::Memory { + memory_bytes: Observed::NotObserved, + }; + let topology = MachineMemoryTopology { + processors: vec![Processor { + id: ProcessorId { + group: 0, + number: 0, + }, + online: true, + capacity: 0, + }], + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: memory(), + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + // The CXL-expander shape: memory with no processors at all. + Domain { + kind: memory(), + processors: windows_topology_sys::ProcessorSet::default(), + observations: vec![Observation::new(Source::RelationshipWalk, 1)], + }, + ], + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 1, + 1, + Some(1), + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!(observation.numa_domains, 2); + assert_eq!( + observation.numa_domains_without_processors, 1, + "and it is subtracted from the by-numa-domain policy, which is why it is counted" + ); + assert_eq!( + observation + .domain_counts() + .into_iter() + .find(|(name, _)| *name == "by-numa-domain-with-processors") + .map(|(_, count)| count), + Some(1), + ); +} + +#[test] +fn a_topology_with_no_processors_at_all_is_not_accused_of_hiding_packages() { + // The boundary on the `online_processors > 0` guards. A synthetic topology + // that reported nothing has no processors either, so "no packages were + // reported, though the machine has one" would assert a machine this + // observation never saw. + let mut observation = agreeing_observation(); + observation.online_processors = 0; + observation.raw_active_processors = 0; + observation.packages = 0; + observation.cores = Vec::new(); + + let check = observation.cross_check(); + assert!( + !check.parse_incomplete.iter().any( + |c| c.contains("no packages were reported") || c.contains("no cores were reported") + ), + "with no processors reported, absent packages and cores are not a separate finding: \ + {check:?}" + ); +} + +#[test] +fn a_named_level_with_no_summary_is_a_probe_defect_not_a_fact_about_the_machine() { + use crate::topology::PartitioningCache; + + // Asserted unreachable by + // `the_outermost_partitioning_cache_is_the_deepest_level_that_splits_the_machine`. + // Pinned as its own variant anyway, because folding it into the `None` arm + // is what would print "no cache level partitions this machine" -- a claim + // about the hardware -- when the truth is that this probe lost the summary. + let mut observation = agreeing_observation(); + observation.caches = Vec::new(); + observation.partitioning_cache_level = Some(3); + + assert_eq!( + observation.partitioning_cache(), + PartitioningCache::SummaryMissing(3) + ); +} + +#[test] +fn observe_reports_a_numa_domain_whose_sources_number_it_differently() { + // Node numbers are machine-wide, so one domain cannot honestly be both node + // 0 and node 1 -- the two sources contradict each other. The crate keeps + // both labels on purpose ("the labels differ and both are kept, which is + // the whole of D-15"); taking the maximum to compare against the counter + // resolves the disagreement silently, which is what this stops. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Observed, Processor, ProcessorId, + Source, + }; + + let topology = MachineMemoryTopology { + processors: vec![Processor { + id: ProcessorId { + group: 0, + number: 0, + }, + online: true, + capacity: 0, + }], + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: DomainKind::Memory { + memory_bytes: Observed::NotObserved, + }, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![ + Observation::new(Source::RelationshipWalk, 0), + Observation::new(Source::CpuSets, 1), + ], + }, + ], + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 1, + 1, + Some(1), + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!(observation.numa_domains_with_conflicting_labels, 1); + assert_eq!( + observation.highest_numa_node, + Some(1), + "the maximum is still taken, so the counter comparison can proceed" + ); + + let check = observation.cross_check(); + assert!( + check.disagreements.is_empty(), + "the sources contradicting each other is not the crate contradicting a counter: {check:?}" + ); + // The entry, not the count. This fixture carries no cache domains either, + // so the empty-survey entry fires alongside; asserting a total would couple + // this test to causes it is not about. + assert!( + check + .parse_incomplete + .iter() + .any(|c| c.contains("more than one distinct node number")), + "{check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "agreeing counters must not certify a node number the sources disputed: {check:?}" + ); +} + +#[test] +fn a_machine_that_changed_mid_run_is_not_compared_rather_than_blamed() { + // The parse and the counters are separate reads. A processor hot-add + // between them leaves both correct for different instants, and comparing + // them would file the difference as the crate parsing wrongly. + let mut observation = agreeing_observation(); + observation.bracket = crate::topology::BracketOutcome::Changed; + observation.groups = 1; + observation.raw_group_count = 9; + + let check = observation.cross_check(); + assert!( + check.disagreements.is_empty(), + "a machine that moved under the run is this measurement's problem, not the parse's: \ + {check:?}" + ); + assert_eq!(check.not_compared.len(), 1, "{check:?}"); + assert!( + !check.parse_in_doubt(), + "the topology is still a valid snapshot of the machine as it was, so the cache \ + conclusions drawn from it stand: {check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{check:?}" + ); +} + +#[test] +fn a_machine_that_changed_still_reports_what_the_parse_itself_lost() { + // The combined case, which the machine-changed test above cannot reach + // because it starts from a clean fixture. A host can hot-add a processor + // AND have dropped a record; the two are unrelated. + // + // Handling the timing skew with an early return suppressed every + // parse-side check, so this reported `parse_incomplete: 0`, + // `parse_in_doubt` was false, and the renderer printed hardware + // conclusions uncaveated while never mentioning the dropped record + // anywhere in the report. + let mut observation = agreeing_observation(); + observation.bracket = crate::topology::BracketOutcome::Changed; + observation.enumeration_anomalies = vec![windows_topology_sys::EnumerationAnomaly { + source: windows_topology_sys::Source::RelationshipWalk, + offset: 64, + kind: windows_topology_sys::AnomalyKind::Undersized { + declared: 8, + minimum: 48, + }, + }]; + + let check = observation.cross_check(); + assert!( + !check.not_compared.is_empty(), + "the timing skew is still recorded: {check:?}" + ); + assert!( + check + .parse_incomplete + .iter() + .any(|c| c.contains("enumeration anomal")), + "and the dropped record is NOT suppressed by it: {check:?}" + ); + assert!( + check.parse_in_doubt(), + "so the renderer still caveats its hardware conclusions: {check:?}" + ); +} + +#[test] +fn a_core_or_attribute_the_sources_disagree_about_blocks_agreement() { + // The core-level and attribute-level twins of the NUMA provenance check. + // `coherence` reaches neither: it compares which PROCESSORS exist, so two + // sources can agree on every processor and still group them into different + // cores, or give the same processor different efficiency classes. + for (label, mutate) in [ + ( + "core(s) were reported only by CPU Sets", + Box::new(|o: &mut crate::topology::Observation| o.cores_only_in_cpu_sets = 1) + as Box, + ), + ( + "attribute(s) carry more than one distinct value", + Box::new(|o: &mut crate::topology::Observation| o.processor_attribute_conflicts = 1), + ), + ] { + let mut observation = agreeing_observation(); + mutate(&mut observation); + + let check = observation.cross_check(); + assert!( + check.disagreements.is_empty(), + "{label}: the sources disagreeing with each other is not the crate contradicting a \ + counter: {check:?}" + ); + assert!( + check.parse_incomplete.iter().any(|c| c.contains(label)), + "{label}: {check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{label}: {check:?}" + ); + } +} + +#[test] +fn observe_counts_a_core_only_cpu_sets_reported() { + // Drives the extraction, so the provenance check cannot be dropped + // silently. The walk describes one core over both processors; CPU Sets + // describes a different core over one of them, which `fold_memberships` + // keeps as its own domain because the memberships differ. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Source, + }; + + let core = |smt: bool| DomainKind::Core { + simultaneous_multithreading: smt, + efficiency_class: 0, + }; + let topology = MachineMemoryTopology { + processors: (0..2) + .map(|number| Processor { + id: ProcessorId { group: 0, number }, + online: true, + capacity: 0, + }) + .collect(), + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8), (0u16, 1u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: core(true), + processors: [(0u16, 0u8), (0u16, 1u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: core(false), + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::CpuSets, 0)], + }, + ], + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 2, + 1, + Some(0), + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!(observation.cores.len(), 2, "both groupings are carried"); + assert_eq!( + observation.cores_only_in_cpu_sets, 1, + "and the one the walk never described is counted, so the inflated \ + by-core count cannot be read as an established core count" + ); +} + +#[test] +fn observe_carries_the_crates_attribute_conflicts() { + // Pins the WIRING, not just the consequence. The test above sets + // `processor_attribute_conflicts` by hand, so replacing this call with a + // constant zero left it green -- the guard was covered and the thing that + // feeds the guard was not. + // + // Two sources give processor 0 a different efficiency class. There is no + // membership to compare, which is why the crate keeps this apart from the + // relations, and why nothing else in this observation can see it. + use windows_topology_sys::{ + AttributeObservation, Domain, DomainKind, MachineMemoryTopology, Observation, + ProcessorAttribute, ProcessorId, Source, + }; + + let processor = ProcessorId { + group: 0, + number: 0, + }; + let topology = MachineMemoryTopology { + processors: vec![windows_topology_sys::Processor { + id: processor, + online: true, + capacity: 0, + }], + domains: vec![Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }], + processor_attributes: vec![ + AttributeObservation::new( + processor, + ProcessorAttribute::EfficiencyClass, + 0, + Source::RelationshipWalk, + ), + AttributeObservation::new( + processor, + ProcessorAttribute::EfficiencyClass, + 1, + Source::CpuSets, + ), + ], + ..MachineMemoryTopology::default() + }; + + assert_eq!( + topology.attribute_conflicts().len(), + 1, + "the fixture must actually produce a conflict, or this proves nothing" + ); + + let observation = crate::topology::observe( + &topology, + 1, + 1, + Some(0), + crate::topology::BracketOutcome::HeldStill, + ); + assert_eq!(observation.processor_attribute_conflicts, 1); + assert!( + observation + .cross_check() + .parse_incomplete + .iter() + .any(|c| c.contains("attribute(s) carry more than one distinct value")), + ); +} + +#[test] +fn a_topology_nobody_measured_cannot_be_certified_against_this_machine() { + // `observe` is public and takes any topology. `MachineMemoryTopology` + // defaults to `Provenance::Synthetic` and deserialization downgrades to it, + // precisely so a topology nobody measured cannot pass for one that was -- + // and without reading that, a hand-built or restored topology with matching + // counters reached `Agree`, certifying consistency with a host no + // enumeration ever read. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Provenance, + Source, + }; + + let build = |provenance| MachineMemoryTopology { + processors: vec![Processor { + id: ProcessorId { + group: 0, + number: 0, + }, + online: true, + capacity: 0, + }], + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: DomainKind::Package, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: DomainKind::Core { + simultaneous_multithreading: false, + efficiency_class: 0, + }, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + ], + provenance, + ..MachineMemoryTopology::default() + }; + + let synthetic = crate::topology::observe( + &build(Provenance::Synthetic), + 1, + 1, + None, + crate::topology::BracketOutcome::HeldStill, + ); + assert!(!synthetic.topology_was_measured); + assert!( + synthetic + .cross_check() + .parse_incomplete + .iter() + .any(|c| c.contains("not measured from a running machine")), + "{:?}", + synthetic.cross_check() + ); + + // The control: the same topology, measured, carries no such entry -- so the + // finding is about provenance and not about the fixture's shape. + let measured = crate::topology::observe( + &build(Provenance::Measured), + 1, + 1, + None, + crate::topology::BracketOutcome::HeldStill, + ); + assert!(measured.topology_was_measured); + assert!( + !measured + .cross_check() + .parse_incomplete + .iter() + .any(|c| c.contains("not measured from a running machine")), + "{:?}", + measured.cross_check() + ); +} + +#[test] +fn a_counter_that_failed_one_read_is_not_reported_as_the_machine_changing() { + // `before != after` treated the failure sentinels as values, so a counter + // that failed once and succeeded once read as "the machine changed while + // this ran" -- a claim about the hardware nothing established, which also + // skipped all three comparisons. + // + // THE shipping predicate, not a copy of it. This test reproduced the + // expression verbatim and asserted against its own closure, so a mutation + // sweep found all six operators in `measure` unkilled -- the test was named + // for a regression it could not detect. + let changed = |before, after| { + crate::topology::bracket_outcome(before, after) == crate::topology::BracketOutcome::Changed + }; + + assert!( + !changed((0, 1, Some(0)), (4, 1, Some(0))), + "a counter that failed the first read and succeeded the second says nothing about change" + ); + assert!( + !changed((4, 1, Some(0)), (0, 1, Some(0))), + "nor the reverse" + ); + assert!( + !changed((4, 1, None), (4, 1, Some(0))), + "nor the NUMA call, whose sentinel is None" + ); + // The group clause too. A mutation sweep found all three of its operators + // unkilled while the other two clauses were covered -- the test asserted + // the rule and exercised it on two of the three counters. + assert!( + !changed((4, 0, Some(0)), (4, 1, Some(0))), + "nor the group count, whose sentinel is also 0" + ); + + assert!( + changed((4, 1, Some(0)), (8, 1, Some(0))), + "two good reads that differ ARE evidence the machine changed" + ); + assert!( + changed((4, 1, Some(0)), (4, 2, Some(0))), + "on the group count as well" + ); + assert!( + changed((4, 1, Some(0)), (4, 1, Some(1))), + "and on the NUMA highest node" + ); +} + +#[test] +fn a_report_of_no_packages_or_no_cores_is_a_finding_not_a_machine() { + // The machine has both whatever the enumeration said. No record needs to + // have FAILED for this to happen -- Windows can simply not report the + // relationship, so no anomaly fires and nothing else here notices. Treated + // exactly as an empty cache survey is. + for (label, mutate) in [ + ( + "packages", + Box::new(|o: &mut crate::topology::Observation| o.packages = 0) + as Box, + ), + ( + "cores", + Box::new(|o: &mut crate::topology::Observation| o.cores = Vec::new()), + ), + ] { + let mut observation = agreeing_observation(); + mutate(&mut observation); + + let check = observation.cross_check(); + assert!( + check.disagreements.is_empty(), + "{label}: an absent relationship is not the crate contradicting a counter: {check:?}" + ); + assert!( + check.parse_incomplete.iter().any(|c| c.contains(label)), + "{label}: {check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{label}: {check:?}" + ); + } +} + +#[test] +fn a_numa_domain_no_source_reported_blocks_agreement_rather_than_accusing_the_parse() { + // A memory domain with an empty `observations` list is what + // windows-topology-sys documents as the honest state for "a relation nobody + // reported". It raises `numa_domains` while contributing no node number, so + // a maximum taken over the rest can under-report and the gap against + // GetNumaHighestNodeNumber would be filed against the crate -- the R6 + // misattribution reached through `observe`, which is public. + // + // It is counted separately from `numa_domains_only_in_cpu_sets` because + // that count's message names CPU Sets as the reporter, and here nobody was. + let mut observation = agreeing_observation(); + observation.numa_domains = 2; + observation.numa_domains_unreported = 1; + + let check = observation.cross_check(); + assert!(check.disagreements.is_empty(), "{check:?}"); + assert_eq!(check.parse_incomplete.len(), 1, "{check:?}"); + assert!( + !check.parse_incomplete[0].contains("CPU Sets"), + "nobody reported it, so the message must not name a reporter: {check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{check:?}" + ); +} + +#[test] +fn observe_counts_an_unreported_numa_domain_apart_from_a_cpu_sets_only_one() { + // Drives the extraction, so the two counters cannot silently merge: the + // predicate for "only CPU Sets" is also satisfied by a domain with no + // observations at all, and asserting only `!observed_by(walk)` described + // such a domain as "reported only by CPU Sets". + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Source, + }; + + let memory = || DomainKind::Memory { + memory_bytes: windows_topology_sys::Observed::NotObserved, + }; + let topology = MachineMemoryTopology { + processors: vec![Processor { + id: ProcessorId { + group: 0, + number: 0, + }, + online: true, + capacity: 0, + }], + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: memory(), + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::CpuSets, 1)], + }, + Domain { + kind: memory(), + processors: Default::default(), + observations: Vec::new(), + }, + ], + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 1, + 1, + Some(1), + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!(observation.numa_domains, 2); + assert_eq!( + observation.numa_domains_only_in_cpu_sets, 1, + "only the domain CPU Sets actually reported" + ); + assert_eq!( + observation.numa_domains_unreported, 1, + "and the one nobody reported is counted apart from it" + ); +} + +#[test] +fn a_contradicted_counter_puts_the_parse_in_doubt_even_with_nothing_else_wrong() { + // The renderer gates its hardware conclusions on `parse_in_doubt`, and this + // is the half that was missing. A counter read independently from Windows + // contradicting the walk is the strongest evidence the parsed list is + // short: a crate reporting one group where GetActiveProcessorGroupCount + // says two did not miscount, it never saw the second group's records -- + // including its caches. Yet nothing failed to decode, coherence agrees, and + // no domain is CPU-Sets-only, so `parse_incomplete` is EMPTY and the + // condition the renderer used printed "this machine reports no L3 at all" + // directly above "=> DISAGREE". + let mut observation = agreeing_observation(); + observation.groups = 1; + observation.raw_group_count = 2; + + let check = observation.cross_check(); + assert!( + check.parse_incomplete.is_empty(), + "the fixture must reach this through disagreements alone, or it proves nothing: {check:?}" + ); + assert!(check.parse_in_doubt(), "{check:?}"); +} + +#[test] +fn a_counter_that_could_not_be_read_does_not_put_the_parse_in_doubt() { + // The other half of `parse_in_doubt`'s claim, and the one that would rot + // silently: excluding `not_compared` is a decision, not an oversight. A + // GetActiveProcessorCount that returned its failure sentinel says nothing + // about the topology parse, so caveating the cache conclusions on it would + // assert a doubt this run does not have. + // + // The verdict still moves -- this run cannot claim agreement -- which is + // exactly the distinction: the two conditions answer different questions. + let mut observation = agreeing_observation(); + observation.raw_active_processors = 0; + + let check = observation.cross_check(); + assert!(!check.not_compared.is_empty(), "{check:?}"); + assert!( + !check.parse_in_doubt(), + "a counter this probe could not read is not evidence about the parse: {check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "but it does block agreement: {check:?}" + ); +} + +#[test] +fn an_uncollected_coherence_blocks_agreement_too() { + // Unreachable from `discover`, which returns `Agreed` or `Disagreed`. Pinned + // anyway because the rule that makes this safe is fiat rather than derived: + // only `Agreed` clears the check, so a variant added to `Coherence` later + // fails the exhaustive match in `cross_check` rather than quietly becoming a + // fourth way to reach `Verdict::Agree`. + let mut observation = agreeing_observation(); + observation.coherence = windows_topology_sys::Coherence::NotCollected; + + let check = observation.cross_check(); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{check:?}" + ); + assert_eq!(check.parse_incomplete.len(), 1, "{check:?}"); +} + +#[test] +fn a_real_disagreement_still_outranks_an_incomplete_parse() { + // Both conditions at once. The verdict must stay `Disagree`, because that is + // the only one that sends a reader to audit the crate's parse -- an + // incomplete parse must not be able to downgrade a genuine contradiction + // into a caveat. + let mut observation = agreeing_observation(); + observation.groups = 9; + observation.coherence = windows_topology_sys::Coherence::NotCollected; + + let check = observation.cross_check(); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Disagree, + "{check:?}" + ); + assert_eq!(check.disagreements.len(), 1, "{check:?}"); + assert_eq!( + check.parse_incomplete.len(), + 1, + "and the caveat is still reported rather than swallowed: {check:?}" + ); +} + +#[test] +fn a_real_disagreement_outranks_a_counter_that_could_not_be_read() { + // Both at once. The verdict must stay `Disagree`, because a finding about + // the crate's parse is not softened by an unrelated failed read -- and the + // skipped counter must still be listed, so the reader knows the + // disagreement is not the whole picture. + let mut observation = agreeing_observation(); + observation.raw_highest_numa_node = None; + observation.groups = 2; + + let check = observation.cross_check(); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Disagree, + "{check:?}" + ); + assert_eq!(check.disagreements.len(), 1, "{check:?}"); + assert_eq!(check.not_compared.len(), 1, "{check:?}"); +} + +#[test] +fn every_core_reports_processors_and_smt_agrees_with_the_count() { + let observation = crate::topology::measure().expect("topology discovery"); + + for core in &observation.cores { + assert!( + core.processors >= 1, + "a core with no processors is a parse error, not a machine" + ); + assert!( + !core.contradicts_itself(), + "SMT is exactly the condition of a core carrying more than one \ + processor, and this record disagrees with itself: {core:?}" + ); + } +} + +#[test] +fn every_cache_level_reports_at_least_one_domain_with_at_least_one_processor() { + let observation = crate::topology::measure().expect("topology discovery"); + + for cache in &observation.caches { + assert!( + cache.level >= 1, + "a cache level of zero is a parse error, not a machine" + ); + // `domains() >= 1` asserted explicitly, and it is what the test's name + // promises. Without it the assertion below passes VACUOUSLY on the case + // worth catching: `all()` over an empty iterator is `true`. + // + // That state is reachable rather than theoretical. `measure` builds one + // summary per entry in `cache_levels()`, which filters on kind alone, + // while the per-level partition count drops domains covering no + // processors -- so a level can appear here with no domains at all, and + // the probe would print `L3 0 domain(s)` while this test, whose whole + // job is to catch a parsing regression on whatever hardware CI runs, + // reported success. + // + // Two ways in, and they differ in whether anything else notices. A + // truncated trailing `GROUP_AFFINITY` array raises an anomaly, so + // `parse_in_doubt` already covers it. A well-formed array whose MASK is + // zero does not: `read_cache_body` reports only a declared-versus-read + // count mismatch and never inspects mask contents, and `AnomalyKind` + // has no variant for it. This comment used to claim a zero mask was + // recorded as an anomaly, which is why that case went uncaveated until + // `cross_check` gained an entry for a level with no partitions. + assert!( + cache.domains() >= 1, + "cache level {} reports no domains at all, which is a parse \ + regression rather than a machine", + cache.level + ); + assert!( + cache.processors_per_domain.iter().all(|&span| span >= 1), + "a cache domain covering no processors is a parse error" + ); + } +} + +#[test] +fn the_outermost_partitioning_cache_is_the_deepest_level_that_splits_the_machine() { + let observation = crate::topology::measure().expect("topology discovery"); + + // What this can check, and deliberately no more. `Observation`'s caches are + // SUMMARIES -- a level, a domain count, and the size of each domain -- with + // processor membership discarded. "Outermost" is defined by set inclusion + // between partitions, so nothing here can re-derive the selection, and the + // two attempts to do so anyway were both wrong: + // + // - Ordering candidates by LEVEL NUMBER is the rule the topology crate + // abandoned, because a higher number is not always coarser. This module's + // own `outermost_partitioning_cache` doc records it, and the synthetic + // test below builds the counterexample: a valid L2 outermost partition + // alongside a finer L3 that still partitions. A level-number assertion + // here would fail that legitimate topology. + // + // - Reading `None` as "no level partitions this machine" is false for the + // same reason the renderer was corrected: `None` is also the answer when + // two partitionings are incomparable, which is a deliberate ambiguity + // result rather than a claim about the hardware. + // + // So this asserts the two properties that survive the summarising: a + // selection is one of the surveyed levels and genuinely partitions, and the + // selection agrees with the level the survey captured. + match observation.outermost_partitioning_cache() { + Some(chosen) => { + assert!( + chosen.domains() > 1, + "a level that does not partition cannot be the partitioning level" + ); + // Asserted against the RAW fields, because anything asked of + // `chosen` is answered by how it was obtained. `chosen` is + // `caches.iter().find(|c| c.level == partitioning_cache_level)`, so + // "the looked-up summary is the level the survey selected" and "the + // selected level is one this survey recorded" are both true by + // construction -- an earlier version asserted exactly those two, and + // neither could fail. This is the same defect as the `None` arm's, + // in the same function, and fixing one arm without sweeping the + // other is how it survived. + // + // Exactly one entry at the level is the property that CAN fail: + // `find` takes the first match, so a duplicated level would make the + // survey's answer depend on enumeration order while every assertion + // about `chosen` still passed. + let at_level = observation + .caches + .iter() + .filter(|c| Some(c.level) == observation.partitioning_cache_level) + .count(); + assert_eq!( + at_level, 1, + "the survey must record exactly one summary for the selected \ + level; {at_level} would make the lookup order-dependent" + ); + } + // `None` must mean the survey captured no level -- and ONLY that. + // + // The other way to reach this arm is a survey that DID capture a level + // whose summary is missing from `caches`, which is a defect in this + // probe: the level the crate named and the summaries the survey carries + // have to agree, or every conclusion drawn from `caches` is about a + // different machine than the one the crate answered about. + // + // The justification used to be a renderer string -- that the report + // "would then print 'no unique outermost partitioning cache was + // established'". It no longer does: that state is + // `PartitioningCache::SummaryMissing`, which the renderer prints as its + // own "BUG IN THIS PROBE" arm, pinned by + // `a_named_level_with_no_summary_is_a_probe_defect_not_a_fact_about_the_machine`. + // Naming a downstream behaviour to justify an invariant is what let the + // comment rot when that behaviour was fixed -- and, worse, gave a reader + // a true observation ("the renderer already handles this") that argues + // for deleting the only assertion guarding the invariant itself. + // + // Written as one condition on purpose. The previous version asserted + // `captured.is_none() || summary_is_missing`, which is the disjunction + // of the two conditions that reach this arm -- unfalsifiable, and it + // permitted the exact inconsistency its own failure message named. + None => assert!( + observation.partitioning_cache_level.is_none(), + "the survey captured level {:?} and carries no summary for it, so the \ + level the crate named and the summaries reported here disagree", + observation.partitioning_cache_level + ), + } +} + +#[test] +fn every_policy_would_produce_at_least_one_domain() { + // The live host first, which is the case that must never regress. + let observation = crate::topology::measure().expect("topology discovery"); + assert_every_policy_is_usable(&observation, "the live host"); +} + +#[test] +fn every_policy_survives_a_topology_that_reported_no_relationships() { + // The degenerate case, and it needs constructing because a healthy machine + // cannot produce it. `MachineMemoryTopology::discover` returns `Ok` when a + // `PROCESSOR_RELATIONSHIP` record is shorter than its declared size: the + // record is dropped and an enumeration anomaly is recorded, so a topology + // with no package and no core relationships is a legal result rather than a + // parse failure. + // + // Running this only against `measure()` was why `by-package` and `by-core` + // could return zero unnoticed -- every real host has both, so the clamp + // those two policies were missing was never exercised. + let mut observation = agreeing_observation(); + observation.packages = 0; + observation.cores = Vec::new(); + observation.numa_domains = 0; + observation.numa_domains_without_processors = 0; + observation.caches = Vec::new(); + observation.partitioning_cache_level = None; + + assert_every_policy_is_usable(&observation, "a topology with no relationships"); +} + +#[test] +fn every_policy_survives_a_selected_cache_level_reporting_no_domains() { + // The one arm the other fixtures never reach. All of them leave + // `partitioning_cache_level` as `None`, so `domain_counts` took the `None` + // default and the selected-value path went untested -- which is how that + // arm kept returning `c.domains` raw while the comment above it claimed + // every count was clamped. + let mut observation = agreeing_observation(); + observation.caches = vec![crate::topology::CacheLevel { + level: 3, + processors_per_domain: Vec::new(), + }]; + observation.partitioning_cache_level = Some(3); + + assert_every_policy_is_usable(&observation, "a selected level reporting no domains"); +} + +#[test] +fn every_policy_survives_more_processorless_numa_domains_than_domains() { + // The two counters are read independently, so their relationship is an + // assumption rather than a guarantee. An unsigned subtraction that trusted + // it would panic instead of reporting. + let mut observation = agreeing_observation(); + observation.numa_domains = 1; + observation.numa_domains_without_processors = 4; + + assert_every_policy_is_usable(&observation, "more processorless domains than domains"); +} + +/// A fleet sized at zero domains performs no I/O at all, so every policy must +/// name at least one whatever the survey found. +fn assert_every_policy_is_usable(observation: &crate::topology::Observation, what: &str) { + let counts = observation.domain_counts(); + + // The SET first, then the values. Iterating alone passed vacuously on an + // empty list and on a single bogus entry, so a mutation sweep found + // `domain_counts` replaceable by `vec![]`, `vec![("", 1)]` and + // `vec![("xyzzy", 1)]` with every caller still green -- the helper named + // "every policy" never checked that any policy was there. + let names: Vec<&str> = counts.iter().map(|(name, _)| *name).collect(); + assert_eq!( + names, + vec![ + "single", + "by-package", + "by-numa-domain-with-processors", + "by-outermost-partitioning-cache", + "by-core", + ], + "on {what}, the reported policies are not the five this probe surveys" + ); + + for (name, count) in counts { + assert!( + count >= 1, + "on {what}, policy {name} would produce {count} domains, and a fleet \ + of zero domains can perform no I/O" + ); + } +} + +// --- M5+.3: the partitioning rule has one implementation --- + +#[test] +fn observe_captures_the_crates_partitioning_level_where_re_derivation_would_differ() { + // Drives `observe`, which is what the synthetic test below does NOT: that + // one hand-sets `partitioning_cache_level`, so it pins the lookup and + // leaves the CAPTURE untested. Swapping `observe`'s call to the crate for + // the old "highest level with more than one domain" would leave it green + // while the report selected the wrong level -- the regression `SH-16.9` + // exists to prevent. + // + // Four processors. L2 splits them into two blocks, L3 into four, so L3 + // REFINES L2 and the coarsest -- the crate's answer -- is L2. Ordering by + // level number answers L3. The two rules disagree here on purpose. + use windows_topology_sys::{ + CacheKind, Domain, DomainKind, MachineMemoryTopology, Observation, Observed, Processor, + ProcessorId, Source, + }; + + let cache = |level: u8| DomainKind::Cache { + level, + associativity: 8, + line_size: 64, + size_bytes: 1 << 20, + cache_type: CacheKind::Unified, + }; + let walk = || vec![Observation::new(Source::RelationshipWalk, 0)]; + let block = |members: &[u8]| -> windows_topology_sys::ProcessorSet { + members.iter().map(|n| (0u16, *n)).collect() + }; + + let topology = MachineMemoryTopology { + processors: (0..4) + .map(|number| Processor { + id: ProcessorId { group: 0, number }, + online: true, + capacity: 0, + }) + .collect(), + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: block(&[0, 1, 2, 3]), + observations: walk(), + }, + Domain { + kind: DomainKind::Memory { + memory_bytes: Observed::NotObserved, + }, + processors: block(&[0, 1, 2, 3]), + observations: walk(), + }, + Domain { + kind: cache(2), + processors: block(&[0, 1]), + observations: walk(), + }, + Domain { + kind: cache(2), + processors: block(&[2, 3]), + observations: walk(), + }, + Domain { + kind: cache(3), + processors: block(&[0]), + observations: walk(), + }, + Domain { + kind: cache(3), + processors: block(&[1]), + observations: walk(), + }, + Domain { + kind: cache(3), + processors: block(&[2]), + observations: walk(), + }, + Domain { + kind: cache(3), + processors: block(&[3]), + observations: walk(), + }, + ], + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 4, + 1, + Some(0), + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!( + observation.partitioning_cache_level, + Some(2), + "the crate answers L2 as the coarsest partitioning level; ordering by \ + level number would answer L3, and the survey must not re-derive" + ); +} + +#[test] +fn the_survey_reports_the_topology_crates_partitioning_level_not_its_own() { + // The restatement this removes differed from the crate's answer in two + // ways: it omitted the pairwise-disjointness check, and it ordered + // candidates by LEVEL NUMBER, which the topology crate stopped doing + // because a higher number is not always coarser. + // + // Here the higher level is the finer partition, so the two rules disagree: + // the old `max_by_key(level)` would answer L3, and asking the crate answers + // L2. The survey must report what the crate says. + let mut observation = agreeing_observation(); + observation.caches = vec![ + crate::topology::CacheLevel { + level: 2, + processors_per_domain: vec![2, 2], + }, + crate::topology::CacheLevel { + level: 3, + processors_per_domain: vec![1, 1, 1, 1], + }, + ]; + observation.partitioning_cache_level = Some(2); + + assert_eq!( + observation.outermost_partitioning_cache().map(|c| c.level), + Some(2), + "the survey must not re-derive; it looks up what the crate decided" + ); +} + +#[test] +fn no_partitioning_level_is_a_real_answer_in_the_survey_too() { + // The fixture's level DOES partition -- four domains -- while the crate + // captured no level. That combination is what makes this test able to fail: + // any rule that re-derived the answer from the summaries would see a + // partitioning level and report L3, so only a survey that genuinely looks + // up what the crate decided answers `None` here. + // + // An earlier version used `domains: 1`. Every re-derivation rule requires + // `domains > 1`, so a survey that re-derived would also have answered + // `None`, and the test could not tell the two apart -- it was named for a + // regression it did not detect. + let mut observation = agreeing_observation(); + observation.caches = vec![crate::topology::CacheLevel { + level: 3, + processors_per_domain: vec![1, 1, 1, 1], + }]; + observation.partitioning_cache_level = None; + + assert!( + observation.outermost_partitioning_cache().is_none(), + "the survey must report the crate's answer, not re-derive one from a \ + level that happens to partition" + ); +} + +// --- the report's claims, pinned --- +// +// A mutation sweep over the renderer found 13 of 13 mutants surviving: `render` +// could return "xyzzy" and the suite stayed green. The survivors were exactly +// the claims that cost the most review rounds -- the "no L3 at all" note, the +// caveat gate on an absent partitioning answer, the heterogeneous-core note -- +// each of which had been checked by running the binary and reading the output, +// which nothing repeats on a later change. + +/// A stand-in for the host fingerprint line. +/// +/// The renderer takes it as an argument rather than reading it: `banner_line` +/// runs a topology discovery of its own, so calling it inside `report` made +/// this module's claim to be testable without a host false of its very first +/// line, and made a second platform read that neither bracketed the other. +const BANNER: &str = "host: TEST-FIXTURE"; + +/// An observation whose report should be free of every caveat. +fn clean_observation() -> crate::topology::Observation { + let mut observation = agreeing_observation(); + observation.caches = vec![ + crate::topology::CacheLevel { + level: 2, + processors_per_domain: vec![2, 2], + }, + crate::topology::CacheLevel { + level: 3, + processors_per_domain: vec![4], + }, + ]; + observation.partitioning_cache_level = Some(2); + observation +} + +#[test] +fn the_report_names_the_outermost_partitioning_level_the_crate_chose() { + let text = crate::topology_report::report(BANNER, &clean_observation()); + + assert!( + text.contains("outermost cache that partitions the processors it covers: L2 (2 domains)"), + "{text}" + ); + assert!( + text.contains(r#""outermost_partitioning_cache":"level""#), + "{text}" + ); +} + +#[test] +fn the_no_l3_note_is_a_hardware_claim_only_when_the_parse_is_whole() { + // The claim that cost two review rounds. Asserted as hardware when nothing + // is in doubt, and as a fact about the parse when something is -- otherwise + // a host whose L3 record alone failed to decode is filed as an ARM64-style + // no-L3 machine. + let mut whole = clean_observation(); + whole.caches = vec![crate::topology::CacheLevel { + level: 2, + processors_per_domain: vec![2, 2], + }]; + let text = crate::topology_report::report(BANNER, &whole); + assert!( + text.contains("NOTE: this machine reports no L3 at all"), + "a whole parse with no L3 states the hardware fact: {text}" + ); + + let mut in_doubt = whole.clone(); + in_doubt.enumeration_anomalies = vec![windows_topology_sys::EnumerationAnomaly { + source: windows_topology_sys::Source::RelationshipWalk, + offset: 64, + kind: windows_topology_sys::AnomalyKind::Undersized { + declared: 8, + minimum: 48, + }, + }]; + let text = crate::topology_report::report(BANNER, &in_doubt); + assert!( + !text.contains("NOTE: this machine reports no L3 at all"), + "a parse in doubt must not assert the hardware fact: {text}" + ); + assert!( + text.contains("no L3 decoded on this run"), + "it states what the parse showed instead: {text}" + ); +} + +#[test] +fn the_report_caveats_its_cache_conclusions_exactly_when_the_parse_is_in_doubt() { + let caveat = "This run did not establish that the parse"; + + let text = crate::topology_report::report(BANNER, &clean_observation()); + assert!( + !text.contains(caveat), + "nothing in doubt, no caveat: {text}" + ); + + // A disagreement, which `parse_in_doubt` includes. + let mut disagreeing = clean_observation(); + disagreeing.raw_group_count = 2; + let text = crate::topology_report::report(BANNER, &disagreeing); + assert!(text.contains(caveat), "{text}"); + + // A failed counter read, which it deliberately does NOT: that bears on this + // probe's ability to check the parse, not on the parse. + let mut unread = clean_observation(); + unread.raw_active_processors = 0; + let text = crate::topology_report::report(BANNER, &unread); + assert!( + !text.contains(caveat), + "a counter this probe could not read is not evidence about the parse: {text}" + ); + assert!(text.contains(r#""cross_check":"incomplete""#), "{text}"); +} + +#[test] +fn an_absent_partitioning_answer_says_which_absent_answer_it_is() { + // Three findings that all rendered as `null` before they were told apart. + let mut none_reported = clean_observation(); + none_reported.caches = Vec::new(); + none_reported.partitioning_cache_level = None; + let text = crate::topology_report::report(BANNER, &none_reported); + assert!( + text.contains("no cache levels were reported at all"), + "{text}" + ); + assert!( + text.contains(r#""outermost_partitioning_cache":"no_levels_reported""#), + "{text}" + ); + + let mut nothing_partitions = clean_observation(); + nothing_partitions.caches = vec![crate::topology::CacheLevel { + level: 3, + processors_per_domain: vec![4], + }]; + nothing_partitions.partitioning_cache_level = None; + let text = crate::topology_report::report(BANNER, ¬hing_partitions); + assert!( + text.contains("no cache level reported more than one domain"), + "{text}" + ); + assert!( + text.contains(r#""outermost_partitioning_cache":"none""#), + "{text}" + ); + + let mut not_unique = clean_observation(); + not_unique.partitioning_cache_level = None; + let text = crate::topology_report::report(BANNER, ¬_unique); + assert!( + text.contains("more than one distinct domain"), + "and it does not claim the machine IS partitioned: {text}" + ); + assert!( + text.contains(r#""outermost_partitioning_cache":"not_unique""#), + "{text}" + ); +} + +#[test] +fn the_heterogeneous_note_appears_only_with_more_than_one_efficiency_class() { + let note = "heterogeneous"; + let one = crate::topology_report::report(BANNER, &clean_observation()); + assert!(!one.contains(note), "{one}"); + + let mut mixed = clean_observation(); + mixed.cores = vec![ + crate::topology::CoreShape { + simultaneous_multithreading: true, + efficiency_class: 0, + processors: 2, + }, + crate::topology::CoreShape { + simultaneous_multithreading: true, + efficiency_class: 1, + processors: 2, + }, + ]; + let text = crate::topology_report::report(BANNER, &mixed); + assert!( + text.contains(note), + "two classes means an unconstrained thread can land on an efficiency core: {text}" + ); + assert!(text.contains(r#""efficiency_classes":2"#), "{text}"); +} + +#[test] +fn a_run_that_could_not_measure_still_emits_a_machine_readable_row() { + // Otherwise a host where discovery FAILED is indistinguishable from a job + // that never ran the probe, which silently excludes exactly the hosts most + // worth counting. + let text = crate::topology_report::report_unmeasured(BANNER, &std::io::Error::other("no")); + + assert!( + text.contains("MachineMemoryTopology::discover failed"), + "{text}" + ); + assert!(text.contains(r#""cross_check":"not_measured""#), "{text}"); +} + +#[test] +fn every_report_carries_the_banner_and_title() { + // The banner is what lets a captured report be attributed to a machine, and + // the taint marker travels with it. Both reports open with it, so neither + // can be pasted somewhere and compared against anything. + for text in [ + crate::topology_report::report(BANNER, &clean_observation()), + crate::topology_report::report_unmeasured(BANNER, &std::io::Error::other("no")), + ] { + assert!( + text.contains( + "== processor topology, and what each partitioning policy would yield ==" + ), + "{text}" + ); + assert_eq!( + text.lines().next().unwrap_or_default(), + BANNER, + "the first line must be the banner the caller supplied: {text}" + ); + } +} + +#[test] +fn the_numa_line_names_domains_only_one_source_reported() { + let quiet = crate::topology_report::report(BANNER, &clean_observation()); + assert!(!quiet.contains("reported only by CPU Sets"), "{quiet}"); + + let mut split = clean_observation(); + split.numa_domains = 2; + split.numa_domains_only_in_cpu_sets = 1; + let text = crate::topology_report::report(BANNER, &split); + assert!( + text.contains("1 reported only by CPU Sets, never by the relationship walk"), + "{text}" + ); + assert!( + text.contains(r#""numa_domains_only_in_cpu_sets":1"#), + "{text}" + ); +} + +#[test] +fn the_missing_level_caveat_needs_both_doubt_and_an_absent_level() { + // A conjunction, and each half matters. It reads "the level that would have + // partitioned this machine is missing", which is a claim about why the + // answer is absent -- so it must not appear when the answer is present, nor + // when nothing is in doubt. + let caveat = "Or the parse is not whole and the level that would have partitioned"; + + // In doubt, but a level WAS named: the caveat above it already covers this. + let mut doubted_with_level = clean_observation(); + doubted_with_level.raw_group_count = 2; + let text = crate::topology_report::report(BANNER, &doubted_with_level); + assert!( + !text.contains(caveat), + "a named level is not missing, whatever else is in doubt: {text}" + ); + + // A level is absent, but nothing is in doubt: the absence is a finding + // about the machine, not about the parse. + let mut absent_but_whole = clean_observation(); + absent_but_whole.partitioning_cache_level = None; + let text = crate::topology_report::report(BANNER, &absent_but_whole); + assert!( + !text.contains(caveat), + "a whole parse does not blame the parse for the absence: {text}" + ); + + // Both. + let mut both = clean_observation(); + both.partitioning_cache_level = None; + both.raw_group_count = 2; + let text = crate::topology_report::report(BANNER, &both); + assert!(text.contains(caveat), "{text}"); +} + +#[test] +fn an_empty_core_or_package_record_blocks_agreement() { + // The empty-record twins of `numa_domains_without_processors`. A zero + // affinity mask raises no anomaly, so a spurious empty record inflates the + // count and the policy derived from it while nothing else notices. The + // live-host test already calls an empty core a parse error; without this, + // cross_check would still certify one. + for (label, mutate) in [ + ( + "core(s) and", + Box::new(|o: &mut crate::topology::Observation| o.cores_without_processors = 1) + as Box, + ), + ( + "package(s) cover no processors", + Box::new(|o: &mut crate::topology::Observation| o.packages_without_processors = 1), + ), + ] { + let mut observation = agreeing_observation(); + mutate(&mut observation); + + let check = observation.cross_check(); + assert!(check.disagreements.is_empty(), "{label}: {check:?}"); + assert!( + check.parse_incomplete.iter().any(|c| c.contains(label)), + "{label}: {check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{label}: {check:?}" + ); + } +} + +#[test] +fn observe_counts_empty_core_and_package_records() { + // Drives the extraction so the counts cannot be dropped silently. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Provenance, + Source, + }; + + let walk = || vec![Observation::new(Source::RelationshipWalk, 0)]; + let topology = MachineMemoryTopology { + processors: vec![Processor { + id: ProcessorId { + group: 0, + number: 0, + }, + online: true, + capacity: 0, + }], + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: DomainKind::Package, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: walk(), + }, + // The spurious empty records: a zero affinity mask decodes cleanly. + Domain { + kind: DomainKind::Package, + processors: windows_topology_sys::ProcessorSet::default(), + observations: walk(), + }, + Domain { + kind: DomainKind::Core { + simultaneous_multithreading: false, + efficiency_class: 0, + }, + processors: windows_topology_sys::ProcessorSet::default(), + observations: walk(), + }, + ], + provenance: Provenance::Measured, + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 1, + 1, + None, + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!(observation.packages, 2, "the empty record is still counted"); + assert_eq!(observation.packages_without_processors, 1); + assert_eq!(observation.cores_without_processors, 1); + assert!(observation.cross_check().parse_in_doubt()); +} + +#[test] +fn a_relation_a_caller_described_is_not_a_measurement() { + // Provenance per RELATION, not just per topology. `Provenance::Measured` is + // object-level and the crate is explicit that it permits hand-inserted + // relations -- `Source::Description` exists for exactly that mixed case -- + // so a measured topology can carry counted relations nobody read from the + // platform. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Provenance, + Source, + }; + + let build = |source| MachineMemoryTopology { + processors: vec![Processor { + id: ProcessorId { + group: 0, + number: 0, + }, + online: true, + capacity: 0, + }], + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: DomainKind::Package, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(source, 0)], + }, + Domain { + kind: DomainKind::Core { + simultaneous_multithreading: false, + efficiency_class: 0, + }, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + ], + provenance: Provenance::Measured, + ..MachineMemoryTopology::default() + }; + + let described = crate::topology::observe( + &build(Source::Description), + 1, + 1, + None, + crate::topology::BracketOutcome::HeldStill, + ); + assert_eq!(described.described_relations, 1); + assert!( + described + .cross_check() + .parse_incomplete + .iter() + .any(|c| c.contains("described by a caller")), + "a measured topology carrying a described relation is not all measured: {:?}", + described.cross_check() + ); + + // The control: the same shape, all walk-sourced, carries no such entry. + let walked = crate::topology::observe( + &build(Source::RelationshipWalk), + 1, + 1, + None, + crate::topology::BracketOutcome::HeldStill, + ); + assert_eq!(walked.described_relations, 0); + assert!( + !walked + .cross_check() + .parse_incomplete + .iter() + .any(|c| c.contains("described by a caller")), + "{:?}", + walked.cross_check() + ); +} + +#[test] +fn a_bracket_left_open_is_not_the_same_as_a_machine_that_held_still() { + use crate::topology::{BracketOutcome, bracket_outcome}; + + // Three outcomes, not two: an open bracket establishes neither change nor + // stability, so comparing anyway could file a concurrent hot-add against + // the parse. One value rather than two flags, because "changed, and also + // held still" is meaningless and two booleans can be set to say it. + let good = (4u32, 1u16, Some(0u32)); + assert_eq!(bracket_outcome(good, good), BracketOutcome::HeldStill); + assert_eq!( + bracket_outcome(good, (8, 1, Some(0))), + BracketOutcome::Changed + ); + + for open in [(0u32, 1u16, Some(0u32)), (4, 0, Some(0)), (4, 1, None)] { + assert_eq!( + bracket_outcome(open, good), + BracketOutcome::NotEstablished, + "a bracket open at one end establishes neither: {open:?}" + ); + } + + // And the observation says which of the three it was. + let mut open = agreeing_observation(); + open.bracket = crate::topology::BracketOutcome::NotEstablished; + let check = open.cross_check(); + assert!(check.disagreements.is_empty(), "{check:?}"); + assert!( + check + .not_compared + .iter() + .any(|c| c.contains("bracket around the parse was not closed")), + "{check:?}" + ); + assert!( + !check.parse_in_doubt(), + "still not a claim about the parse: {check:?}" + ); +} + +#[test] +fn observe_will_not_claim_a_bracket_it_was_not_given() { + // The bracket is the CALLER's fact. `observe` set it to `HeldStill` itself, + // which claims "every counter was read twice and none moved" -- something it + // never establishes, having been handed one set of counters and no way to + // know what produced them. A caller's stale pair could then reach `Agree`, + // or be filed against the parse as a `Disagree`. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Provenance, + Source, + }; + + let topology = MachineMemoryTopology { + processors: vec![Processor { + id: ProcessorId { + group: 0, + number: 0, + }, + online: true, + capacity: 0, + }], + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: DomainKind::Package, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: DomainKind::Core { + simultaneous_multithreading: false, + efficiency_class: 0, + }, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + ], + provenance: Provenance::Measured, + ..MachineMemoryTopology::default() + }; + + let unbracketed = crate::topology::observe( + &topology, + 1, + 1, + None, + crate::topology::BracketOutcome::NotEstablished, + ); + let check = unbracketed.cross_check(); + assert!( + check.disagreements.is_empty(), + "an unestablished bracket is not a finding about the parse: {check:?}" + ); + assert!( + check + .not_compared + .iter() + .any(|c| c.contains("bracket around the parse was not closed")), + "{check:?}" + ); + assert_ne!( + check.verdict(), + crate::topology::Verdict::Agree, + "and it cannot certify agreement: {check:?}" + ); +} + +#[test] +fn a_relation_no_source_reported_is_counted_whatever_its_kind() { + // Checked only inside the memory arm, so a hand-inserted CORE, PACKAGE or + // CACHE with no observations was counted -- and could change a policy or the + // cache partitioning -- with every quality check still clear. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Provenance, + Source, + }; + + let walk = || vec![Observation::new(Source::RelationshipWalk, 0)]; + let topology = MachineMemoryTopology { + processors: vec![Processor { + id: ProcessorId { + group: 0, + number: 0, + }, + online: true, + capacity: 0, + }], + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: DomainKind::Package, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: DomainKind::Core { + simultaneous_multithreading: false, + efficiency_class: 0, + }, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: walk(), + }, + // A cache nobody reported. Not a memory domain, so the old check + // could not see it -- yet it joins `caches` and can move the + // partitioning answer. + Domain { + kind: DomainKind::Cache { + level: 2, + associativity: 8, + line_size: 64, + size_bytes: 1 << 20, + cache_type: windows_topology_sys::CacheKind::Unified, + }, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: Vec::new(), + }, + ], + provenance: Provenance::Measured, + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 1, + 1, + None, + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!(observation.unreported_relations, 1); + let check = observation.cross_check(); + assert!(check.disagreements.is_empty(), "{check:?}"); + assert!( + check + .parse_incomplete + .iter() + .any(|c| c.contains("carry no observation from any source")), + "{check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{check:?}" + ); +} + +#[test] +fn a_measured_topology_reporting_no_processors_or_groups_blocks_agreement() { + // Zero is how BOTH raw counters report failure, so a zero parse beside a + // failed read was filed only as `not_compared`: `parse_in_doubt` stayed + // false and the report was free to make uncaveated hardware claims about a + // machine its own parse says has no processors at all. The guard on the + // package and core checks reads `online_processors > 0`, so the impossible + // case silently switched those off as well. + for (label, mutate) in [ + ( + "no online processors", + Box::new(|o: &mut crate::topology::Observation| { + o.online_processors = 0; + o.raw_active_processors = 0; + }) as Box, + ), + ( + "no processor groups", + Box::new(|o: &mut crate::topology::Observation| { + o.groups = 0; + o.raw_group_count = 0; + }), + ), + ] { + let mut observation = agreeing_observation(); + mutate(&mut observation); + + let check = observation.cross_check(); + assert!( + check.disagreements.is_empty(), + "{label}: a counter that could not be read is not the crate contradicting it: \ + {check:?}" + ); + assert!( + check.parse_incomplete.iter().any(|c| c.contains(label)), + "{label}: {check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{label}: {check:?}" + ); + assert!( + check.parse_in_doubt(), + "{label}: so the renderer caveats what it says about the hardware: {check:?}" + ); + } +} + +#[test] +fn both_absent_counts_are_named_together_rather_than_one_standing_for_the_pair() { + // The rule is stated over the LIST, so a topology missing both must name + // both. Reporting only the first would leave the second to be discovered by + // fixing the first and running again. + let mut observation = agreeing_observation(); + observation.online_processors = 0; + observation.raw_active_processors = 0; + observation.groups = 0; + observation.raw_group_count = 0; + + let check = observation.cross_check(); + assert!( + check + .parse_incomplete + .iter() + .any(|c| c.contains("no online processors and processor groups")), + "{check:?}" + ); +} + +#[test] +fn a_topology_nobody_measured_is_not_accused_of_describing_no_machine() { + // The premise of the rule above is that a RUNNING machine was read. A + // hand-built topology carrying no processors is not describing a machine + // wrongly; it is not describing one at all, which + // `topology_was_measured` already reports. + let mut observation = agreeing_observation(); + observation.topology_was_measured = false; + observation.online_processors = 0; + observation.raw_active_processors = 0; + observation.groups = 0; + observation.raw_group_count = 0; + + let check = observation.cross_check(); + assert!( + !check + .parse_incomplete + .iter() + .any(|c| c.contains("cannot have none")), + "an unmeasured topology is not held to what a running machine must have: {check:?}" + ); + assert!( + check + .parse_incomplete + .iter() + .any(|c| c.contains("was not measured from a running machine")), + "and the reason it is exempt is itself reported: {check:?}" + ); +} + +#[test] +fn two_walk_records_of_one_kind_claiming_a_processor_block_agreement() { + // A logical processor belongs to exactly one physical package and exactly + // one core, so two walk records covering it cannot both describe this + // machine. Nothing else reaches it: no raw counter measures packages or + // cores, an overlapping record raises no enumeration anomaly, and both + // records are non-empty, so the `*_without_processors` counts stay clear + // while `packages` and `by-core` hold the duplicate. + let mut observation = agreeing_observation(); + observation.overlapping_walk_relations = 2; + + let check = observation.cross_check(); + assert!(check.disagreements.is_empty(), "{check:?}"); + assert!( + check + .parse_incomplete + .iter() + .any(|c| c.contains("share a processor with another")), + "{check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{check:?}" + ); + assert!(check.parse_in_doubt(), "{check:?}"); +} + +#[test] +fn observe_counts_walk_packages_that_claim_the_same_processor() { + // Drives the extraction, and pins the EXCLUSION that makes the rule honest: + // a core only CPU Sets described overlaps a walk core by design -- the crate + // keeps both groupings deliberately -- and `cores_only_in_cpu_sets` is the + // count that reports it. Folding it in here would report one disagreement + // twice. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Provenance, + Source, + }; + + let walk = || vec![Observation::new(Source::RelationshipWalk, 0)]; + let core = |smt: bool| DomainKind::Core { + simultaneous_multithreading: smt, + efficiency_class: 0, + }; + let topology = MachineMemoryTopology { + processors: (0..2) + .map(|number| Processor { + id: ProcessorId { group: 0, number }, + online: true, + capacity: 0, + }) + .collect(), + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8), (0, 1)].into_iter().collect(), + observations: walk(), + }, + // Two packages, both from the walk, both claiming processor 1. + Domain { + kind: DomainKind::Package, + processors: [(0u16, 0u8), (0, 1)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: DomainKind::Package, + processors: [(0u16, 1u8)].into_iter().collect(), + observations: walk(), + }, + // One core from the walk, and a differently-grouped core only CPU + // Sets described. They overlap, and that is not this rule's subject. + Domain { + kind: core(true), + processors: [(0u16, 0u8), (0, 1)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: core(false), + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::CpuSets, 0)], + }, + ], + provenance: Provenance::Measured, + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 2, + 1, + None, + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!( + observation.overlapping_walk_relations, 2, + "both packages are counted -- neither is the one that is right" + ); + assert_eq!( + observation.cores_only_in_cpu_sets, 1, + "and the overlapping core is reported as the source disagreement it is" + ); + assert_eq!( + observation.packages_without_processors, 0, + "no record is empty, which is why the existing counts do not see this" + ); + assert!(observation.cross_check().parse_in_doubt()); +} + +#[test] +fn observe_counts_walk_cores_that_claim_the_same_processor() { + // The core half of the same rule, so it cannot be dropped for one kind + // while the package half keeps passing. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Provenance, + Source, + }; + + let walk = || vec![Observation::new(Source::RelationshipWalk, 0)]; + let core = |smt: bool| DomainKind::Core { + simultaneous_multithreading: smt, + efficiency_class: 0, + }; + let topology = MachineMemoryTopology { + processors: (0..2) + .map(|number| Processor { + id: ProcessorId { group: 0, number }, + online: true, + capacity: 0, + }) + .collect(), + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8), (0, 1)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: DomainKind::Package, + processors: [(0u16, 0u8), (0, 1)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: core(true), + processors: [(0u16, 0u8), (0, 1)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: core(false), + processors: [(0u16, 1u8)].into_iter().collect(), + observations: walk(), + }, + ], + provenance: Provenance::Measured, + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 2, + 1, + None, + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!(observation.overlapping_walk_relations, 2); + assert_eq!( + observation.cores_only_in_cpu_sets, 0, + "both came from the walk, so this is not a disagreement between sources" + ); +} + +#[test] +fn walk_records_that_share_no_processor_are_not_counted_as_overlapping() { + // The negative case, so the rule cannot be satisfied by counting every + // record of a kind that has more than one. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Provenance, + Source, + }; + + let walk = || vec![Observation::new(Source::RelationshipWalk, 0)]; + let topology = MachineMemoryTopology { + processors: (0..2) + .map(|number| Processor { + id: ProcessorId { group: 0, number }, + online: true, + capacity: 0, + }) + .collect(), + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8), (0, 1)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: DomainKind::Package, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: DomainKind::Package, + processors: [(0u16, 1u8)].into_iter().collect(), + observations: walk(), + }, + ], + provenance: Provenance::Measured, + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 2, + 1, + None, + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!(observation.packages, 2); + assert_eq!(observation.overlapping_walk_relations, 0); +} + +/// A host fingerprint to bracket a measurement with, with `processors` the one +/// field a caller varies to make two readings differ. +/// +/// Internally consistent, deliberately: two logical processors per core so +/// `smt` is true by the owning crate's definition, and two cache domains so a +/// `partitioning_cache_level` that claims to divide the machine actually does. +/// The earlier version set four cores over four processors with `smt: true` and +/// a single cache domain at a partitioning level -- a host that cannot exist, +/// in the fixture for the code that reports on hosts. +fn fingerprint(processors: usize) -> windows_placement_probe::fingerprint::Fingerprint { + let half = processors / 2; + windows_placement_probe::fingerprint::Fingerprint { + arch: "test-arch", + processors, + cores: half, + smt: true, + partitioning_cache_level: Some(2), + cache_domain_sizes: vec![half, half], + efficiency_classes: vec![(0, processors)], + numa_node_sizes: vec![processors], + provenance: windows_topology_sys::Provenance::Measured, + } +} + +#[test] +fn an_unchanged_host_prints_one_banner_line() { + // The equal case must render exactly as `banner_line` always did, so every + // fingerprint string already recorded elsewhere stays comparable with this + // probe's -- and so this probe's banner matches every other probe's. + let text = crate::topology_report::attribution(&Ok(fingerprint(8)), &Ok(fingerprint(8))); + assert_eq!( + text, + windows_placement_probe::fingerprint::banner_line_for(&Ok(fingerprint(8))), + "rendered through the one place that owns the format, not a copy: {text}" + ); + assert_eq!(text.lines().count(), 1, "{text}"); +} + +#[test] +fn a_host_that_changed_across_the_run_says_so_and_keeps_both_readings() { + // The fingerprint is a topology rendering, not a name, so two readings that + // disagree mean neither identifies the machine the body describes. Both are + // kept: which one is stale is exactly what cannot be known here. + let text = crate::topology_report::attribution(&Ok(fingerprint(8)), &Ok(fingerprint(4))); + assert!(text.contains("8p/"), "{text}"); + assert!(text.contains("4p/"), "{text}"); + assert!(text.contains("HOST READINGS DISAGREE"), "{text}"); + assert!( + !text.contains("HOST NOT ESTABLISHED"), + "two readings that were both MADE did establish a difference: {text}" + ); + assert!( + !text.contains("CHANGED"), + "a difference does not establish that the HOST changed -- discovery \ + returns Ok on a parse that dropped a record, so the enumeration may \ + simply have been flaky: {text}" + ); +} + +#[test] +fn a_failed_host_read_is_not_reported_as_a_host_that_changed() { + // Compared as rendered strings, a failed read is a line like any other, so + // one failure beside one success -- or two failures whose `io::Error` text + // differs -- read as a machine that moved. That is a claim about the host + // drawn from a gap in the measurement, which is the inversion this probe + // exists to avoid: a failed read establishes neither that the host moved + // nor that it held still. + let failed = || Err(std::io::Error::other("discovery failed")); + for (label, before, after) in [ + ("failed first", failed(), Ok(fingerprint(8))), + ("failed second", Ok(fingerprint(8)), failed()), + ("failed both", failed(), failed()), + ] { + let text = crate::topology_report::attribution(&before, &after); + assert!( + !text.contains("HOST READINGS DISAGREE"), + "{label}: nothing established that the host changed: {text}" + ); + assert!( + text.contains("HOST NOT ESTABLISHED"), + "{label}: and the gap is reported rather than passed off as a host: {text}" + ); + } +} + +#[test] +fn two_failed_host_reads_with_different_messages_are_still_not_a_change() { + // The case the string comparison got wrong most quietly: both readings + // failed, so nothing about the machine was established at all, yet the two + // rendered lines differ because the errors do. + let text = crate::topology_report::attribution( + &Err(std::io::Error::other("first")), + &Err(std::io::Error::other("second")), + ); + assert!(!text.contains("HOST READINGS DISAGREE"), "{text}"); + assert!(text.contains("HOST NOT ESTABLISHED"), "{text}"); + // Both failed, so the sentence must not say "one of" them did. The arm is + // shared with the two one-sided cases, which is how the narrower wording + // came to cover this one. + assert!( + text.contains("at least one of the two readings"), + "the message does not claim a more specific state than the run established: {text}" + ); +} + +#[test] +fn a_count_holding_relations_no_platform_reported_cannot_contradict_a_counter() { + // `observe` is public and takes any topology, so a caller's described + // relation -- or one nobody reported -- raises `groups`, or a NUMA label, + // that no platform API ever produced. Comparing that against the counter + // reads the difference as the shipping crate contradicting Windows, and + // `verdict` gives `Disagree` precedence, so the report would lead with an + // accusation this run did not establish. + for (label, mutate) in [ + ( + "not measured", + Box::new(|o: &mut crate::topology::Observation| o.topology_was_measured = false) + as Box, + ), + ( + "described", + Box::new(|o: &mut crate::topology::Observation| o.described_relations = 1), + ), + ( + "unreported", + Box::new(|o: &mut crate::topology::Observation| o.unreported_relations = 1), + ), + ] { + let mut observation = agreeing_observation(); + mutate(&mut observation); + // A group the platform never reported, which the counter cannot match. + observation.groups = 2; + + assert!( + observation.counts_include_unparsed_relations(), + "{label}: the predicate is what the gate reads: {observation:?}" + ); + let check = observation.cross_check(); + assert!( + check.disagreements.is_empty(), + "{label}: a count the platform did not produce is not the crate contradicting a \ + counter: {check:?}" + ); + assert!( + check + .not_compared + .iter() + .any(|c| c.contains("could not be attributed to the parse")), + "{label}: and the reason no comparison was made is reported: {check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{label}: {check:?}" + ); + } +} + +#[test] +fn a_wholly_parsed_topology_is_still_compared_against_its_counters() { + // The negative case, so the gate above cannot be satisfied by never + // comparing anything. A real disagreement must still reach `Disagree`. + let mut observation = agreeing_observation(); + observation.groups = 2; + + assert!(!observation.counts_include_unparsed_relations()); + let check = observation.cross_check(); + assert!( + check.disagreements.iter().any(|c| c.contains("groups:")), + "{check:?}" + ); + assert_eq!(check.verdict(), crate::topology::Verdict::Disagree); +} + +#[test] +fn a_core_whose_smt_flag_contradicts_its_own_processor_count_blocks_agreement() { + // The owning crate defines the flag as "whether this core has more than one + // logical processor", so the two sit beside each other in one record and can + // be checked against each other. Nothing else reaches it: no counter + // measures cores, and a contradictory flag raises no enumeration anomaly. + // + // The asserted live-host test already held the parse to this while + // `cross_check` did not, so the probe could print that every check it could + // make had matched on a host whose own test had just gone red -- and CI now + // prints the report even when the tests fail, which is when it gets read. + for (label, core) in [ + ( + "flag says no, membership says yes", + crate::topology::CoreShape { + simultaneous_multithreading: false, + efficiency_class: 0, + processors: 2, + }, + ), + ( + "flag says yes, membership says no", + crate::topology::CoreShape { + simultaneous_multithreading: true, + efficiency_class: 0, + processors: 1, + }, + ), + ] { + assert!( + core.contradicts_itself(), + "{label}: the predicate both the test and cross_check read: {core:?}" + ); + + let mut observation = agreeing_observation(); + observation.cores = vec![core]; + + let check = observation.cross_check(); + assert!(check.disagreements.is_empty(), "{label}: {check:?}"); + assert!( + check + .parse_incomplete + .iter() + .any(|c| c.contains("disagrees with the number of processors")), + "{label}: {check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "{label}: {check:?}" + ); + } +} + +#[test] +fn a_core_whose_smt_flag_matches_its_membership_is_not_a_finding() { + // The negative case, so the rule cannot be satisfied by calling every core + // contradictory. Both consistent shapes must pass. + for core in [ + crate::topology::CoreShape { + simultaneous_multithreading: true, + efficiency_class: 0, + processors: 2, + }, + crate::topology::CoreShape { + simultaneous_multithreading: false, + efficiency_class: 0, + processors: 1, + }, + ] { + assert!(!core.contradicts_itself(), "{core:?}"); + + let mut observation = agreeing_observation(); + observation.cores = vec![core]; + assert_eq!( + observation.cross_check().verdict(), + crate::topology::Verdict::Agree, + "{core:?}" + ); + } +} + +#[test] +fn a_cache_level_numbered_zero_blocks_agreement() { + // Windows numbers cache levels from 1, so a level of 0 is a level the parse + // did not read rather than one the machine has. The asserted live-host test + // held the parse to this too, and `cross_check` did not. + let mut observation = agreeing_observation(); + observation.caches = vec![crate::topology::CacheLevel { + level: 0, + processors_per_domain: vec![4], + }]; + + let check = observation.cross_check(); + assert!(check.disagreements.is_empty(), "{check:?}"); + assert!( + check + .parse_incomplete + .iter() + .any(|c| c.contains("numbered 0")), + "{check:?}" + ); + assert_eq!(check.verdict(), crate::topology::Verdict::Incomplete); +} + +#[test] +fn a_relation_the_walk_reported_is_not_counted_as_one_a_caller_described() { + // `observed_by(Description)` also matched a relation the walk reported and a + // caller then annotated -- whose membership IS platform-backed -- so the + // entry this feeds claimed it was "described by a caller rather than + // reported by any platform API" of a relation the platform had reported. + // Every observation must be a `Description` for that sentence to be true. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Provenance, + Source, + }; + + let topology = MachineMemoryTopology { + processors: vec![Processor { + id: ProcessorId { + group: 0, + number: 0, + }, + online: true, + capacity: 0, + }], + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + // Reported by the walk AND annotated by a caller. Platform-backed. + Domain { + kind: DomainKind::Package, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![ + Observation::new(Source::RelationshipWalk, 0), + Observation::new(Source::Description, 0), + ], + }, + Domain { + kind: DomainKind::Core { + simultaneous_multithreading: false, + efficiency_class: 0, + }, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + ], + provenance: Provenance::Measured, + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 1, + 1, + None, + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!( + observation.described_relations, 0, + "the walk reported it, so it is not a relation only a caller described" + ); + assert!( + !observation.counts_include_unparsed_relations(), + "and it does not suppress the counter comparisons" + ); +} + +#[test] +fn a_caller_described_numa_label_is_not_filed_against_the_win32_counter() { + // A memory domain the walk reported AND a caller annotated is + // platform-backed, so `described_relations` does not count it and the + // provenance gate stays open. If the caller's label then joined the maximum, + // it would be compared against `GetNumaHighestNodeNumber` and the difference + // filed as a `disagreement` -- an accusation against the shipping parse for + // a node number no platform API ever produced. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Provenance, + Source, + }; + + let topology = MachineMemoryTopology { + processors: vec![Processor { + id: ProcessorId { + group: 0, + number: 0, + }, + online: true, + capacity: 0, + }], + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: DomainKind::Package, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + Domain { + kind: DomainKind::Core { + simultaneous_multithreading: false, + efficiency_class: 0, + }, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![Observation::new(Source::RelationshipWalk, 0)], + }, + // The walk says node 0. A caller says node 7 about the same domain. + Domain { + kind: DomainKind::Memory { + memory_bytes: windows_topology_sys::Observed::NotObserved, + }, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: vec![ + Observation::new(Source::RelationshipWalk, 0), + Observation::new(Source::Description, 7), + ], + }, + ], + provenance: Provenance::Measured, + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 1, + 1, + Some(0), + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!( + observation.highest_numa_node, + Some(0), + "the maximum reads what the PLATFORM reported, not what a caller added" + ); + assert!( + !observation.counts_include_unparsed_relations(), + "the domain is platform-backed, so the gate is open and the comparison IS made" + ); + + let check = observation.cross_check(); + assert!( + check.disagreements.is_empty(), + "so nothing is filed against the parse: {check:?}" + ); + assert_ne!( + check.verdict(), + crate::topology::Verdict::Disagree, + "the subject is the absent accusation, not this minimal fixture's other \ + complaints -- it carries no cache survey and no coherence: {check:?}" + ); +} + +#[test] +fn the_heterogeneity_conclusion_is_caveated_when_the_parse_is_in_doubt() { + // "An unconstrained thread can land on an efficiency core" is a claim about + // the hardware, so it is gated on the same `parse_in_doubt` as every other + // one the renderer draws. It was printed before that condition was even + // computed, which made it the single exception to a rule the design note + // states without one -- and the exception was the conclusion drawn from the + // one field the two sources are known to contradict each other about. + let mut mixed = clean_observation(); + mixed.cores = vec![ + crate::topology::CoreShape { + simultaneous_multithreading: true, + efficiency_class: 0, + processors: 2, + }, + crate::topology::CoreShape { + simultaneous_multithreading: true, + efficiency_class: 1, + processors: 2, + }, + ]; + + let whole = crate::topology_report::report(BANNER, &mixed); + assert!(whole.contains("heterogeneous"), "{whole}"); + assert!( + !whole.contains("the classes"), + "a whole parse states it without a caveat: {whole}" + ); + + // The sources disagreed about exactly this field. + mixed.processor_attribute_conflicts = 1; + assert!(mixed.cross_check().parse_in_doubt()); + + let disputed = crate::topology_report::report(BANNER, &mixed); + assert!( + disputed.contains("heterogeneous"), + "the fact is still reported: {disputed}" + ); + assert!( + disputed.contains("did not establish that the parse is whole"), + "but it is no longer stated as a settled fact about the machine: {disputed}" + ); +} + +#[test] +fn observe_counts_walk_numa_nodes_that_claim_the_same_processor() { + // A processor belongs to exactly one NUMA node, just as it belongs to one + // package and one core, so a walk reporting it in two nodes describes no + // machine -- and `numa_domains` feeds a policy directly. The overlap rule + // was written as two `+`-joined calls naming Package and Core, which is the + // enumerate-the-kinds shape this file has been bitten by before; memory was + // the kind left out. + // + // The highest-label comparison cannot stand in for it: two overlapping + // nodes can carry any labels at all, including the correct maximum, which + // is what this fixture uses. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Observed, Processor, ProcessorId, + Provenance, Source, + }; + + let walk = || vec![Observation::new(Source::RelationshipWalk, 0)]; + let memory = || DomainKind::Memory { + memory_bytes: Observed::NotObserved, + }; + let topology = MachineMemoryTopology { + processors: (0..2) + .map(|number| Processor { + id: ProcessorId { group: 0, number }, + online: true, + capacity: 0, + }) + .collect(), + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8), (0, 1)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: DomainKind::Package, + processors: [(0u16, 0u8), (0, 1)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: DomainKind::Core { + simultaneous_multithreading: true, + efficiency_class: 0, + }, + processors: [(0u16, 0u8), (0, 1)].into_iter().collect(), + observations: walk(), + }, + // Two NUMA nodes from the walk, both claiming processor 1. Both + // labelled 0, so the highest-node comparison still matches. + Domain { + kind: memory(), + processors: [(0u16, 0u8), (0, 1)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: memory(), + processors: [(0u16, 1u8)].into_iter().collect(), + observations: walk(), + }, + ], + provenance: Provenance::Measured, + ..MachineMemoryTopology::default() + }; + + let observation = crate::topology::observe( + &topology, + 2, + 1, + Some(0), + crate::topology::BracketOutcome::HeldStill, + ); + + assert_eq!( + observation.overlapping_walk_relations, 2, + "both nodes are counted -- neither is the one that is right" + ); + assert_eq!( + observation.highest_numa_node, + Some(0), + "and the counter comparison is satisfied, which is why it cannot catch this" + ); + + let check = observation.cross_check(); + assert!( + check.disagreements.is_empty(), + "the counter agreed, so nothing is filed against the parse: {check:?}" + ); + assert!( + check + .parse_incomplete + .iter() + .any(|c| c.contains("share a processor with another")), + "{check:?}" + ); + assert!(check.parse_in_doubt(), "{check:?}"); +} + +#[test] +fn a_conflict_count_says_what_it_counted_rather_than_which_source_said_it() { + // Both predicates look for more than one DISTINCT VALUE and neither groups + // the claims by the API that issued them, so "the two sources disagreed" is + // more than either establishes. It is the usual cause on the path `measure` + // takes, and it is not what was checked. + for (label, mutate) in [ + ( + "distinct node number", + Box::new(|o: &mut crate::topology::Observation| { + o.numa_domains_with_conflicting_labels = 1; + }) as Box, + ), + ( + "distinct attribute value", + Box::new(|o: &mut crate::topology::Observation| { + o.processor_attribute_conflicts = 1; + }), + ), + ] { + let mut observation = agreeing_observation(); + mutate(&mut observation); + + let check = observation.cross_check(); + let entries = check.parse_incomplete.join(" "); + assert!( + entries.contains("more than one distinct"), + "{label}: states what it counted: {check:?}" + ); + assert!( + !entries.contains("the two sources") && !entries.contains("from each source"), + "{label}: and does not name a source split it never made: {check:?}" + ); + } +} + +#[test] +fn a_single_domain_at_a_level_is_reported_as_a_finding_not_as_hardware() { + // "No cache boundary divides the work" is a claim about the silicon. A + // level whose one domain covers half the online processors lands in the + // same arm, and nothing here checks that a level's domains cover the + // machine -- so the sentence states what this report found instead. + let mut observation = clean_observation(); + observation.online_processors = 4; + observation.partitioning_cache_level = None; + observation.caches = vec![crate::topology::CacheLevel { + level: 2, + processors_per_domain: vec![2], + }]; + + let text = crate::topology_report::report(BANNER, &observation); + assert!( + text.contains("nothing here divides"), + "the finding is about this report: {text}" + ); + assert!( + !text.contains("no cache boundary"), + "not about the machine, whose cache coverage this run never checked: {text}" + ); +} + +#[test] +fn a_bracket_outcome_reaches_the_observation_the_caller_passed_it() { + // `observe` takes the bracket as an ARGUMENT and stores what the caller + // passed. The field's doc said `observe` "leaves it `false`" -- true while + // the field was a `bool`, and left behind by the change to `BracketOutcome`, + // so it named a value the type no longer has for a parameter the function + // now takes. Pinned so the doc and the signature cannot drift apart again. + use windows_topology_sys::{ + Domain, DomainKind, MachineMemoryTopology, Observation, Processor, ProcessorId, Provenance, + Source, + }; + + let walk = || vec![Observation::new(Source::RelationshipWalk, 0)]; + let topology = MachineMemoryTopology { + processors: vec![Processor { + id: ProcessorId { + group: 0, + number: 0, + }, + online: true, + capacity: 0, + }], + domains: vec![ + Domain { + kind: DomainKind::Group, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: DomainKind::Package, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: walk(), + }, + Domain { + kind: DomainKind::Core { + simultaneous_multithreading: false, + efficiency_class: 0, + }, + processors: [(0u16, 0u8)].into_iter().collect(), + observations: walk(), + }, + ], + provenance: Provenance::Measured, + ..MachineMemoryTopology::default() + }; + + for outcome in [ + crate::topology::BracketOutcome::HeldStill, + crate::topology::BracketOutcome::Changed, + crate::topology::BracketOutcome::NotEstablished, + ] { + let observation = crate::topology::observe(&topology, 1, 1, None, outcome); + assert_eq!( + observation.bracket, outcome, + "observe stores the caller's outcome rather than deciding one" + ); + } +} diff --git a/crates/windows-platform-probes/src/topology.rs b/crates/windows-platform-probes/src/topology.rs new file mode 100644 index 000000000..b9ca0744a --- /dev/null +++ b/crates/windows-platform-probes/src/topology.rs @@ -0,0 +1,1430 @@ +// Copyright (c) Mike Grier. + +//! What shape is the machine, and which cache level actually partitions it? +//! +//! **An experiment, not a component.** These probes measure platform behaviour +//! and are not for production use. Do not call them from production code, and +//! do not lift a technique out of here. See this crate's DESIGN-NOTES.md. +//! +//! # Why this is a probe rather than a test +//! +//! Almost every number here is host-specific, so there is nothing to assert +//! about its *value* -- only about its internal consistency. That is the +//! binary-plus-asserted split this crate is built around: the binary prints the +//! shape for whoever is reading, and the tests pin the invariants that must hold +//! on any machine, so a parsing regression fails the build even though a core +//! count cannot. +//! +//! Running it in CI is the point. Hosted runners are a heterogeneous fleet, so +//! printing the discovered shape on every build turns ordinary CI into a slow +//! survey of what real machines look like -- including the negative result that +//! cloud runners are consistently single-node, which is itself evidence for how +//! the [uniform tunable architecture](../../../design-sessions/DESIGN-SESSION-2026-08-30-numa-sharded-io-execution-domains.md) +//! should size itself by default. +//! +//! # It measures the shipping crate, deliberately +//! +//! The parse comes from [`windows_topology_sys::MachineMemoryTopology::discover`] rather +//! than from a reimplementation here, for the same reason the pool-growth probe +//! uses the real thread-pool crate: a reimplementation would measure the +//! reimplementation. The raw counters below are then read *independently* +//! through Win32 and compared against it, so this probe doubles as a +//! cross-check on that crate's parsing across every machine CI ever runs on. + +use std::io; + +use windows_sys::Win32::System::Threading::{ + ALL_PROCESSOR_GROUPS, GetActiveProcessorCount, GetActiveProcessorGroupCount, + GetNumaHighestNodeNumber, +}; + +use windows_topology_sys::{ + Coherence, DomainKind, EnumerationAnomaly, MachineMemoryTopology, ProcessorSet, Provenance, + Source, +}; + +/// One cache level, summarised across the machine. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CacheLevel { + /// 1, 2, 3, ... as the firmware reports it. + pub level: u8, + /// Processors per partition, in discovery order. + pub processors_per_domain: Vec, +} + +impl CacheLevel { + /// How many distinct processor *partitions* exist at this level. + /// + /// Not the number of caches: a level Windows reports once per cache -- L1 + /// as separate `data` and `instruction` domains over the same processors -- + /// is several relationships but one partition per processor set, and it is + /// the partition a caller dividing work cares about. + /// + /// **Derived, not stored.** This was a `domains: usize` field that + /// `measure` always filled with `processors_per_domain.len()`, which made + /// two tests assert one expression against itself -- they read as a check + /// that the count matches the spans, and could not fail. Deleting them + /// would have removed the dead assertions; deriving the count removes the + /// disagreement they were written to catch, so no future test can restate + /// it either. + #[must_use] + pub fn domains(&self) -> usize { + self.processors_per_domain.len() + } +} + +/// One core, summarised. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct CoreShape { + /// Whether this core carries more than one logical processor. + pub simultaneous_multithreading: bool, + /// The firmware's performance ranking for this core. More than one distinct + /// value across the machine means heterogeneous cores, and therefore that + /// an unconstrained thread can be scheduled onto a slow one. + pub efficiency_class: u8, + /// Logical processors this core covers. + pub processors: usize, +} + +impl CoreShape { + /// Whether this record contradicts itself. + /// + /// `windows-topology-sys` documents `simultaneous_multithreading` as + /// "whether this core has more than one logical processor" -- a fact about + /// the membership recorded beside it, not a capability the hardware might + /// hold while only one sibling is online. So the two can be checked against + /// each other, and a record where they disagree describes no machine. + /// + /// A method rather than an expression written twice: the asserted live-host + /// test held the parse to this and [`Observation::cross_check`] did not, so + /// the probe could print that every check it could make matched on a host + /// whose own test had just gone red. Both now call this. + #[must_use] + pub fn contradicts_itself(&self) -> bool { + self.simultaneous_multithreading != (self.processors > 1) + } +} + +/// The machine's shape, as the shipping topology crate sees it, plus the raw +/// counters read independently for cross-checking. +#[derive(Debug, Clone)] +pub struct Observation { + // --- read through windows-topology-sys --- + /// Logical processors reported as online. + pub online_processors: usize, + /// Processor groups. More than one is a hard affinity boundary: a thread's + /// affinity names exactly one group, so above 64 logical processors the + /// partition is forced whether or not it is wanted. + pub groups: usize, + /// NUMA domains, including any that report no processors. + pub numa_domains: usize, + /// NUMA domains that report no processors at all -- ordinary on machines + /// with CXL expanders or HBM tiers, and the reason a domain count cannot be + /// used as a thread count. + /// + /// Named for what it counts. This was `memoryless_numa_domains` until a + /// review round caught that the name says the opposite of the measurement: + /// a *memoryless* node is the established term for one with processors and + /// no memory, whereas the filter here is `processors.is_empty()` and the + /// documented examples -- CXL expanders, HBM tiers -- are memory with no + /// processors. The arithmetic was right and only the label was inverted, + /// which is the dangerous shape: nothing misbehaved, so only a reader + /// aggregating the JSON key across a fleet would have been misled, and + /// about the reverse population. + pub numa_domains_without_processors: usize, + /// NUMA domains the relationship walk never described, which only CPU Sets + /// reported. + /// + /// A different question from [`Self::coherence`], which compares PROCESSOR + /// SETS: two sources can name exactly the same processors and still group + /// them into nodes differently, and that disagreement lands here as a + /// domain only one source reported rather than as a coherence failure. No + /// counter reaches it either -- the node totals can match while the + /// membership does not -- so if the probe does not carry it, nothing does. + pub numa_domains_only_in_cpu_sets: usize, + /// NUMA domains no source reported at all. + /// + /// Separate from [`Self::numa_domains_only_in_cpu_sets`] because that + /// count's message names CPU Sets as the reporter, which would be false + /// here. Unreachable through [`measure`] and reachable through the public + /// [`observe`]; it blocks agreement rather than being ignored, because a + /// domain nobody described still raises [`Self::numa_domains`] while + /// contributing no node number to compare. + pub numa_domains_unreported: usize, + /// NUMA domains carrying more than one distinct node number. + /// + /// A contradiction, not an alias: node numbers are machine-wide, so one + /// domain cannot honestly be both. The crate keeps every label on purpose + /// (D-15); taking the maximum to compare against the counter would resolve + /// the disagreement without reporting it. + /// + /// Counted over DISTINCT VALUES, and named for that rather than for "the + /// two sources disagreed" -- which is the usual cause on the path + /// [`measure`] takes, but is not what the count establishes. Nothing here + /// groups the labels by which API issued them, so a message naming the two + /// sources would assert more than was checked. + pub numa_domains_with_conflicting_labels: usize, + /// Whether the topology came from enumerating this machine. + /// + /// `MachineMemoryTopology` defaults to [`Provenance::Synthetic`] and + /// deserialization downgrades to it, precisely so a topology nobody + /// measured cannot pass for one that was. [`observe`] is public and takes + /// any topology, so without this a hand-built or restored one with matching + /// counters reached `Agree` -- certifying consistency with a machine no + /// enumeration ever read. + pub topology_was_measured: bool, + /// Cores the relationship walk never described, which only CPU Sets + /// reported. + /// + /// The core-level twin of [`Self::numa_domains_only_in_cpu_sets`]: two + /// sources that group the same processors into cores differently leave both + /// groupings in [`Self::cores`], so the count is inflated and `by-core` + /// oversizes. + pub cores_only_in_cpu_sets: usize, + /// Cores that cover no processors, and packages likewise. + /// + /// The empty-record twins of [`Self::numa_domains_without_processors`]. A + /// zero affinity mask raises no anomaly, so a spurious empty record inflates + /// the count and the policy derived from it while nothing else notices. + pub cores_without_processors: usize, + /// Packages that cover no processors. See [`Self::cores_without_processors`]. + pub packages_without_processors: usize, + /// Relations the relationship walk reported that share a processor with + /// another walk-reported relation of the same kind. + /// + /// A logical processor belongs to exactly one physical package and exactly + /// one core, so two walk records covering it cannot both describe this + /// machine. Nothing else here reaches that: no raw counter measures + /// packages or cores, an overlapping record raises no enumeration anomaly, + /// and both records are non-empty, so the `*_without_processors` counts stay + /// clear while [`Self::packages`] and `by-core` silently hold the duplicate. + /// + /// Restricted to the WALK. Two *sources* grouping the same processors + /// differently is a state `windows-topology-sys` keeps deliberately, and + /// [`Self::cores_only_in_cpu_sets`] is the count that reports it; including + /// it here would report one disagreement twice. + pub overlapping_walk_relations: usize, + /// Relations a caller described rather than any platform API reporting them. + /// + /// Distinct from [`Self::topology_was_measured`], which is object-level: + /// `Provenance::Measured` permits hand-inserted relations, and + /// `Source::Description` is the crate's marker for exactly that mixed case. + /// So a measured topology can still carry counted relations nobody read + /// from the platform. + pub described_relations: usize, + /// Relations no source reported at all, of any kind. + /// + /// Counted for every kind, not just memory domains: the crate documents an + /// empty observation list as the honest state for a relation built by hand, + /// and such a core, package or cache is still counted here and can still + /// change a policy or the cache partitioning. + pub unreported_relations: usize, + /// Per-processor attributes carrying more than one distinct value. + /// + /// The crate preserves these separately from the relations + /// (`attribute_conflicts`) because they have no membership to compare -- + /// efficiency class is the case, reported per core by the walk and per + /// processor by CPU Sets. [`Self::cores`] carries the walk's value alone, + /// so without this the report could call a machine homogeneous while the + /// other source disagreed. + /// + /// Named for distinct VALUES, not for "the two sources disagreed": + /// `attribute_conflicts` groups claims by subject and keeps those with more + /// than one distinct value, without grouping by the source that issued + /// them. The two-source case is the usual cause and is not what the count + /// establishes. + pub processor_attribute_conflicts: usize, + /// The largest NUMA node number the topology crate reported, or `None` when + /// no memory domain carried a label from either source. + /// + /// Not "when it reported no memory domain at all", which is what this said + /// and is a stronger claim than the code makes: a memory domain with an + /// empty `observations` list -- which `windows-topology-sys` documents as + /// the honest state for "a relation nobody reported" -- raises + /// [`Self::numa_domains`] while contributing no label here. Unreachable + /// through [`measure`], since every domain `discover` builds carries at + /// least one observation, but [`observe`] is public and takes any topology. + /// + /// Kept beside the count because the two answer different questions and + /// Windows only promises the second one: node numbers are not guaranteed + /// dense, so a machine with nodes 0 and 2 has a count of two and a highest + /// of two. Comparing the count against `GetNumaHighestNodeNumber` would + /// call that correct machine a parsing regression. + pub highest_numa_node: Option, + /// Physical packages (sockets). + pub packages: usize, + /// Every physical core. + pub cores: Vec, + /// Cache levels, ascending, each summarised across the machine. + pub caches: Vec, + /// Which of [`Self::caches`] the topology crate named as the outermost + /// level that splits the machine into more than one domain, if any. + /// + /// Captured from `MachineMemoryTopology::outermost_partitioning_cache` + /// rather than derived from the summaries above, so the rule has one + /// implementation (`SH-16.9`). `None` is a real answer and not a failure -- + /// but it is "no level was NAMED", which is not the same as "no level + /// divides this machine", and this said the latter. The crate also answers + /// `None` when two levels partition the machine incomparably, and when its + /// candidate filter left nothing at all. [`Self::partitioning_cache`] is + /// what tells those apart; read it rather than reading a cause into this. + pub partitioning_cache_level: Option, + /// What the topology crate could not fully decode of what Windows returned. + /// + /// **Not all of them are dropped records**, which is the trap: an + /// `Undersized` or `OverrunsBuffer` record decodes to nothing and leaves + /// the counts above short, but a `TruncatedArray` record is *kept* -- + /// `windows-topology-sys` says so directly, "the entries that did fit are + /// decoded and kept; this records that the count claimed more". A cache + /// record kept with a partial affinity mask presents a processor set that + /// is smaller than the truth and therefore DISTINCT from it, which + /// `cache_partitions_at_level` counts as its own partition. So an anomaly + /// can leave a count short or overstated, and this observation must not + /// claim to know which. + /// + /// Carried because `discover` returns `Ok` when it hits one: the record is + /// recorded here and the parse continues, so nothing else in this + /// observation is sensitive to it. `windows-topology-sys` states the + /// consequence directly -- dropping the list "would leave a consumer + /// unable to tell a truncated enumeration from a small machine" -- and a + /// probe whose whole job is to say what the run established is exactly + /// such a consumer. + pub enumeration_anomalies: Vec, + /// Whether the topology crate's two Win32 sources described the same + /// machine. + /// + /// Also `Ok` when they did not: [`Coherence::Disagreed`] is the crate's + /// *conclusion* that the disagreement is real rather than transient, and + /// it names processors that CPU Sets saw and the relationship walk did not + /// -- processors deliberately absent from the parsed list, so no count + /// derived from that list can reveal them. + pub coherence: Coherence, + + // --- read independently through Win32 --- + /// `GetActiveProcessorCount(ALL_PROCESSOR_GROUPS)`. + pub raw_active_processors: u32, + /// `GetActiveProcessorGroupCount()`. + pub raw_group_count: u16, + /// `GetNumaHighestNodeNumber()`, or `None` if the call failed. + pub raw_highest_numa_node: Option, + /// Whether the counters moved across the parse, so the two readings + /// describe different instants and cannot be compared. + /// + /// Bears on the COMPARISON only. The topology is still a valid snapshot of + /// the machine as it was, so the parse-side findings stand on their own and + /// `cross_check` evaluates them regardless. + /// + /// [`measure`] computes this from its two counter readings; [`observe`] + /// takes it as an argument and stores what the caller passed, because only + /// the caller knows what produced its counters. This said `observe` "leaves + /// it `false`", which was true while the field was a `bool` and survived + /// the change to [`BracketOutcome`] -- naming a value the type no longer + /// has, for a parameter the function now takes. + pub bracket: BracketOutcome, +} + +impl Observation { + /// The outermost cache level that actually splits the machine into more + /// than one domain, if any. + /// + /// **Asked of `windows-topology-sys`, not re-derived here.** This method + /// used to restate the rule as "the highest level with more than one + /// domain", and by the time `M4+.4` landed that restatement differed from + /// the crate's own answer in two ways: it omitted the pairwise-disjointness + /// check, so a hand-built topology with overlapping blocks would have been + /// accepted, and it ordered candidates by **level number**, which the + /// topology crate stopped doing because a higher number is not always + /// coarser -- the ARM64 machine with no L3 is the standing counterexample. + /// + /// So the level is now captured at survey time from + /// `MachineMemoryTopology::outermost_partitioning_cache`, and this method + /// only looks up the summary for it. `SH-16.9` records this rule going + /// wrong three times in two crates; there is now one implementation. + #[must_use] + pub fn outermost_partitioning_cache(&self) -> Option<&CacheLevel> { + let level = self.partitioning_cache_level?; + self.caches.iter().find(|c| c.level == level) + } + + /// The same answer as [`Self::outermost_partitioning_cache`], with the + /// `None` cases told apart. + /// + /// `None` is several different findings wearing one face, and which one it is + /// changes what a reader may conclude about the machine. The prose report + /// has always refused to conflate them -- "naming only the first turns a + /// reported ambiguity into a false claim about the hardware" -- but the + /// NDJSON emitted `"outermost_partitioning_cache_level":null` for all of + /// them, on a line the verdict had already certified as `agree`, because an + /// incomparable partitioning touches nothing `cross_check` consults. A + /// fleet query counting nulls as "machines no cache level partitions" -- + /// the natural reading, and the one this crate's own no-L3 story invites -- + /// silently folded in the others. + /// + /// What separates them is data the probe already holds, and reading it is + /// not the re-derivation `SH-16.9` forbids: the crate is still the only + /// thing that decides WHICH level is outermost. This only asks whether any + /// level partitions at all, to classify an answer the crate already gave. + #[must_use] + pub fn partitioning_cache(&self) -> PartitioningCache<'_> { + let Some(level) = self.partitioning_cache_level else { + // Deliberately not "incomparable". The crate reaches `None` both + // when two maximal candidates are not the same partition and when + // its candidate filter left nothing at all -- a partitioning level + // rejected for overlapping blocks lands in the second. This probe + // cannot tell those apart and does not guess. + // Checked BEFORE the others, because both of them read as facts + // about the hardware and an empty list supports neither. `any()` on + // it is vacuously false, so it fell into "no level partitions this + // machine" -- a claim about cache structure drawn from a survey that + // reported no cache structure at all. + return if self.caches.is_empty() { + PartitioningCache::NoLevelsReported + } else if self.caches.iter().any(|c| c.domains() > 1) { + PartitioningCache::NoUniqueOutermost + } else { + PartitioningCache::NoLevelPartitions + }; + }; + self.caches.iter().find(|c| c.level == level).map_or( + PartitioningCache::SummaryMissing(level), + PartitioningCache::Level, + ) + } + + /// How many execution domains each candidate policy would produce. + /// + /// Reported rather than recommended. The point of printing all of them is + /// that they disagree, and the disagreement is the finding. + #[must_use] + pub fn domain_counts(&self) -> Vec<(&'static str, usize)> { + vec![ + ("single", 1), + // Every count below is clamped to one, and the clamp is the + // contract rather than defensiveness: there is always at least one + // execution domain, because the machine exists, and a fleet sized + // at zero domains performs no I/O at all. + // + // Zero is reachable for each of these, which is why none of them is + // passed through raw. A `PROCESSOR_RELATIONSHIP` record whose body + // is shorter than its declared size decodes to nothing and is + // dropped as an enumeration anomaly, while `discover` still returns + // `Ok` -- so a topology with no package or core relationships is a + // legal, non-error result rather than a parse failure. + // + // "Every count below" is meant literally, and was not: an earlier + // version clamped `packages` and `cores` while leaving the cache + // arm's `c.domains` raw, because the `1` there is only the `None` + // default and reads like a clamp without being one. `Observation`'s + // fields are public and `domain_counts` is `pub`, so a cache summary + // reporting zero domains is constructible even though `measure` + // cannot produce one -- and this comment asserted otherwise. + ("by-package", self.packages.max(1)), + ( + "by-numa-domain-with-processors", + // Saturating as well as clamped. Not because the two counts can + // diverge in a measured run -- they come from one loop over one + // enumeration, so the second is a subset of the first -- but + // because `Observation`'s fields and `domain_counts` are both + // public, so a caller can hand this an out-of-range pair, and + // an unsigned subtraction would panic rather than report. + self.numa_domains + .saturating_sub(self.numa_domains_without_processors) + .max(1), + ), + ( + "by-outermost-partitioning-cache", + // `.max(1)` on the SELECTED value, not just the `None` default: + // the default covers "no level was chosen", and this covers "a + // level was chosen whose summary reports no domains". + self.outermost_partitioning_cache() + .map_or(1, |c| c.domains().max(1)), + ), + ("by-core", self.cores.len().max(1)), + ] + } + + /// Whether these counts include relations no platform API reported. + /// + /// A single named predicate that argues its own membership, rather than a + /// condition restated where it is used. Each member is a way for a count + /// here to have come from somewhere other than a parse of this machine: + /// + /// - not measured at all, so no enumeration produced any of it; + /// - a relation a caller *described*, which `Provenance::Measured` permits + /// on an otherwise measured topology and `Source::Description` marks; + /// - a relation carrying no observation from any source, which the owning + /// crate documents as the honest state for one built by hand. + /// + /// The three are one idea -- the counts are not wholly a parse -- which is + /// why they are named once here rather than re-tested at the comparison. + /// Being conservative costs nothing on the path this probe actually takes: + /// `discover` gives every relation at least one platform source, so + /// [`measure`] never reaches this. Only the public [`observe`] can. + #[must_use] + pub fn counts_include_unparsed_relations(&self) -> bool { + !self.topology_was_measured || self.described_relations > 0 || self.unreported_relations > 0 + } + + /// Whether the independently-read Win32 counters agree with what the + /// topology crate parsed. + /// + /// A disagreement is a real finding: it means the shipping crate's parse of + /// `GetLogicalProcessorInformationEx` diverges from what the simple + /// counters report on this machine. + /// + /// # Why a counter that could not be read is not a disagreement + /// + /// The two outcomes mean different things and belong to different owners. A + /// disagreement is a statement about the shipping crate; a counter that + /// could not be read is a statement about this measurement, and reporting + /// the second as the first would send a reader to audit a parse that was + /// never contradicted. + /// + /// Collapsing them also produced the failure this whole probe exists to + /// avoid. An earlier version skipped the NUMA comparison when + /// `GetNumaHighestNodeNumber` failed, contributed no entry, and so let the + /// renderer print its agreement line directly below + /// "GetNumaHighestNodeNumber : failed" -- claiming an agreement the run had + /// not established, in the report whose entire job is to separate what was + /// measured from what was assumed. + /// + /// # Why the crate's own report is consulted before the counters + /// + /// The three counters are scalars, and none of them is sensitive to a + /// dropped `PROCESSOR_RELATIONSHIP` record or to a processor that only CPU + /// Sets saw. So a parse that `windows-topology-sys` itself reported as + /// incomplete could satisfy all three and reach `Agree` -- the same shape + /// as the NUMA defect above, and worse, because the evidence was already + /// in hand rather than needing a fourth call to go and get. + /// + /// The rule is therefore fiat and simple: **`Agree` requires all three + /// lists empty**, so anything this method pushes blocks it, whether or not + /// a counter noticed. Stated over the LISTS and not over their causes, + /// deliberately -- the causes are enumerated in exactly one place, the body + /// below, so adding a fourth needs no prose anywhere to be corrected. An + /// earlier wording named the two causes that existed at the time ("an empty + /// anomaly list and `Coherence::Agreed`"), a third was added without the + /// sweep, and the stale pair was then transcribed into DESIGN-NOTES.md as + /// settled fiat -- inviting a future reader to delete the undocumented + /// branch to make the code match its own rule. + /// + /// **The causes are not listed here, and deliberately are not.** The + /// paragraph above once ended with "as of now the body pushes for ..." + /// naming the three that existed; a fourth was added a commit later without + /// the sweep, so the very doc diagnosing that rot had rotted the same way + /// inside two rounds. A list of causes kept beside the rule is not a + /// summary of the body, it is a second copy of it that nothing checks. Read + /// the body. + /// + /// The match on coherence is exhaustive so that a variant added later is a + /// compile error here rather than a silent new path to agreement. + #[must_use] + pub fn cross_check(&self) -> CrossCheck { + let mut check = CrossCheck::default(); + + if !self.enumeration_anomalies.is_empty() { + check.parse_incomplete.push(format!( + // States the CONDITION, not a direction. "Records were dropped + // and the counts are short" was true of the two anomaly kinds + // that decode to nothing and false of `TruncatedArray`, which + // keeps the record with the entries that fit -- and a cache + // record kept with a partial affinity mask can INFLATE a + // partition count rather than shorten it, so the old message + // pointed a reader the wrong way. `AnomalyKind` is + // `#[non_exhaustive]`, so classifying here would need a + // catch-all arm that a future variant falls into silently; + // saying only what is true of all of them cannot rot that way. + "windows-topology-sys recorded {} enumeration anomal{}, so what Windows returned \ + was not fully decoded and the counts above may be short, or overstated where a \ + record was kept with an incomplete processor set", + self.enumeration_anomalies.len(), + if self.enumeration_anomalies.len() == 1 { + "y" + } else { + "ies" + }, + )); + } + + if self.numa_domains_only_in_cpu_sets > 0 { + check.parse_incomplete.push(format!( + "{} NUMA domain(s) were reported only by CPU Sets and never by the relationship \ + walk, so the two sources group nodes differently -- which no counter and no \ + coherence check reaches, since coherence compares processor sets", + self.numa_domains_only_in_cpu_sets, + )); + } + + // No cache levels at all. Distinct from the per-level case below, and + // NOT reachable the same way: an empty affinity mask still leaves the + // level present, because `cache_levels` filters on kind. This is the + // survey reporting no cache relationships whatsoever, so there is no + // record that failed to decode, nothing raises an anomaly, and every + // conclusion the report draws about cache structure would be drawn from + // nothing. + if self.caches.is_empty() { + check.parse_incomplete.push( + "no cache levels were reported at all, so what divides this machine by cache \ + was not established in either direction" + .to_string(), + ); + } + + // A level the survey DOES carry, with no partitions at all. + // `cache_levels` filters on kind while `cache_partitions_at_level` + // drops domains covering no processors, so a cache record whose + // affinity mask is empty yields exactly this -- and it raises no + // anomaly, because `read_cache_body` reports only a declared-versus-read + // count mismatch and never inspects mask contents. So nothing else here + // is sensitive to it, and without this the report printed + // "L3 0 domain(s)" a few lines above "no cache level partitions this + // machine", with the verdict certifying the pair as `agree`. + let empty_levels: Vec = self + .caches + .iter() + .filter(|c| c.domains() == 0) + .map(|c| c.level) + .collect(); + if !empty_levels.is_empty() { + check.parse_incomplete.push(format!( + "cache level(s) {empty_levels:?} decoded to no partitions at all, so what \ + divides this machine at those levels was not established" + )); + } + + // A running machine has processors and groups whatever the enumeration + // said, so a MEASURED topology reporting none of either did not describe + // its host. Nothing below reaches this: both raw counters report failure + // as zero, so a zero parse beside a failed read is filed as + // `not_compared` -- leaving `parse_in_doubt` false, and the report free + // to state an impossible machine without a caveat. + // + // Stated over the LIST rather than once per count, so a third such count + // joins the array instead of needing its own rule to be remembered. + let absent: Vec<&str> = [ + ("online processors", self.online_processors), + ("processor groups", self.groups), + ] + .into_iter() + .filter(|(_, count)| *count == 0) + .map(|(name, _)| name) + .collect(); + if self.topology_was_measured && !absent.is_empty() { + check.parse_incomplete.push(format!( + "this topology was measured from a running machine, which cannot have none, but \ + it reported no {}", + absent.join(" and "), + )); + } + + // The machine has packages and cores whatever the enumeration said, so + // reporting none of either means the enumeration did not describe them. + // Treated exactly as an empty cache survey is, and for the same reason: + // no record needs to have FAILED for this to happen -- Windows can + // simply not report the relationship, in which case no anomaly fires + // and nothing else here notices. + if self.online_processors > 0 && self.packages == 0 { + check + .parse_incomplete + .push("no packages were reported at all, though the machine has one".to_string()); + } + if self.online_processors > 0 && self.cores.is_empty() { + check + .parse_incomplete + .push("no cores were reported at all, though the machine has one".to_string()); + } + + // A record that contradicts ITSELF, which no counter reaches: nothing + // independent measures cores or caches, and neither shape raises an + // enumeration anomaly. Both were already invariants the asserted + // live-host tests hold the parse to, and `cross_check` did not -- so on + // a host where one of those tests went red, this said every check it + // could make had matched. CI now prints the report even when the tests + // fail, which is exactly when that sentence gets read. + let contradictory_cores = self + .cores + .iter() + .filter(|core| core.contradicts_itself()) + .count(); + if contradictory_cores > 0 { + check.parse_incomplete.push(format!( + "{contradictory_cores} core(s) report an SMT flag that disagrees with the number \ + of processors recorded beside it, so the record contradicts itself" + )); + } + + // Windows numbers cache levels from 1, so a level of 0 is a level the + // parse did not read rather than one the machine has. + let unnumbered_levels = self.caches.iter().filter(|c| c.level == 0).count(); + if unnumbered_levels > 0 { + check.parse_incomplete.push(format!( + "{unnumbered_levels} cache level(s) are numbered 0, which is not a level Windows \ + reports, so what they describe was not established" + )); + } + + if !self.topology_was_measured { + check.parse_incomplete.push( + "this topology was not measured from a running machine, so nothing here \ + describes the host it is reported on" + .to_string(), + ); + } + + if self.cores_without_processors > 0 || self.packages_without_processors > 0 { + check.parse_incomplete.push(format!( + "{} core(s) and {} package(s) cover no processors, so they raise those counts \ + and the policies derived from them without describing any part of the machine", + self.cores_without_processors, self.packages_without_processors, + )); + } + + if self.unreported_relations > 0 { + check.parse_incomplete.push(format!( + "{} relation(s) carry no observation from any source, so they are counted here \ + without any platform API having described them", + self.unreported_relations, + )); + } + + if self.described_relations > 0 { + check.parse_incomplete.push(format!( + "{} relation(s) were described by a caller rather than reported by any platform \ + API, so the counts above are not all of them measured", + self.described_relations, + )); + } + + if self.cores_only_in_cpu_sets > 0 { + check.parse_incomplete.push(format!( + "{} core(s) were reported only by CPU Sets and never by the relationship walk, so \ + the two group processors into cores differently and the core count above holds \ + both groupings", + self.cores_only_in_cpu_sets, + )); + } + + if self.overlapping_walk_relations > 0 { + check.parse_incomplete.push(format!( + "{} relation(s) reported by the relationship walk share a processor with another \ + of the same kind, so one processor is claimed by two packages, two cores or two \ + NUMA nodes and the counts above hold both", + self.overlapping_walk_relations, + )); + } + if self.processor_attribute_conflicts > 0 { + check.parse_incomplete.push(format!( + "{} per-processor attribute(s) carry more than one distinct value, so the \ + efficiency classes above are one claim rather than an agreed one", + self.processor_attribute_conflicts, + )); + } + + if self.numa_domains_with_conflicting_labels > 0 { + check.parse_incomplete.push(format!( + "{} NUMA domain(s) carry more than one distinct node number, so what node they \ + are was not established and the highest below takes the larger", + self.numa_domains_with_conflicting_labels, + )); + } + + if self.numa_domains_unreported > 0 { + check.parse_incomplete.push(format!( + "{} NUMA domain(s) carry no observation from either source, so they raise the \ + domain count while contributing no node number to compare", + self.numa_domains_unreported, + )); + } + + match &self.coherence { + Coherence::Agreed => {} + Coherence::Disagreed { + walk_only, + cpu_sets_only, + attempts, + } => check.parse_incomplete.push(format!( + "windows-topology-sys reports its two enumerations never agreed within {attempts} \ + attempt(s): {} processor(s) seen only by the relationship walk, {} seen only by \ + CPU Sets and so absent from the parsed list entirely", + walk_only.len(), + cpu_sets_only.len(), + )), + // Unreachable from `discover`, which returns `Agreed` or + // `Disagreed`. Reported rather than ignored because reaching it + // would mean the parse came from somewhere that read nothing + // twice, and a cross-check cannot certify that either. + Coherence::NotCollected => check.parse_incomplete.push( + "windows-topology-sys reports its coherence was never collected, so nothing \ + established that its two enumerations describe the same machine" + .to_string(), + ), + } + + // Everything above is about the PARSE and is evaluated unconditionally. + // Only the counter comparisons below are skipped when the machine moved + // under the run, because only they compare two readings. + // + // This was an early return, which suppressed every check above it: a + // host that hot-added a processor AND dropped a record reported + // `parse_incomplete: 0`, so `parse_in_doubt` was false, the renderer + // printed "this machine reports no L3 at all" as an uncaveated hardware + // fact, and the dropped record appeared nowhere in the report at all -- + // the exact failure this probe exists to prevent, introduced by the fix + // for the timing skew. + // Matched exhaustively, so a fourth outcome cannot silently become a + // reason to compare. + match self.bracket { + BracketOutcome::HeldStill => {} + BracketOutcome::Changed => { + check.not_compared.push( + "the machine changed while this ran -- the counters moved across the parse, \ + so the two readings describe different instants" + .to_string(), + ); + return check; + } + BracketOutcome::NotEstablished => { + check.not_compared.push( + "the bracket around the parse was not closed -- a counter failed one of its \ + two reads, so nothing established that the machine held still" + .to_string(), + ); + return check; + } + } + + // A mismatch can only be filed against the PARSE while the counts ARE + // the parse. `observe` is public and takes any topology, so a caller's + // described relation -- or one nobody reported -- raises `groups`, or a + // NUMA label, that no platform API ever produced; the comparisons below + // then read that difference as the shipping crate contradicting Windows. + // Same inversion the NUMA labels once carried, and `verdict` gives + // `Disagree` precedence, so the report leads with an accusation this run + // did not establish while the reason sits further down the same page. + // + // Filed as `not_compared` rather than skipped silently: this is a + // reading that could not be trusted, which is exactly what that list is. + if self.counts_include_unparsed_relations() { + check.not_compared.push( + "these counts include relations no platform API reported, so a counter mismatch \ + could not be attributed to the parse" + .to_string(), + ); + return check; + } + + // Zero is how these two report failure, and no machine has zero active + // processors or zero groups, so zero is a failed read rather than a + // count to compare against. Treating it as a count reported a failed + // measurement as though the crate's parse were wrong. + if self.raw_active_processors == 0 { + check.not_compared.push( + "GetActiveProcessorCount returned 0, which is its failure report".to_string(), + ); + } else if self.online_processors != self.raw_active_processors as usize { + check.disagreements.push(format!( + "online processors: topology crate says {}, GetActiveProcessorCount says {}", + self.online_processors, self.raw_active_processors + )); + } + + if self.raw_group_count == 0 { + check.not_compared.push( + "GetActiveProcessorGroupCount returned 0, which is its failure report".to_string(), + ); + } else if self.groups != self.raw_group_count as usize { + check.disagreements.push(format!( + "groups: topology crate says {}, GetActiveProcessorGroupCount says {}", + self.groups, self.raw_group_count + )); + } + + let Some(highest) = self.raw_highest_numa_node else { + check.not_compared.push( + "GetNumaHighestNodeNumber failed, so no NUMA comparison was made".to_string(), + ); + return check; + }; + + if self.highest_numa_node != Some(highest) { + // Highest against highest, deliberately, and not a count against + // `highest + 1`. `GetNumaHighestNodeNumber` reports the largest node + // *number*, which Windows does not promise equals the node count -- + // nodes 0 and 2 are a valid sparse topology, and the count form + // would report a regression on hardware that is reporting itself + // correctly. + check.disagreements.push(format!( + "NUMA nodes: topology crate's highest node is {}, GetNumaHighestNodeNumber says {}", + self.highest_numa_node + .map_or_else(|| "none".to_string(), |n| n.to_string()), + highest + )); + } + check + } +} + +/// What this run established about the topology crate's parse. +/// +/// Three lists rather than one, because they are claims about three different +/// things and each has a different owner -- see [`Observation::cross_check`]. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub struct CrossCheck { + /// Counters that were compared and did not match. Each is a finding about + /// the shipping crate's parse. + pub disagreements: Vec, + /// Comparisons this run could not make, or could not trust. Each is a gap + /// in this measurement, not a finding about the parse. + /// + /// Stated over what the list MEANS rather than over what fills it. This + /// read "counters that could not be read", which named one cause and was + /// already false of two others -- a machine that changed under the run + /// (a bracket that closed on two different instants), and counts holding + /// relations no platform API reported. Both are comparisons that were not + /// made; neither is a counter that failed. The causes are enumerated in + /// exactly one place, [`Observation::cross_check`]'s body. + pub not_compared: Vec, + /// Ways the parse is short, or its claims mutually inconsistent, such that + /// agreeing counters cannot certify it. + /// + /// Neither of the other two: nothing this probe read was contradicted, and + /// nothing it wanted to read was missing. Established from the PARSE rather + /// than from any counter, which is why no counter agreeing can retire an + /// entry here. + /// + /// Not "things the crate reported about itself", which is how this read + /// while every entry happened to be a crate self-assessment. One is not: + /// a NUMA domain only CPU Sets described is derived here, from provenance + /// the crate carries but draws no conclusion about. + pub parse_incomplete: Vec, +} + +impl CrossCheck { + /// What this run is entitled to claim. + /// + /// `Agree` requires all three lists empty, so it means "everything this + /// probe could check was checked and matched, and nothing about the parse + /// itself left it unable to certify" -- which is what the rendered line + /// claims. Only [`Self::disagreements`] produces `Disagree`, because that + /// is the only list whose entries contradict the parse; an incomplete parse + /// is not a wrong one, and reporting it as a divergence would send a reader + /// to audit a mismatch that does not exist. + #[must_use] + pub fn verdict(&self) -> Verdict { + if !self.disagreements.is_empty() { + Verdict::Disagree + } else if self.not_compared.is_empty() && self.parse_incomplete.is_empty() { + Verdict::Agree + } else { + Verdict::Incomplete + } + } + + /// Whether anything here bears on the parsed topology describing this + /// machine -- and so on whether the counts derived from it may be read as + /// hardware facts. + /// + /// Two of the three lists, deliberately, and this is the one place that + /// says which. [`Self::disagreements`] belongs because a counter read + /// independently from Windows contradicting the walk is the *strongest* + /// available evidence that the parsed list is short: a crate reporting one + /// processor group where `GetActiveProcessorGroupCount` says two did not + /// merely miscount, it never saw the second group's records, including its + /// caches. [`Self::parse_incomplete`] belongs by construction. + /// + /// [`Self::not_compared`] is excluded, and that exclusion is a claim rather + /// than an oversight. Every entry there is a reading this probe could not + /// make or could not trust -- which bears on its ability to CHECK the parse + /// and on nothing in the parse itself. Caveating the cache conclusions + /// there would assert a doubt this run does not have, which is the same + /// defect as asserting a certainty it does not have. + /// + /// Stated over what the list means, not over what fills it. This said "its + /// only entries are the three Win32 counters failing to read", which two + /// later rounds falsified by adding the bracket outcomes -- and one of + /// those, a machine that CHANGED, is the opposite of a counter that failed + /// to read: every counter was read, twice, and they differed. + #[must_use] + pub fn parse_in_doubt(&self) -> bool { + !self.disagreements.is_empty() || !self.parse_incomplete.is_empty() + } +} + +/// Which answer [`Observation::partitioning_cache`] gave, with the ways there +/// can be no level named held apart from each other. +/// +/// Counted rather than enumerated, once, as "the three ways" -- and a fourth +/// arrived a round later. A count is a restatement of the variant list below +/// that nothing checks, so there is none. +/// +/// An enum rather than an `Option` for the reason [`Verdict`] is one rather +/// than a `bool`: a renderer or a serialiser cannot emit the absent case +/// without having decided which absent case it is. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PartitioningCache<'a> { + /// The crate named this level, and its summary is here. + Level(&'a CacheLevel), + /// The survey reported no cache levels at all. + /// + /// Not a fact about cache structure in either direction -- nothing was + /// measured. Its own variant because `any()` over an empty list is + /// vacuously false, so this used to reach [`Self::NoLevelPartitions`] and + /// print "no cache level reported more than one domain, so no cache + /// boundary divides the work": a conclusion about the hardware drawn from a + /// survey that found no hardware to draw it from. + NoLevelsReported, + /// At least one level was reported, none reports more than one domain, and + /// no level was named. + /// + /// Says what was COUNTED, and nothing about coverage. This read "every + /// level reported covers the whole machine, so no cache boundary divides + /// the work" -- a claim the probe holds no data for: a level whose one + /// domain covers half the online processors lands here too, and nothing + /// checks that a level's domains cover the machine. The renderer's sentence + /// was narrowed for exactly this reason and this copy was not swept with + /// it. + /// + /// A level whose records decoded to no partitions at all is also not more + /// than one, and `cross_check` pushes a `parse_incomplete` entry for that, + /// so the verdict moves off `Agree` and the renderer caveats it. Read the + /// cross-check before taking this for anything. + NoLevelPartitions, + /// No level was named, yet at least one level reports more than one + /// distinct domain. + /// + /// Named as "not the two cases above" and no further, because + /// `CacheLevel::domains` counts DISTINCT processor sets and the topology + /// crate is explicit that "distinct is not disjoint ... the result is a set + /// of domains rather than a proven partition". So this covers both two + /// levels partitioning the machine incomparably AND a level whose blocks + /// overlap, which `outermost_partitioning_cache` rejects and which + /// partitions nothing at all. Saying "at least one level does split the + /// machine" -- as this once did -- asserts the first and is contradicted by + /// the second. + NoUniqueOutermost, + /// The crate named this level and the survey has no summary for it. + /// + /// A defect in this probe rather than anything about the machine, and + /// `the_outermost_partitioning_cache_is_the_deepest_level_that_splits_the_machine` + /// asserts it cannot happen. It is a variant anyway so that the renderer + /// must handle it rather than folding it into an answer about the hardware, + /// which is how it would otherwise be printed as "no level partitions". + SummaryMissing(u8), +} + +/// The three things a cross-check can conclude. +/// +/// An enum rather than a `bool` so that a renderer cannot print "agree" without +/// having handled the third case: the compiler makes the incomplete arm +/// impossible to omit, which is what a stray `is_empty()` on one of the two +/// lists would have allowed. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Verdict { + /// Every counter was read, every one matched, and nothing about the parse + /// itself blocked certifying it. All three of [`CrossCheck`]'s lists empty. + Agree, + /// At least one counter was read and did not match. + Disagree, + /// Nothing disagreed, but something was missing or unestablished -- a + /// counter that could not be read, or the parse being short or disputed -- + /// so this run did not establish agreement. + /// + /// Stated over the lists rather than over their causes: this is every + /// entry in [`CrossCheck::not_compared`] and + /// [`CrossCheck::parse_incomplete`], whatever put it there. Naming the + /// causes here is what let this doc go stale when a third was added. + Incomplete, +} + +/// Discover the machine's shape. +/// +/// # Errors +/// +/// Propagates a failure from [`MachineMemoryTopology::discover`]. +pub fn measure() -> io::Result { + // Bracketed, because the parse and the counters are separate reads of a + // machine that can change between them: Windows supports processor hot-add, + // and a machine that gained one mid-run would have both readings correct + // for different instants and be reported as the crate parsing wrongly. + // Re-reading afterwards detects it, since the same hot-add moves the + // counters. + let before = read_counters(); + let topology = MachineMemoryTopology::discover()?; + let after = read_counters(); + + let observation = observe( + &topology, + after.0, + after.1, + after.2, + bracket_outcome(before, after), + ); + Ok(observation) +} + +/// Whether two bracketing counter reads establish that the machine changed. +/// +/// Extracted rather than inlined in [`measure`], for the reason [`observe`] is: +/// a stable host produces `before == after` with every read succeeding, so no +/// test driving `measure` can reach the interesting branches. Inlined, the test +/// for this predicate reproduced it verbatim and asserted against its own copy +/// -- a mutation sweep killed nothing across all six operators here. +/// +/// Only counters read successfully in BOTH brackets are evidence. Comparing the +/// failure sentinels as values made a counter that failed once and succeeded +/// once -- 0 then 4 -- read as "the machine changed while this ran", a claim +/// about the hardware nothing established. A read that failed is reported as a +/// failed read instead. +#[must_use] +pub fn bracket_outcome( + before: (u32, u16, Option), + after: (u32, u16, Option), +) -> BracketOutcome { + let changed = (before.0 != 0 && after.0 != 0 && before.0 != after.0) + || (before.1 != 0 && after.1 != 0 && before.1 != after.1) + || (before.2.is_some() && after.2.is_some() && before.2 != after.2); + if changed { + return BracketOutcome::Changed; + } + let closed = before.0 != 0 + && after.0 != 0 + && before.1 != 0 + && after.1 != 0 + && before.2.is_some() + && after.2.is_some(); + if closed { + BracketOutcome::HeldStill + } else { + BracketOutcome::NotEstablished + } +} + +/// What bracketing the parse with two counter reads established. +/// +/// One value rather than a `changed` flag beside a `held_still` flag, for the +/// reason [`Verdict`] is an enum rather than a bool: those two can be set to +/// contradict each other, and were -- "changed, and also held still" is +/// meaningless, and a test constructed it the moment both existed. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BracketOutcome { + /// Every counter was read twice and none moved. + HeldStill, + /// Two good reads of some counter differed: the machine changed under the + /// run, so the parse and the counters describe different instants. + Changed, + /// A counter failed one of its two reads, leaving the bracket open at one + /// end. + /// + /// Establishes NEITHER change nor stability, which is why it is not the + /// absence of [`Self::Changed`]. Comparing anyway would let a concurrent + /// hot-add be filed against the parse. + NotEstablished, +} + +/// The three counters, read machine-wide. +/// +/// # Safety +/// +/// The first two take no pointer arguments, so there is nothing to alias or +/// outlive; `ALL_PROCESSOR_GROUPS` is the documented way to ask for the +/// machine-wide count. `highest` is a live local for the duration of its call. +/// Failure is handled rather than dismissed: the first two report it by +/// returning 0, which `cross_check` treats as a read that did not happen. +fn read_counters() -> (u32, u16, Option) { + // SAFETY: as stated above. + let active = unsafe { GetActiveProcessorCount(ALL_PROCESSOR_GROUPS) }; + // SAFETY: as stated above. + let groups = unsafe { GetActiveProcessorGroupCount() }; + let mut highest = 0u32; + // SAFETY: as stated above. + let highest_node = if unsafe { GetNumaHighestNodeNumber(&raw mut highest) } != 0 { + Some(highest) + } else { + None + }; + (active, groups, highest_node) +} + +/// Walk-reported domains matching `is_kind` that share a processor with another. +/// +/// Split out so the rule is stated once and applied to a list of kinds, rather +/// than written out per kind where a third kind would have to be remembered. +/// See [`Observation::overlapping_walk_relations`] for why overlap is a defect +/// and why only the relationship walk is considered. +fn overlapping_walk_records( + topology: &MachineMemoryTopology, + is_kind: impl Fn(&DomainKind) -> bool, +) -> usize { + let members: Vec<&ProcessorSet> = topology + .domains + .iter() + .filter(|domain| is_kind(&domain.kind) && domain.observed_by(Source::RelationshipWalk)) + .map(|domain| &domain.processors) + .collect(); + members + .iter() + .enumerate() + .filter(|(index, set)| { + members + .iter() + .enumerate() + .any(|(other, candidate)| other != *index && !set.is_disjoint(candidate)) + }) + .count() +} + +/// Reduce a topology, three already-read counters and what their bracket +/// established to an [`Observation`]. +/// +/// The bracket is an ARGUMENT because only the caller knows it. This set it to +/// [`BracketOutcome::HeldStill`] itself, which claims "every counter was read +/// twice and none moved" -- a thing `observe` never establishes, having been +/// handed one set of counters and no way to know what produced them. Asserting +/// it let a caller's stale pair reach `Agree`, or be filed against the parse as +/// a `Disagree`. +/// +/// Split out of [`measure`] so the extraction is reachable without a host. It +/// is the half that decides what the probe reports, and it was untestable while +/// it sat behind `MachineMemoryTopology::discover`: the defect where a NUMA node +/// only CPU Sets reported raised the domain count but could not raise +/// `highest_numa_node` lived here, and no test could construct the topology that +/// exhibits it. The counters stay in `measure` because nothing but the running +/// machine can produce them. +#[must_use] +pub fn observe( + topology: &MachineMemoryTopology, + raw_active_processors: u32, + raw_group_count: u16, + raw_highest_numa_node: Option, + bracket: BracketOutcome, +) -> Observation { + let online_processors = topology.processors.iter().filter(|p| p.online).count(); + + let mut groups = 0usize; + let mut numa_domains = 0usize; + let mut numa_domains_without_processors = 0usize; + let mut numa_domains_only_in_cpu_sets = 0usize; + let mut numa_domains_unreported = 0usize; + let mut numa_domains_with_conflicting_labels = 0usize; + let mut cores_only_in_cpu_sets = 0usize; + let mut cores_without_processors = 0usize; + let mut packages_without_processors = 0usize; + let mut described_relations = 0usize; + let mut unreported_relations = 0usize; + let mut highest_numa_node: Option = None; + let mut packages = 0usize; + let mut cores = Vec::new(); + let mut by_level: Vec<(u8, Vec)> = Vec::new(); + + for domain in &topology.domains { + // Provenance per RELATION, not just per topology. `Provenance::Measured` + // is an object-level fact and the crate is explicit that it permits + // hand-inserted relations -- `Source::Description` exists for "a caller + // adding described relations to a topology that was discovered, which + // is exactly the case per-relation provenance exists to make visible". + // So a measured topology can carry counted relations nobody measured. + // + // **Every** observation must be a `Description`, not merely one of them. + // `observed_by(Description)` also matched a relation the walk reported + // and a caller then annotated -- whose membership IS platform-backed -- + // so the entry this feeds said it was "described by a caller rather than + // reported by any platform API" of a relation the platform had reported. + if !domain.observations.is_empty() + && domain + .observations + .iter() + .all(|o| o.source == Source::Description) + { + described_relations += 1; + } + // A relation nobody reported, which the crate documents as the honest + // state for one built by hand. Counted for EVERY kind: this was checked + // only inside the memory arm, so a hand-inserted core, package or cache + // was counted -- and could change a policy or the cache partitioning -- + // with every quality check still clear. + if domain.observations.is_empty() { + unreported_relations += 1; + } + + match &domain.kind { + DomainKind::Group => groups += 1, + DomainKind::Package => { + packages += 1; + // The empty-record twin of `numa_domains_without_processors`. + // A zero affinity mask raises no anomaly, so a spurious empty + // package inflates the count and `by-package` with it. + if domain.processors.is_empty() { + packages_without_processors += 1; + } + } + DomainKind::Memory { .. } => { + numa_domains += 1; + // Every label the crate reported for this node, from EITHER + // source: node numbers are machine-wide from both, so all of + // them are comparable with the machine-wide counter. Reading + // the walk's alone hid a node only CPU Sets described. + // + // More than one DISTINCT label on one domain is a contradiction + // rather than an alias, and the crate keeps both deliberately -- + // "the labels differ and both are kept, which is the whole of + // D-15". Taking the maximum silently resolves it, so it is + // counted and reported instead. + // Labels from a PLATFORM source only. A caller-described label + // is not something `GetNumaHighestNodeNumber` could have + // reported, so folding one into the maximum files the caller's + // number against the counter -- an accusation against the + // shipping parse for a number no platform API produced. It is + // reachable: a domain the walk reported AND a caller annotated + // is platform-backed, so `described_relations` does not count + // it and the provenance gate stays open. + // + // The conflict count below reads the same filtered list. It + // establishes that a domain carries more than one DISTINCT + // label, not which source supplied which -- nothing here groups + // the labels by their source. Two platform sources disagreeing + // is the usual cause on the path `measure` takes, and is not + // what the count checks. + let mut labels: Vec = domain + .observations + .iter() + .filter(|o| o.source != Source::Description) + .map(|o| o.label) + .collect(); + labels.sort_unstable(); + labels.dedup(); + if labels.len() > 1 { + numa_domains_with_conflicting_labels += 1; + } + for node in labels { + highest_numa_node = + Some(highest_numa_node.map_or(node, |seen: u32| seen.max(node))); + } + // A memory domain the walk never described. Not the same + // question as `coherence`, which compares PROCESSOR SETS: two + // sources can name the same processors and still group them + // into nodes differently, and that lands here as a domain only + // one of them reported. No counter can see it either -- the + // node totals can match while the membership does not. + // Both halves asserted, so the message this feeds stays true. + // `!observed_by(walk)` alone is also satisfied by a domain with + // NO observations, which "reported only by CPU Sets" would then + // describe wrongly -- nobody reported it. + if domain.observed_by(Source::CpuSets) + && !domain.observed_by(Source::RelationshipWalk) + { + numa_domains_only_in_cpu_sets += 1; + } else if domain.observations.is_empty() { + numa_domains_unreported += 1; + } + if domain.processors.is_empty() { + numa_domains_without_processors += 1; + } + } + DomainKind::Core { + simultaneous_multithreading, + efficiency_class, + } => { + // Same provenance question as a memory domain, and the same + // answer. `fold_memberships` pushes a core only CPU Sets + // described as its own domain, so two sources that group the + // same processors into cores differently leave BOTH groupings + // here -- inflating `by-core`, which is a sizing decision. + if domain.observed_by(Source::CpuSets) + && !domain.observed_by(Source::RelationshipWalk) + { + cores_only_in_cpu_sets += 1; + } + if domain.processors.is_empty() { + cores_without_processors += 1; + } + cores.push(CoreShape { + simultaneous_multithreading: *simultaneous_multithreading, + efficiency_class: *efficiency_class, + processors: domain.processors.len(), + }); + } + _ => {} + } + } + + // Asked of the topology rather than counted in the loop above, and stated + // over a LIST of kinds so the rule lives in one place: an empty affinity + // mask is not the only way a record can fail to describe the machine, and a + // record that overlaps another is the way none of the existing counts sees. + // + // MEMORY IS IN THE LIST. Writing the list out as two `+`-joined calls was + // the same enumerate-the-causes shape this file has been bitten by before: + // a processor belongs to exactly one NUMA node just as it belongs to one + // package and one core, so a walk that reports it in two nodes describes no + // machine -- and `numa_domains` feeds a policy directly. The highest-label + // comparison cannot see it, because two overlapping nodes can carry any + // labels at all, including the right maximum. + let overlapping_walk_relations: usize = [ + &(|kind: &DomainKind| matches!(kind, DomainKind::Package)) as &dyn Fn(&DomainKind) -> bool, + &|kind: &DomainKind| matches!(kind, DomainKind::Core { .. }), + &|kind: &DomainKind| matches!(kind, DomainKind::Memory { .. }), + ] + .into_iter() + .map(|is_kind| overlapping_walk_records(topology, is_kind)) + .sum(); + + // Asked of the topology rather than counted from `domains` above, because + // Windows reports one relationship per *cache* and not per partition. + // Measured here: L1 arrives as eight `data` domains plus eight + // `instruction` domains over the same eight processor pairs, so counting + // relationships printed "L1 16 domain(s)" on a machine with eight L1 + // partitions -- and fed a doubled count to every policy in + // `domain_counts`. + for level in topology.cache_levels() { + let spans = topology + .cache_partitions_at_level(level) + .iter() + .map(|domain| domain.processors.len()) + .collect(); + by_level.push((level, spans)); + } + + by_level.sort_by_key(|(level, _)| *level); + let caches = by_level + .into_iter() + .map(|(level, processors_per_domain)| CacheLevel { + level, + processors_per_domain, + }) + .collect(); + + // Asked once, here, rather than restated: the crate that owns the topology + // owns the rule (D-21). + let partitioning_cache_level = topology + .outermost_partitioning_cache() + .map(|(level, _)| level); + + Observation { + online_processors, + groups, + numa_domains, + numa_domains_without_processors, + numa_domains_only_in_cpu_sets, + numa_domains_unreported, + numa_domains_with_conflicting_labels, + topology_was_measured: topology.provenance == Provenance::Measured, + cores_only_in_cpu_sets, + cores_without_processors, + packages_without_processors, + overlapping_walk_relations, + described_relations, + unreported_relations, + processor_attribute_conflicts: topology.attribute_conflicts().len(), + highest_numa_node, + packages, + cores, + caches, + partitioning_cache_level, + enumeration_anomalies: topology.enumeration_anomalies.clone(), + coherence: topology.coherence.clone(), + raw_active_processors, + raw_group_count, + raw_highest_numa_node, + bracket, + } +} diff --git a/crates/windows-platform-probes/src/topology_report.rs b/crates/windows-platform-probes/src/topology_report.rs new file mode 100644 index 000000000..109405727 --- /dev/null +++ b/crates/windows-platform-probes/src/topology_report.rs @@ -0,0 +1,584 @@ +// Copyright (c) Mike Grier. +// Split from bin/topology.rs at c335ae5. + +//! The topology probe's report, as text. +//! +//! Split out of the binary because it was untestable there: `render` called +//! [`crate::topology::measure`] itself, so every branch needed a live host and +//! none could be +//! driven from a test. A mutation sweep found 13 of 13 mutants surviving -- +//! `render` could return `"xyzzy"` and the suite stayed green -- and the +//! survivors were exactly the claims that cost the most review rounds: the +//! "no L3 at all" note, the caveat gate on an absent partitioning answer, and +//! the heterogeneous-core note. Each had been checked by running the binary and +//! reading the output, which nothing repeats on a later change. +//! +//! The same split as [`crate::topology::observe`] out of +//! [`crate::topology::measure`], for the same reason. + +use std::fmt::Write as _; +use std::io; + +use windows_placement_probe::fingerprint::{Fingerprint, banner_line_for}; + +use crate::topology::{Observation, PartitioningCache, Verdict}; + +/// The banner and title both reports open with. +/// +/// The banner is part of the returned text rather than written out separately: +/// a captured report must carry the line naming the machine that produced it, +/// and the taint marker with it. Without it a number can be pasted anywhere and +/// compared against anything. +/// +/// **Taken as an argument rather than read here.** This called +/// `fingerprint::banner_line()`, which runs a topology discovery of its own -- +/// so this module's claim to be testable without a host was false of its very +/// first line, and the probe made a second, unbracketed platform read whose +/// result could describe a different instant from the body's. The caller reads +/// it once and passes it in. +fn preamble(banner: &str) -> String { + let mut out = String::new(); + let _ = writeln!(out, "{banner}"); + let _ = writeln!( + out, + "== processor topology, and what each partitioning policy would yield ==\n" + ); + out +} + +/// The banner to print, given the host fingerprint read before and after the +/// measurement. +/// +/// The fingerprint is itself a topology rendering -- architecture, processor and +/// core counts, cache domain sizes, NUMA nodes -- read through a *second* +/// discovery that `measure`'s bracket does not enclose. Reading it once and +/// calling that "attribution" was the weaker claim it sounded: a machine that +/// changed across the run would print one shape in the banner and a different +/// one in the body, with nothing saying which described the measurement. +/// +/// So it is bracketed like everything else here. Equal readings print as one +/// line, exactly as before. Readings that differ print both and say so, because +/// which of them describes the body is what the run did not establish. +/// +/// **It says WHICH reading is unknown, not that both are wrong.** The +/// measurement happened between them, so one of the two may well name the +/// machine the body describes -- what a disagreement at the endpoints +/// establishes is that this run cannot say which. Calling both wrong would be +/// its own over-claim, in the sentence added to stop one. +/// +/// **It says the readings DIFFER, not that the host changed.** +/// `Fingerprint::discover` returns `Ok` on a parse that dropped a record or +/// whose two sources disagreed, so a fingerprint can differ from the one before +/// it because the enumeration was flaky rather than because any hardware moved. +/// Naming a cause this run cannot distinguish would be the same over-claim the +/// body below is built to avoid; what is established is that the two disagree, +/// and that is all this says. +/// +/// **Takes the discoveries rather than two rendered lines.** Compared as +/// strings, a failed read is a line like any other, so one failure beside one +/// success -- or two failures whose `io::Error` text differs -- read as a host +/// that changed. That is a claim about the machine drawn from a gap in the +/// measurement: a failed read establishes neither that the host moved nor that +/// it held still. +#[must_use] +pub fn attribution(before: &io::Result, after: &io::Result) -> String { + let first = banner_line_for(before); + match (before, after) { + (Ok(one), Ok(two)) if one == two => first, + (Ok(_), Ok(_)) => format!( + "{first}\n{}\nHOST READINGS DISAGREE: the two readings above bracket the measurement\n\ + and differ, so which of them names the machine the body below describes\n\ + was not established.", + banner_line_for(after) + ), + // Covers (Err, Ok), (Ok, Err) AND (Err, Err), so the text says "at + // least one". "One of the two readings failed" understates the case + // where both did -- a small thing, but the same shape as every other + // sentence corrected here: claiming a more specific state than the run + // established. + _ => format!( + "{first}\n{}\nHOST NOT ESTABLISHED: at least one of the two readings that bracket \ + the measurement\nfailed, so nothing confirmed the machine held still under it.", + banner_line_for(after) + ), + } +} + +/// The report for a run whose discovery failed. +/// +/// Carries an `x-probe-topology` row of its own, so the fleet survey can tell a +/// host where discovery FAILED from a job that never ran the probe. Returning +/// only prose made those indistinguishable, which silently excluded exactly the +/// hosts most worth counting. +#[must_use] +pub fn report_unmeasured(banner: &str, error: &io::Error) -> String { + let mut out = preamble(banner); + let _ = writeln!(out, "MachineMemoryTopology::discover failed: {error}"); + let _ = writeln!( + out, + "(Reported rather than measured: a probe that cannot read its" + ); + let _ = writeln!( + out, + "subject must say so instead of printing a misleading shape.)" + ); + let _ = writeln!( + out, + r#"{{"reason":"x-probe-topology","arch":"{}","cross_check":"not_measured"}}"#, + std::env::consts::ARCH + ); + out +} + +/// The report for a run whose discovery succeeded. +#[must_use] +pub fn report(banner: &str, observation: &Observation) -> String { + let mut out = preamble(banner); + + // Computed HERE rather than beside the cache conclusions it was first + // written for. The heterogeneity note below is a hardware claim too -- "an + // unconstrained thread can land on an efficiency core" -- and it was printed + // before this line existed, so it was structurally ungated while the design + // note said `parse_in_doubt` gates every hardware conclusion the renderer + // draws. A rule with one exception is not a rule, and the exception was the + // conclusion drawn from the one field the two sources are known to + // contradict each other about. + let check = observation.cross_check(); + let parse_in_doubt = check.parse_in_doubt(); + + let _ = writeln!( + out, + "processors (online) : {}", + observation.online_processors + ); + let _ = writeln!(out, "processor groups : {}", observation.groups); + let _ = writeln!(out, "packages : {}", observation.packages); + let _ = writeln!( + out, + "NUMA domains : {} ({} with no processors)", + observation.numa_domains, observation.numa_domains_without_processors + ); + if observation.numa_domains_only_in_cpu_sets > 0 { + let _ = writeln!( + out, + " ({} reported only by CPU Sets, never by the relationship walk:", + observation.numa_domains_only_in_cpu_sets + ); + let _ = writeln!(out, " the two sources group nodes differently)"); + } + let _ = writeln!(out, "physical cores : {}", observation.cores.len()); + + let smt = observation + .cores + .iter() + .filter(|c| c.simultaneous_multithreading) + .count(); + let mut classes: Vec = observation + .cores + .iter() + .map(|c| c.efficiency_class) + .collect(); + classes.sort_unstable(); + classes.dedup(); + let _ = writeln!(out, " cores with SMT : {smt}"); + let _ = writeln!(out, " efficiency classes: {classes:?}"); + if classes.len() > 1 { + let _ = writeln!( + out, + " (heterogeneous: an I/O thread left unconstrained can land on an" + ); + let _ = writeln!( + out, + " efficiency core, which is why even a single domain wants a mask)" + ); + if parse_in_doubt { + let _ = writeln!( + out, + " (This run did not establish that the parse is whole, and the classes" + ); + let _ = writeln!( + out, + " above are what decoded -- see the cross-check below for why.)" + ); + } + } + + let _ = writeln!(out, "\ncaches:"); + if observation.caches.is_empty() { + let _ = writeln!(out, " none reported"); + } + for cache in &observation.caches { + let _ = writeln!( + out, + " L{:<2} {:>3} domain(s), processors per domain: {:?}", + cache.level, + cache.domains(), + cache.processors_per_domain + ); + } + + // Every conclusion below is drawn from the cache summaries above, and those + // describe what DECODED rather than what the machine has. An undersized + // `CACHE_RELATIONSHIP` decodes to nothing and is recorded as an anomaly, + // while one whose trailing affinity array is truncated is KEPT with the + // entries that fit -- and `discover` returns `Ok` either way. So the + // summaries can be short a level or carry one whose processor set is + // smaller than the truth, and these lines say what the parse contains + // rather than what the hardware is. + // + // The truncated case is worth stating precisely, because an earlier version + // of this comment said it "decodes to nothing": a kept record with a + // partial mask presents a set that is distinct from the full one, so it can + // ADD a partition rather than remove one. A maintainer who believed the old + // wording would have ruled out the only way that inflation happens. + // + // Named for the CONDITION, not for one of its causes. It was `dropped`, and + // the caveats it gated said "a record was dropped" -- which became false the + // moment the condition widened to every `parse_incomplete` entry: forcing a + // coherence disagreement with zero anomalies printed a caveat blaming a + // dropped record. Each caveat now states that the run did not establish the + // parse is whole and points at the cross-check, which is where the actual + // reason is already printed in full. + // + // ASKED, not re-derived, because every time this condition has been + // restated here it has been restated wrongly. First as + // `!enumeration_anomalies.is_empty()`, so a host reporting + // `Coherence::Disagreed` with no anomalies printed "this machine reports no + // L3 at all" a few lines above "=> INCOMPLETE ... its two enumerations never + // agreed". Then as `!parse_incomplete.is_empty()`, which missed + // `disagreements` -- so a host whose group count Windows contradicts printed + // the same hardware claim directly above "=> DISAGREE", in the one case + // where the evidence that the parse does not describe this machine was + // already in hand. Both times the accompanying comment asserted the + // condition was complete. + // + // `CrossCheck::parse_in_doubt` is now the single definition, and it is the + // place that argues which lists belong. It is computed at the top of this + // function so that every hardware conclusion below reads the same one. + + // Matched exhaustively, so an absent level cannot be printed without having + // decided WHICH absent case it is. This arm used to recite the ambiguity -- + // "either no level partitions this machine, or two partition it + // incomparably" -- because the probe genuinely could not tell. It can tell + // the first apart from the rest, so it now says so. + match observation.partitioning_cache() { + PartitioningCache::Level(cache) => { + // "of the processors it covers", not "of this machine". The owning + // crate's filter is `blocks.len() > 1 && are_pairwise_disjoint`, + // with no coverage check, so two disjoint domains covering + // processors 0 and 1 of a four-processor host qualify -- and + // nothing here verifies otherwise. The mirror sentence in the + // `NoLevelPartitions` arm below was narrowed for this exact reason; + // this one kept the claim, which is the half of a pair being swept + // and the other half left behind. + let _ = writeln!( + out, + "\noutermost cache that partitions the processors it covers: L{} ({} domains)", + cache.level, + cache.domains() + ); + if parse_in_doubt { + let _ = writeln!( + out, + " (of the levels that decoded. This run did not establish that the parse" + ); + let _ = writeln!( + out, + " is whole -- see the cross-check below for why -- so a coarser level may" + ); + let _ = writeln!(out, " exist on this machine and be missing above.)"); + } + } + PartitioningCache::NoLevelPartitions => { + let _ = writeln!( + out, + "\nno cache level reported more than one domain, so nothing here divides" + ); + // Not "every level covers the whole machine", which was the reading + // this offered and is false for a level that decoded to NO + // partitions -- that covers nothing, not everything, and lands here + // too. `cross_check` now caveats that case, but the sentence should + // not have needed the caveat to stop being wrong. + // + // Nor "no cache boundary divides the work", which was the next + // wording and is a claim about the HARDWARE. A single domain + // covering half the online processors also lands here, and nothing + // in this probe checks that the domains at a level cover the + // machine -- so the honest statement is about what this report + // found, not about what the silicon does. + let _ = writeln!(out, "the work by cache."); + } + PartitioningCache::NoLevelsReported => { + let _ = writeln!( + out, + "\nno cache levels were reported at all, so nothing here says whether a" + ); + let _ = writeln!(out, "cache boundary divides this machine."); + } + PartitioningCache::NoUniqueOutermost => { + // "More than one DISTINCT domain", not "partitions this machine". + // `domains()` counts distinct processor sets and the topology crate + // is explicit that distinct is not disjoint, so a level whose blocks + // overlap lands here while partitioning nothing -- which the third + // line below then names, contradicting an opening that claimed the + // machine was partitioned. + let _ = writeln!( + out, + "\nat least one cache level reported more than one distinct domain, but" + ); + let _ = writeln!( + out, + "no unique outermost one was established: either two partition this" + ); + let _ = writeln!( + out, + "machine incomparably, or the candidates were rejected as overlapping" + ); + let _ = writeln!(out, "-- in which case none of them partitions it at all."); + } + PartitioningCache::SummaryMissing(level) => { + let _ = writeln!( + out, + "\nBUG IN THIS PROBE: the topology crate named L{level} as the outermost" + ); + let _ = writeln!( + out, + "partitioning cache and this survey carries no summary for it. Nothing" + ); + let _ = writeln!(out, "below about cache partitioning can be trusted."); + } + } + if parse_in_doubt + && !matches!( + observation.partitioning_cache(), + PartitioningCache::Level(_) + ) + { + // A parse that is short or disputed is exactly how the level that would + // have partitioned this machine goes missing, so none of the three + // absent answers above is safe to read as hardware either. + let _ = writeln!( + out, + "Or the parse is not whole and the level that would have partitioned this" + ); + let _ = writeln!( + out, + "machine is missing -- see the cross-check below for why." + ); + } + if !observation.caches.iter().any(|c| c.level == 3) { + // "no L3" is a claim about the machine, and the list it is read off is + // only what decoded. Asserted as hardware when the parse is whole, and + // as a fact about the parse when it is not -- otherwise a host whose L3 + // record alone failed to decode is filed as an ARM64-style no-L3 + // machine, and its "outermost partitioning cache: L2" is read as a real + // cluster boundary. + if parse_in_doubt { + let _ = writeln!( + out, + "NOTE: no L3 decoded on this run, and this run did not establish that the" + ); + let _ = writeln!( + out, + "parse is whole -- so that is a fact about the parse, not evidence the" + ); + let _ = writeln!(out, "machine has no L3. See the cross-check below."); + } else { + let _ = writeln!( + out, + "NOTE: this machine reports no L3 at all, so a policy keyed literally" + ); + let _ = writeln!( + out, + "on \"L3\" would find nothing here. That is the measured case behind" + ); + let _ = writeln!( + out, + "phrasing the rule as \"the outermost level that partitions\"." + ); + } + } + + let _ = writeln!(out, "\ndomains each policy would produce:"); + for (name, count) in observation.domain_counts() { + let _ = writeln!(out, " {name:<34} {count}"); + } + + let _ = writeln!( + out, + "\ncross-check against independently read Win32 counters:" + ); + let _ = writeln!( + out, + " GetActiveProcessorCount : {}", + observation.raw_active_processors + ); + let _ = writeln!( + out, + " GetActiveProcessorGroupCount: {}", + observation.raw_group_count + ); + match observation.raw_highest_numa_node { + Some(highest) => { + // The identifier, and deliberately no count derived from it. + // `GetNumaHighestNodeNumber` reports the largest node NUMBER, and + // node numbers may be sparse -- a machine with nodes 0 and 2 has two + // nodes and a highest of 2. `highest + 1` would print three, which + // is the same mistake `Observation::cross_check` was corrected to + // stop making; re-deriving it here would put it back in the output + // the cross-check is printed beside. + let _ = writeln!(out, " GetNumaHighestNodeNumber : {highest}"); + let _ = writeln!( + out, + " (the largest node NUMBER, not a count: node numbers can be sparse)" + ); + } + None => { + let _ = writeln!(out, " GetNumaHighestNodeNumber : failed"); + } + } + // Matched exhaustively on purpose. The verdict used to be `complaints + // .is_empty()`, which printed "agree" when a counter had merely failed to + // read -- on the line directly below "GetNumaHighestNodeNumber : failed". + // A three-state verdict makes that arm impossible to omit. + match check.verdict() { + Verdict::Agree => { + let _ = writeln!( + out, + " => agree. Every check this probe could make was made and matched." + ); + } + Verdict::Disagree => { + let _ = writeln!(out, " => DISAGREE. This is a finding, not a nuisance:"); + for complaint in &check.disagreements { + let _ = writeln!(out, " - {complaint}"); + } + // Listed even here, so a reader knows the disagreement above is not + // the whole picture. + for skipped in &check.not_compared { + let _ = writeln!(out, " (not compared) {skipped}"); + } + for caveat in &check.parse_incomplete { + let _ = writeln!(out, " (parse incomplete) {caveat}"); + } + } + Verdict::Incomplete => { + let _ = writeln!( + out, + " => INCOMPLETE. Nothing this probe compared disagreed, but this run" + ); + let _ = writeln!(out, " did not establish that the parse is consistent:"); + for skipped in &check.not_compared { + let _ = writeln!(out, " - {skipped}"); + } + for caveat in &check.parse_incomplete { + let _ = writeln!(out, " - {caveat}"); + } + } + } + + // One machine-readable line, so accumulated CI logs can be mined without + // parsing the prose above. Kept to a single line on purpose. + // + // `cross_check` is the one field a mining pass must read before trusting + // any other. Every count here is taken from what decoded, so a dropped + // record makes `caches`, `packages`, `cores` and + // `outermost_partitioning_cache_level` short by an amount no field states + // -- and a query grouping by cache level has no reason to join against + // `enumeration_anomalies` on its own. `cross_check` closes that: anomalies + // populate `parse_incomplete`, and a non-empty `parse_incomplete` forces + // the verdict away from "agree", so `cross_check == "agree"` IMPLIES no + // record failed to decode. One way only: a run whose counter failed to + // read has a complete parse and still reports "incomplete". That is a rule about + // the code above, so the test + // `a_dropped_enumeration_record_blocks_agreement_even_when_every_counter_matches` + // pins it rather than leaving it as a promise in a comment. + // + // That is narrower than "the counts are complete", and the gap is not + // closed anywhere: nothing independent measures packages, cores or caches, + // so a record that decoded cleanly while describing less of the machine + // than exists raises no anomaly and reaches "agree". `agree` says every + // check this probe could make was made and matched -- not that a check + // exists for every field on this line. + // + // "Not short" is all it says, and the distinction is load-bearing. This + // comment used to promise that `agree` made the rest of the line "describe + // the machine rather than the parse", which was false for + // `outermost_partitioning_cache_level`: a machine whose levels partition it + // incomparably has a complete parse, agrees with every counter, and still + // cannot be said to have an outermost partitioning cache. It emitted `null` + // exactly like a machine no level partitions -- opposite conclusions for + // anything sizing itself by cache boundary, on a row already certified. + // Hence the `outermost_partitioning_cache` field beside it, which a + // consumer must read rather than inferring from the level being absent. + let cache_json: Vec = observation + .caches + .iter() + .map(|c| format!(r#"{{"level":{},"domains":{}}}"#, c.level, c.domains())) + .collect(); + let policy_json: Vec = observation + .domain_counts() + .into_iter() + .map(|(name, count)| format!(r#""{name}":{count}"#)) + .collect(); + let _ = writeln!( + out, + concat!( + r#"{{"reason":"x-probe-topology","arch":"{}","processors":{},"groups":{},"#, + r#""packages":{},"numa_domains":{},"numa_domains_without_processors":{},"cores":{},"#, + r#""efficiency_classes":{},"caches":[{}],"outermost_partitioning_cache_level":{},"#, + r#""outermost_partitioning_cache":"{}","#, + r#""policies":{{{}}},"cross_check":"{}","not_compared":{},"parse_incomplete":{},"#, + r#""enumeration_anomalies":{},"numa_domains_only_in_cpu_sets":{}}}"# + ), + std::env::consts::ARCH, + observation.online_processors, + observation.groups, + observation.packages, + observation.numa_domains, + observation.numa_domains_without_processors, + observation.cores.len(), + classes.len(), + cache_json.join(","), + observation + .outermost_partitioning_cache() + .map_or("null".to_string(), |c| c.level.to_string()), + // The level alone said `null` for every absent case alike, on a line + // the verdict had already certified as "agree" -- an incomparable + // partitioning touches nothing `cross_check` consults. A query counting + // nulls as "machines no cache level partitions" then folded in machines + // where a level DOES partition, which is the opposite conclusion for + // anything sizing itself by cache boundary. Always a string, so a + // consumer filters on `== "none"` rather than on the absence of a + // number. + match observation.partitioning_cache() { + PartitioningCache::Level(_) => "level", + PartitioningCache::NoLevelsReported => "no_levels_reported", + PartitioningCache::NoLevelPartitions => "none", + PartitioningCache::NoUniqueOutermost => "not_unique", + PartitioningCache::SummaryMissing(_) => "summary_missing", + }, + policy_json.join(","), + // A tri-state rather than a boolean, for the reason the prose above + // gives: a log-mining pass over accumulated CI output must be able to + // tell "all three counters agreed" from "two agreed and the third was + // never compared". `cross_check_ok:true` said the same thing for both. + match check.verdict() { + Verdict::Agree => "agree", + Verdict::Disagree => "disagree", + Verdict::Incomplete => "incomplete", + }, + check.not_compared.len(), + // Separate from `not_compared`, because a mining pass that finds + // `"cross_check":"incomplete"` needs to know whether this probe failed + // to read a counter or the parse itself was short or disputed -- the + // first is a gap in the measurement, the second a fact about the + // machine worth going and looking at. The two counts beside it say + // which kind, without a consumer having to know what `cross_check` + // currently pushes for. + check.parse_incomplete.len(), + observation.enumeration_anomalies.len(), + observation.numa_domains_only_in_cpu_sets, + ); + out +} From e1008e7f1fa11a1ef09c0170816fbb9a0b016525 Mon Sep 17 00:00:00 2001 From: Mike Grier Date: Wed, 9 Sep 2026 00:10:44 -0400 Subject: [PATCH 3/7] fix(platform-probes): emit the efficiency classes, not how many there are `"efficiency_classes"` was a count under a plural name, which is ambiguous in the one way that matters for a field whose whole purpose is log mining. On a single-class host the row read `"efficiency_classes":1` while the prose two lines above it printed `efficiency classes: [0]` -- the same fact, in the same report, rendered so the JSON value coincides with a plausible class *label*. A consumer reading the row alone cannot tell "one class" from "class 1", and the two renderings cannot be reconciled. It now emits the list the name promises. That agrees with the prose, removes the count/label collision, and carries strictly more: the count is still available as the array's length, and a fleet survey can additionally see WHICH classes a host reported. Safe to change now and never again -- the probe has not merged, so no accumulated CI output depends on the old shape. Also drops `topology_report.rs`'s provenance header. It named commit `c335ae5`, which was squashed away and is not an ancestor of this branch, so the reference resolves to nothing on any other clone. The header exists so `git blame -C` can trace lines through a split commit; this branch contains no such commit -- both files arrive together -- so there is nothing for a reader to follow and the module doc already records why the renderer is its own module. Raised by review on #82. --- crates/windows-platform-probes/src/tests.rs | 37 ++++++++++++++++++- .../src/topology_report.rs | 37 ++++++++++++------- 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/crates/windows-platform-probes/src/tests.rs b/crates/windows-platform-probes/src/tests.rs index 659216f27..335fdd536 100644 --- a/crates/windows-platform-probes/src/tests.rs +++ b/crates/windows-platform-probes/src/tests.rs @@ -2665,7 +2665,42 @@ fn the_heterogeneous_note_appears_only_with_more_than_one_efficiency_class() { text.contains(note), "two classes means an unconstrained thread can land on an efficiency core: {text}" ); - assert!(text.contains(r#""efficiency_classes":2"#), "{text}"); + assert!(text.contains(r#""efficiency_classes":[0,1]"#), "{text}"); +} + +#[test] +fn the_json_efficiency_classes_are_the_classes_and_not_how_many() { + // A plural name over a count is ambiguous in the one way that matters: a + // single-class host emitted `"efficiency_classes":1`, which reads exactly + // like a machine whose one class is class 1 -- while the prose above it + // printed `efficiency classes: [0]`. Same fact, same report, two renderings + // a consumer cannot reconcile. + let mut one = clean_observation(); + one.cores = vec![crate::topology::CoreShape { + simultaneous_multithreading: true, + efficiency_class: 0, + processors: 2, + }]; + let text = crate::topology_report::report(BANNER, &one); + assert!( + text.contains(r#""efficiency_classes":[0]"#), + "the class is 0, and the row says so rather than saying `1`: {text}" + ); + assert!( + text.contains("efficiency classes: [0]"), + "and the prose agrees with it: {text}" + ); + + // A host whose single class is genuinely class 1 must not render the same + // as the one above -- which is precisely what a count did. + let mut class_one = clean_observation(); + class_one.cores = vec![crate::topology::CoreShape { + simultaneous_multithreading: true, + efficiency_class: 1, + processors: 2, + }]; + let text = crate::topology_report::report(BANNER, &class_one); + assert!(text.contains(r#""efficiency_classes":[1]"#), "{text}"); } #[test] diff --git a/crates/windows-platform-probes/src/topology_report.rs b/crates/windows-platform-probes/src/topology_report.rs index 109405727..733b12559 100644 --- a/crates/windows-platform-probes/src/topology_report.rs +++ b/crates/windows-platform-probes/src/topology_report.rs @@ -1,19 +1,17 @@ // Copyright (c) Mike Grier. -// Split from bin/topology.rs at c335ae5. //! The topology probe's report, as text. //! -//! Split out of the binary because it was untestable there: `render` called -//! [`crate::topology::measure`] itself, so every branch needed a live host and -//! none could be -//! driven from a test. A mutation sweep found 13 of 13 mutants surviving -- -//! `render` could return `"xyzzy"` and the suite stayed green -- and the -//! survivors were exactly the claims that cost the most review rounds: the -//! "no L3 at all" note, the caveat gate on an absent partitioning answer, and -//! the heterogeneous-core note. Each had been checked by running the binary and -//! reading the output, which nothing repeats on a later change. +//! A separate module from the binary because it was untestable there: `render` +//! called [`crate::topology::measure`] itself, so every branch needed a live +//! host and none could be driven from a test. A mutation sweep found 13 of 13 +//! mutants surviving -- `render` could return `"xyzzy"` and the suite stayed +//! green -- and the survivors were exactly the claims that cost the most review +//! rounds: the "no L3 at all" note, the caveat gate on an absent partitioning +//! answer, and the heterogeneous-core note. Each had been checked by running +//! the binary and reading the output, which nothing repeats on a later change. //! -//! The same split as [`crate::topology::observe`] out of +//! The same division as [`crate::topology::observe`] out of //! [`crate::topology::measure`], for the same reason. use std::fmt::Write as _; @@ -526,7 +524,7 @@ pub fn report(banner: &str, observation: &Observation) -> String { concat!( r#"{{"reason":"x-probe-topology","arch":"{}","processors":{},"groups":{},"#, r#""packages":{},"numa_domains":{},"numa_domains_without_processors":{},"cores":{},"#, - r#""efficiency_classes":{},"caches":[{}],"outermost_partitioning_cache_level":{},"#, + r#""efficiency_classes":[{}],"caches":[{}],"outermost_partitioning_cache_level":{},"#, r#""outermost_partitioning_cache":"{}","#, r#""policies":{{{}}},"cross_check":"{}","not_compared":{},"parse_incomplete":{},"#, r#""enumeration_anomalies":{},"numa_domains_only_in_cpu_sets":{}}}"# @@ -538,7 +536,20 @@ pub fn report(banner: &str, observation: &Observation) -> String { observation.numa_domains, observation.numa_domains_without_processors, observation.cores.len(), - classes.len(), + // The CLASSES, not how many there are. A plural name over a count is + // ambiguous in the one way that matters here: on a single-class host + // this emitted `"efficiency_classes":1`, which reads exactly like a + // machine whose one class is class *1* -- while the prose two lines + // above printed `efficiency classes: [0]`. Same fact, same report, two + // renderings a consumer cannot reconcile. The list is what the name + // promises, agrees with the prose, and carries strictly more: a fleet + // survey can still get the count from its length, and can now also see + // WHICH classes a host reported. + classes + .iter() + .map(u8::to_string) + .collect::>() + .join(","), cache_json.join(","), observation .outermost_partitioning_cache() From 033d78981227142db336d8f0e035289712c791bd Mon Sep 17 00:00:00 2001 From: Mike Grier Date: Wed, 9 Sep 2026 00:36:17 -0400 Subject: [PATCH 4/7] fix(platform-probes): block agreement on a named cache level with no summary `PartitioningCache::SummaryMissing` is the state where the topology crate named a level as the outermost partitioning cache and this survey carries no summary for it. The renderer already printed it as "BUG IN THIS PROBE ... Nothing below about cache partitioning can be trusted" -- and `cross_check` said nothing about it, so `verdict()` could certify the same run as `agree`. Two paragraphs of one report, contradicting each other, which is the exact failure this probe exists to prevent. `domain_counts` read the same state through `outermost_partitioning_cache`, whose `None` folds four distinct answers together, so a sizing policy silently got "one domain" from a state the report calls a bug. It now matches on `partitioning_cache()` per variant: every absent case still sizes to one, but they are spelled out, so a variant added later is a compile error here instead of joining the fallback unnoticed. That is the whole reason `PartitioningCache` exists rather than an `Option`. Unreachable through `observe`, which builds the level and the summaries from one survey; reachable through the public fields and `partitioning_cache`, which is the same reason `domain_counts` clamps at all. Also replaces the `&dyn Fn` array behind `overlapping_walk_relations` with plain `fn` pointers. The predicates capture nothing, so the trait objects bought nothing and cost a cast on the first element plus a reliance on temporary lifetimes that a small edit could break. Raised by review on #82. --- crates/windows-platform-probes/src/tests.rs | 60 +++++++++++++++++++ .../windows-platform-probes/src/topology.rs | 42 +++++++++++-- 2 files changed, 96 insertions(+), 6 deletions(-) diff --git a/crates/windows-platform-probes/src/tests.rs b/crates/windows-platform-probes/src/tests.rs index 335fdd536..06d5476a8 100644 --- a/crates/windows-platform-probes/src/tests.rs +++ b/crates/windows-platform-probes/src/tests.rs @@ -4188,3 +4188,63 @@ fn a_bracket_outcome_reaches_the_observation_the_caller_passed_it() { ); } } + +#[test] +fn a_named_level_with_no_summary_blocks_agreement_and_sizes_to_one_domain() { + // The renderer prints this state as "BUG IN THIS PROBE ... Nothing below + // about cache partitioning can be trusted", and `cross_check` said nothing + // about it -- so the verdict could certify the same run as `agree`, two + // paragraphs apart on one page. + // + // `domain_counts` also read it through `outermost_partitioning_cache`, + // whose `None` folds four distinct answers together, so a sizing policy + // silently got "one domain" from a state the report calls a bug. + let mut observation = agreeing_observation(); + observation.caches = vec![crate::topology::CacheLevel { + level: 2, + processors_per_domain: vec![2, 2], + }]; + // A level the survey has no summary for. + observation.partitioning_cache_level = Some(3); + + assert_eq!( + observation.partitioning_cache(), + crate::topology::PartitioningCache::SummaryMissing(3) + ); + + let check = observation.cross_check(); + assert!(check.disagreements.is_empty(), "{check:?}"); + assert!( + check + .parse_incomplete + .iter() + .any(|c| c.contains("carries no summary for it")), + "{check:?}" + ); + assert_eq!( + check.verdict(), + crate::topology::Verdict::Incomplete, + "the verdict cannot say `agree` beside a report that says BUG: {check:?}" + ); + + // And the sizing fallback is still one domain -- stated per variant rather + // than inherited from a `None`, so a new variant is a compile error. + let counts = observation.domain_counts(); + let (_, cache_policy) = counts + .iter() + .find(|(name, _)| *name == "by-outermost-partitioning-cache") + .expect("the policy is in the table"); + assert_eq!(*cache_policy, 1); + + // The report says so too, and the two agree. + let text = crate::topology_report::report(BANNER, &observation); + assert!(text.contains("BUG IN THIS PROBE"), "{text}"); + assert!( + text.contains("=> INCOMPLETE"), + "the prose verdict agrees with the bug note rather than saying agree: {text}" + ); + assert!( + text.contains(r#""cross_check":"incomplete""#), + "and the mining row says the same: {text}" + ); +} diff --git a/crates/windows-platform-probes/src/topology.rs b/crates/windows-platform-probes/src/topology.rs index b9ca0744a..3a9353ca3 100644 --- a/crates/windows-platform-probes/src/topology.rs +++ b/crates/windows-platform-probes/src/topology.rs @@ -433,11 +433,24 @@ impl Observation { ), ( "by-outermost-partitioning-cache", - // `.max(1)` on the SELECTED value, not just the `None` default: + // Matched per variant rather than read through + // `outermost_partitioning_cache`, whose `None` folds four + // distinct answers into one. Every absent case does size to a + // single domain, but they are spelled out so a variant added + // later is a compile error here instead of silently joining + // the fallback -- which is the whole reason `PartitioningCache` + // exists rather than an `Option`. + // + // `.max(1)` is on the SELECTED value, not just the default: // the default covers "no level was chosen", and this covers "a // level was chosen whose summary reports no domains". - self.outermost_partitioning_cache() - .map_or(1, |c| c.domains().max(1)), + match self.partitioning_cache() { + PartitioningCache::Level(cache) => cache.domains().max(1), + PartitioningCache::NoLevelsReported + | PartitioningCache::NoLevelPartitions + | PartitioningCache::NoUniqueOutermost + | PartitioningCache::SummaryMissing(_) => 1, + }, ), ("by-core", self.cores.len().max(1)), ] @@ -663,6 +676,23 @@ impl Observation { )); } + // The renderer prints this state as "BUG IN THIS PROBE ... Nothing + // below about cache partitioning can be trusted", and nothing here + // said anything about it -- so the verdict could certify the same run + // as `agree`, two paragraphs apart on one page. `domain_counts` sized + // it to a single domain besides. + // + // Unreachable through `observe`, since the level is captured from the + // same survey the summaries are built from. `Observation`'s fields and + // `partitioning_cache` are both public, which is the same reason the + // clamps in `domain_counts` exist. + if let PartitioningCache::SummaryMissing(level) = self.partitioning_cache() { + check.parse_incomplete.push(format!( + "L{level} was named as the outermost partitioning cache and this survey carries \ + no summary for it, so what it divides was not established" + )); + } + if !self.topology_was_measured { check.parse_incomplete.push( "this topology was not measured from a running machine, so nothing here \ @@ -1360,9 +1390,9 @@ pub fn observe( // comparison cannot see it, because two overlapping nodes can carry any // labels at all, including the right maximum. let overlapping_walk_relations: usize = [ - &(|kind: &DomainKind| matches!(kind, DomainKind::Package)) as &dyn Fn(&DomainKind) -> bool, - &|kind: &DomainKind| matches!(kind, DomainKind::Core { .. }), - &|kind: &DomainKind| matches!(kind, DomainKind::Memory { .. }), + (|kind: &DomainKind| matches!(kind, DomainKind::Package)) as fn(&DomainKind) -> bool, + |kind| matches!(kind, DomainKind::Core { .. }), + |kind| matches!(kind, DomainKind::Memory { .. }), ] .into_iter() .map(|is_kind| overlapping_walk_records(topology, is_kind)) From b5bd58db8e41c25ac5a8ef6062a3c9f40decf110 Mon Sep 17 00:00:00 2001 From: Mike Grier Date: Wed, 9 Sep 2026 00:58:00 -0400 Subject: [PATCH 5/7] docs(platform-probes): record why correspondence failures survived every instrument Twenty-eight review rounds, two independent readers each, and zero surviving mutants on both modules did not find a state where the renderer printed "BUG IN THIS PROBE ... nothing below about cache partitioning can be trusted" while `cross_check` had no branch for it and the verdict could print `agree` two paragraphs below. A pull-request review found it in code none of that had touched. The decision records why each instrument was structurally incapable rather than unlucky: mutation testing perturbs code that exists, so a missing branch has no mutants and the 180/0 score was unaffected by the gap; exhaustiveness checking compels a `match`, not a concept, so it bought nothing in the two consumers that reached the same state through an `Option`-returning accessor or never asked; and per-artifact review confirms both sides of a contradiction, because each side is locally true. It also records that the reviewers' shared prompt -- which had accumulated focus areas and "already verified" facts -- correlated them far more than their differing models decorrelated them. It records the split that follows: a sparse matrix as a transient exploratory instrument for finding which correlations exist, and an oracle as the durable home for the few that turn out to mean something. Committing the matrix as a maintained table is rejected explicitly -- it would be a second copy of the code's structure that nothing verifies, and a stale "all cells covered" table is worse than none. The work is queued as M2 in the crate's CHECKLIST rather than left in the decision, per the rule that design notes are not a work queue. Whether this generalises past this crate is left open for the engineer. --- crates/windows-platform-probes/CHECKLIST.md | 53 +++++++ .../windows-platform-probes/DESIGN-NOTES.md | 131 ++++++++++++++++++ crates/windows-platform-probes/PLANS.md | 2 +- 3 files changed, 185 insertions(+), 1 deletion(-) diff --git a/crates/windows-platform-probes/CHECKLIST.md b/crates/windows-platform-probes/CHECKLIST.md index a69a23693..f8f4d7d8d 100644 --- a/crates/windows-platform-probes/CHECKLIST.md +++ b/crates/windows-platform-probes/CHECKLIST.md @@ -53,3 +53,56 @@ piece of work rather than a correction to that one. and record the Ctrl-C observation in [DESIGN-NOTES.md](DESIGN-NOTES.md) -- an interactive signal is not something to assert in CI, but it is the property the milestone exists for, so it must be measured once rather than assumed. + + +## M2 -- Check correspondence between the report's parts, not just each part + +A pull-request review found a state where [src/topology_report.rs](src/topology_report.rs) printed +`BUG IN THIS PROBE ... Nothing below about cache partitioning can be trusted` while `cross_check` had +no branch for that state, so the verdict could print `=> agree` two paragraphs below. Twenty-eight +rounds of per-artifact review and a zero-surviving-mutant `cargo-mutants` result had both passed over +it, because every function involved was correct on its own terms and the defect lived in the relation +between two of them. + +See [DESIGN-NOTES.md](DESIGN-NOTES.md) -> [The defects that survived were correspondence +failures](DESIGN-NOTES.md#d-correspondence-failures) for why each instrument was structurally +incapable of finding it, and for the matrix-as-exploration / oracle-as-durable split this milestone +implements. + +The three correlations below are known to be real because each was violated. They are not a +speculative list to extend by imagination -- a fourth is added when a fourth contradiction is found. + +- [ ] **M2.1** -- Add a report oracle to this crate: one shared executable definition of the + correlations that must hold between the parts of a rendered report, checked against the rendered + artifact rather than against internal state. Seed it with the three known invariants: an alarm in + the prose implies the verdict is not `agree`; a fact rendered in both prose and NDJSON agrees across + the two; an uncaveated hardware claim implies `!parse_in_doubt`. Model it on + [../windows-file-watcher/src/contract.rs](../windows-file-watcher/src/contract.rs)'s + `ContractChecker`, which is this repository's worked example and which existed unused while this + probe was being written. + +- [ ] **M2.2** -- Route every test that renders a report through the oracle, so the roughly + twenty-five existing `report()` call sites inherit the checks and every future one does too. This is + the step that makes it an oracle rather than three more tests: a test added beside the others checks + one case, whereas binding the call sites checks every case anyone writes later. Verify the binding by + sabotage -- change an invariant and confirm existing tests go red -- because a binding that only moves + when its own test moves is cosmetic. + +- [ ] **M2.3** -- Add the missing integration test: run `measure()` against the real host, render the + report, and apply the oracle. At the time of M2 the crate had one integration test, asserting only + that a probe writes to stdout, and none of the twenty-five `report()` calls rendered from a real + measurement -- every one used a hand-built `Observation`, which can only contain states its author + already imagined. On CI this runs across the whole hosted-runner fleet, which is where states no + fixture anticipates will actually appear. + +- [ ] **M2.4** -- Explore, with the sparse matrix as the instrument, whether the same correspondence + failures exist for `Coherence`, `BracketOutcome` and `Verdict`, and in the sibling probes' renderers. + Expect the matrix to be mostly empty; that is the expected shape and not a sign the exercise failed. + **Record the vacuous results as well as the findings** -- "X and Y were examined and need not + correspond" is what stops the next person re-exploring the same cells, and is the half that normally + evaporates. Promote only what proves meaningful into the oracle from M2.1. + +> **-> OPEN QUESTION for the engineer:** M2.4 may show this generalises past this crate, in which case +> the oracle belongs somewhere shared and the question becomes a repository-wide convention rather than +> a probe-crate one. That is a design decision, not a mechanical follow-on, and is deliberately left +> unanswered here. diff --git a/crates/windows-platform-probes/DESIGN-NOTES.md b/crates/windows-platform-probes/DESIGN-NOTES.md index 217c83d27..fa66244c9 100644 --- a/crates/windows-platform-probes/DESIGN-NOTES.md +++ b/crates/windows-platform-probes/DESIGN-NOTES.md @@ -876,3 +876,134 @@ This is a wider window than `measure`'s own bracket rather than a duplicate of it: it closes over the whole run including both banner reads, where `measure` closes only over the counters. Neither subsumes the other, and no work is scheduled by this decision. + +## The defects that survived were correspondence failures, and no instrument here could see them + + + +This probe was reviewed twenty-eight times before it opened as a pull request, +by two independent readers per round on different models, with `cargo-mutants` +reporting **zero surviving mutants** on both of its modules. A review on the +pull request then found, in code none of that had touched, a state where the +renderer printed + +``` +BUG IN THIS PROBE: the topology crate named L3 as the outermost +partitioning cache and this survey carries no summary for it. Nothing +below about cache partitioning can be trusted. +``` + +while `cross_check` had no branch for that state at all, so `verdict()` could +return `Agree` for the same run and print `=> agree` two paragraphs below. A +second finding in the same review had the same shape: one fact rendered twice +in one report -- `efficiency classes: [0]` in prose, `"efficiency_classes":1` +in the NDJSON -- in two shapes a consumer cannot reconcile, where the numeral +happens to read as a plausible class *label*. + +Neither is a bug inside a function. Every function involved was correct on its +own terms, and each had been read repeatedly and found so. The defect lived in +the **relation between two artifacts**, and that is a place none of the +instruments in use could look. + +### Why each instrument was structurally incapable, not merely unlucky + +**Mutation testing cannot find absent code.** `cargo-mutants` perturbs what is +written and asks whether a test notices. A missing branch has no mutants, so +the missing `SummaryMissing` check did not lower the score -- it was invisible +to it. The 180/0 result was true and said nothing about the gap. A perfect +mutation score is compatible with an entirely missing feature, and this +component is the proof. + +The same run also shows the weaker half of what a mutation score means. A test +existed asserting `"efficiency_classes":2`, so every mutant of that line died. +It was pinning the wrong shape faithfully. **Mutation testing measures whether +behavior is pinned by tests; it is silent on whether the pinned behavior is +right.** Both halves were over-read here for many rounds as though they were +evidence of correctness. + +**Exhaustiveness checking protects `match` expressions, not concepts.** +`PartitioningCache` exists precisely to force a decision -- its own doc says a +renderer or serialiser "cannot emit the absent case without having decided +which absent case it is" -- and it worked, in the two consumers that wrote a +`match`. It bought nothing in the two that did not: `domain_counts` reached the +same information through `outermost_partitioning_cache`, a second accessor +returning `Option`, which launders five states into two; and `cross_check` +never asked. A type can only compel a consumer that consults it. + +**Per-artifact review finds per-artifact defects.** Two readers checking each +function against its own documentation will confirm both sides of a +contradiction, because each side is locally true. Worse, the readers were +answering questions posed in a prompt, and across rounds that prompt +accumulated focus areas and "already verified, do not re-litigate" facts. The +shared prompt correlated the readers far more strongly than their differing +models decorrelated them; the instrument was being shaped to agree with its +author. Removing that framing in the final round is what got a reader to trace +`simultaneous_multithreading` out of this crate into `windows-topology-sys` and +check it against the Win32 `LTP_PC_SMT` contract. + +The single sentence that covers all three: **every instrument in use verified +properties of things that exist.** Tests assert existing behavior, mutation +perturbs existing code, reviewers check written claims. A correspondence +failure is a property of a *pair*, and an absent branch is not a thing at all. + +### Integration-level analysis was absent, which is where these live + +At the time of the pull request the crate had one integration test, asserting +that a probe writes something to stdout. Of twenty-five `report()` calls in the +suite, **none rendered from a real host's `measure()`** -- every one used a +synthetic `Observation` built by hand. A hand-built fixture can only contain +states its author already imagined, and each assertion checked one local fact +about it. Nothing anywhere rendered the artifact a consumer actually reads and +asked whether it was self-consistent. + +### What to do instead: a sparse matrix to explore with, an oracle to keep + +The obvious response -- tabulate every state against every consumer and fill +the grid -- is wrong, and was proposed and rejected during this analysis. Such +a table grows combinatorially, most of its cells are meaningless, and a version +of it committed beside the code would be a second copy of the code's structure +that nothing verifies. It would rot exactly as every restatement in this +component rotted, and a stale "all cells covered" table is more dangerous than +no table. + +The division that does work: + +- **The matrix is a transient, exploratory instrument.** Draw it for one type + at one boundary to find out which correlations exist. It is expected to be + **sparse**; most cells are empty and discovering that is cheap. Correlations + cannot be derived -- which is why twenty-eight rounds of reading produced + none -- so populating it is exploration, not specification. +- **An oracle is the durable artifact.** Only cells that turn out to mean + something graduate into it. It stays small because discovery, not + enumeration, fills it. + +`windows-file-watcher`'s `ContractChecker` is this repository's worked example +of the oracle half: a shared executable definition of the rules, owned by the +crate that owns the contract, that the producing crate's own tests and every +consumer's test doubles all bind to. It already existed while this probe was +being written, and was not reached for. + +Three correlations are known to be real here, each because it was violated: + +1. an alarm in the report implies the verdict is not `agree`; +2. a fact rendered twice must agree across its renderings; +3. an uncaveated hardware claim implies `!parse_in_doubt`. + +What makes an oracle different from three more tests is where it is invoked: if +every test renders *through* it, all twenty-five existing call sites inherit +the checks and so does every future one. A test added beside them checks one +case; an oracle checks every case anyone ever writes. + +**Record the vacuous findings too.** "We examined whether X and Y must +correspond, and they need not" is a result, and it is the half that normally +evaporates -- without it the next person re-explores the same empty cells. + +An oracle is a forcing function for correlations already discovered. It will +not find a new one. The discipline that makes it compound is that each newly +found cross-artifact contradiction adds an invariant to the oracle rather than +a one-off test. + +Whether this generalises to `Coherence`, `BracketOutcome`, `Verdict` and the +sibling probes is **an open question, deliberately not answered here.** The work +this decision implies is queued as M2 in [CHECKLIST.md](CHECKLIST.md); this +section schedules nothing on its own. diff --git a/crates/windows-platform-probes/PLANS.md b/crates/windows-platform-probes/PLANS.md index e2aa1b16a..b119098bc 100644 --- a/crates/windows-platform-probes/PLANS.md +++ b/crates/windows-platform-probes/PLANS.md @@ -4,5 +4,5 @@ Design decisions are in [DESIGN-NOTES.md](DESIGN-NOTES.md). | Path to CHECKLIST.md | Status | Brief description | Design Notes | |---|---|---|---| -| [CHECKLIST.md](CHECKLIST.md) | not started | M1: stream a probe's report as it is measured. The report sink buffers each report into a `String`, so a termination that does not unwind -- Ctrl-C, or an abort during unwinding -- discards it, where the line-by-line printing it replaced kept it. Costs most on `probe-cancel-io`, which runs about twenty seconds precisely when the wedge it hunts for occurs. | [DESIGN-NOTES.md](DESIGN-NOTES.md#d-buffered-report) | +| [CHECKLIST.md](CHECKLIST.md) | not started | M1: stream a probe's report as it is measured. The report sink buffers each report into a `String`, so a termination that does not unwind -- Ctrl-C, or an abort during unwinding -- discards it, where the line-by-line printing it replaced kept it. Costs most on `probe-cancel-io`, which runs about twenty seconds precisely when the wedge it hunts for occurs. M2: check correspondence between a report's parts rather than each part alone, after a pull-request review found the renderer calling a state a bug while the verdict certified the same run as `agree`. | [DESIGN-NOTES.md](DESIGN-NOTES.md#d-buffered-report), [#d-correspondence-failures](DESIGN-NOTES.md#d-correspondence-failures) | | [../../CHECKLIST-thread-ambient.md](../../CHECKLIST-thread-ambient.md) | in progress | M27: create the crate, migrate this session's probes into it under the three-tier scheme, and queue migration of the nine earlier measurements that still live only in git-ignored scratch. | [DESIGN-NOTES.md](DESIGN-NOTES.md) | From 3860dbfb6d07553bdca324b85428df86290ff5a3 Mon Sep 17 00:00:00 2001 From: Mike Grier Date: Wed, 9 Sep 2026 01:22:59 -0400 Subject: [PATCH 6/7] fix(platform-probes): stop calling a named cache level missing, and stop laundering it to null Both defects are consequences of 033d789, which made `SummaryMissing` block agreement. Before that commit `SummaryMissing` did not put the parse in doubt on its own, so the missing-level caveat -- whose gate excludes `Level(_)` alone -- could not reach it. Once it did, the report printed BUG IN THIS PROBE: the topology crate named L3 as the outermost partitioning cache and this survey carries no summary for it. Or the parse is not whole and the level that would have partitioned this machine is missing -- see the cross-check below for why. A level WAS named; its summary is what is absent, and the arm above already says so. `the_missing_level_caveat_needs_both_doubt_and_an_absent_level` states that invariant for `Level` -- "a named level is not missing, whatever else is in doubt" -- and `SummaryMissing` names a level too. Both variants that name one are now excluded from the gate. The same commit took `domain_counts` off `outermost_partitioning_cache`, whose `None` folds the summary-missing case in with the three that name no level, and did not sweep the NDJSON serialiser reading through the same accessor. So the row carried `"outermost_partitioning_cache_level":null` beside `"outermost_partitioning_cache":"summary_missing"` while the prose printed the number: one fact, two renderings, and a mining pass counting nulls as "no level was named" folding in a host where one was. The level is now read per variant. Found by a review round aimed specifically at correspondence between the report's parts rather than at either part alone -- the class recorded in DESIGN-NOTES.md under `#d-correspondence-failures`, whose worked example is the contradiction 033d789 fixed. That the fix for one instance produced two more is the strongest argument for the oracle queued as M2: these were caught by a reviewer looking for the shape, and nothing in the suite would have caught them on the next change. --- crates/windows-platform-probes/src/tests.rs | 48 +++++++++++++++++++ .../src/topology_report.rs | 32 ++++++++++--- 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/crates/windows-platform-probes/src/tests.rs b/crates/windows-platform-probes/src/tests.rs index 06d5476a8..b4fb5f734 100644 --- a/crates/windows-platform-probes/src/tests.rs +++ b/crates/windows-platform-probes/src/tests.rs @@ -4248,3 +4248,51 @@ fn a_named_level_with_no_summary_blocks_agreement_and_sizes_to_one_domain() { "and the mining row says the same: {text}" ); } + +#[test] +fn a_named_level_with_no_summary_is_not_also_called_missing() { + // Two contradictions in one arm, both introduced by the fix that made + // `SummaryMissing` block agreement. + // + // The prose says "the topology crate named L3 as the outermost partitioning + // cache", and the caveat gate -- which excludes only `Level(_)` -- then + // added "the level that would have partitioned this machine is missing" + // directly beneath it. A level was named; it is its SUMMARY that is absent. + // `the_missing_level_caveat_needs_both_doubt_and_an_absent_level` already + // states that invariant for `Level`, and `SummaryMissing` names a level too. + // + // The NDJSON separately laundered the level away: it read through + // `outermost_partitioning_cache()`, whose `None` covers this case, so the + // row carried `"outermost_partitioning_cache_level":null` beside + // `"outermost_partitioning_cache":"summary_missing"` while the prose printed + // the number. `domain_counts` was de-laundered in the same commit that + // created the contradiction; this consumer was not swept with it. + let mut observation = agreeing_observation(); + observation.caches = vec![crate::topology::CacheLevel { + level: 2, + processors_per_domain: vec![2, 2], + }]; + observation.partitioning_cache_level = Some(3); + assert_eq!( + observation.partitioning_cache(), + crate::topology::PartitioningCache::SummaryMissing(3) + ); + assert!(observation.cross_check().parse_in_doubt()); + + let text = crate::topology_report::report(BANNER, &observation); + assert!(text.contains("named L3"), "{text}"); + assert!( + !text.contains("the level that would have partitioned this"), + "a level WAS named -- its summary is what is absent, and the two \ + statements cannot both be printed: {text}" + ); + assert!( + text.contains(r#""outermost_partitioning_cache_level":3"#), + "the row carries the level the prose names, rather than laundering it \ + to null through an Option accessor: {text}" + ); + assert!( + text.contains(r#""outermost_partitioning_cache":"summary_missing""#), + "and still says which absent case it is: {text}" + ); +} diff --git a/crates/windows-platform-probes/src/topology_report.rs b/crates/windows-platform-probes/src/topology_report.rs index 733b12559..ea2b64b68 100644 --- a/crates/windows-platform-probes/src/topology_report.rs +++ b/crates/windows-platform-probes/src/topology_report.rs @@ -349,12 +349,20 @@ pub fn report(banner: &str, observation: &Observation) -> String { if parse_in_doubt && !matches!( observation.partitioning_cache(), - PartitioningCache::Level(_) + PartitioningCache::Level(_) | PartitioningCache::SummaryMissing(_) ) { // A parse that is short or disputed is exactly how the level that would - // have partitioned this machine goes missing, so none of the three - // absent answers above is safe to read as hardware either. + // have partitioned this machine goes missing, so the absent answers + // above are not safe to read as hardware either. + // + // Both variants that NAME a level are excluded, not just `Level`. The + // exclusion list read `Level(_)` alone while `SummaryMissing` could not + // reach here -- it did not put the parse in doubt by itself -- and the + // check that made it do so turned this into "the topology crate named + // L3 ... " printed directly above "the level that would have + // partitioned this machine is missing". A level was named; its SUMMARY + // is what is absent, and the arm above says exactly that. let _ = writeln!( out, "Or the parse is not whole and the level that would have partitioned this" @@ -551,9 +559,21 @@ pub fn report(banner: &str, observation: &Observation) -> String { .collect::>() .join(","), cache_json.join(","), - observation - .outermost_partitioning_cache() - .map_or("null".to_string(), |c| c.level.to_string()), + // The level the prose names, read per variant rather than through + // `outermost_partitioning_cache`, whose `None` covers the + // summary-missing case too. Routing through it emitted + // `"outermost_partitioning_cache_level":null` beside + // `"outermost_partitioning_cache":"summary_missing"` while the prose + // printed the number -- one fact, two renderings, no way to reconcile + // them. `domain_counts` was taken off the same accessor for the same + // reason; this consumer was not swept with it. + match observation.partitioning_cache() { + PartitioningCache::Level(cache) => cache.level.to_string(), + PartitioningCache::SummaryMissing(level) => level.to_string(), + PartitioningCache::NoLevelsReported + | PartitioningCache::NoLevelPartitions + | PartitioningCache::NoUniqueOutermost => "null".to_string(), + }, // The level alone said `null` for every absent case alike, on a line // the verdict had already certified as "agree" -- an incomparable // partitioning touches nothing `cross_check` consults. A query counting From 00042e00b71a7c2f1f6bd4b309bba69bde356fce Mon Sep 17 00:00:00 2001 From: Mike Grier Date: Wed, 9 Sep 2026 04:21:01 -0400 Subject: [PATCH 7/7] docs(platform-probes): queue the banner/body read correspondence as M2.5 A probe run performs three independent topology discoveries -- one for the banner, one inside `measure()` for the body, one for the banner again -- and `attribution` compares only the two endpoints. Equal endpoints therefore print an unqualified banner without establishing that the read the body describes agreed with them. Recorded rather than fixed in this pull request, with the reachability stated precisely in both directions. `measure()` already brackets its counters around its own discovery, so a processor, group or NUMA change during the middle read is caught as `BracketOutcome::Changed`; what no counter reaches is cache and efficiency-class structure. The reachable case is therefore narrow -- cache structure differing between reads while processor, group and NUMA counts hold -- which real hardware essentially cannot do and a misbehaving hypervisor can, which is the population the probe exists to survey. Filed under M2 because "the banner describes the measured read" is a correspondence invariant and belongs in that milestone's oracle, checked on every rendered report, rather than asserted once beside it. The reviewer disagreement is recorded with it: across two rounds one reader raised this twice while two others cleared it, one explicitly after being asked the question directly. Nothing in the test suite decides it either way, and that is the argument for the oracle rather than a fact about who was right. --- crates/windows-platform-probes/CHECKLIST.md | 31 +++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/crates/windows-platform-probes/CHECKLIST.md b/crates/windows-platform-probes/CHECKLIST.md index f8f4d7d8d..3efef01fd 100644 --- a/crates/windows-platform-probes/CHECKLIST.md +++ b/crates/windows-platform-probes/CHECKLIST.md @@ -106,3 +106,34 @@ speculative list to extend by imagination -- a fourth is added when a fourth con > the oracle belongs somewhere shared and the question becomes a repository-wide convention rather than > a probe-crate one. That is a design decision, not a mechanical follow-on, and is deliberately left > unanswered here. + +- [ ] **M2.5** -- Make the banner describe the read the body describes. A probe run performs + **three** independent `MachineMemoryTopology::discover()` calls: `Fingerprint::discover()` for the + banner, `measure()`'s own discovery for the body, and `Fingerprint::discover()` again. `attribution` + compares only the two endpoints, so equal endpoints print an unqualified banner without establishing + that the middle read agreed with them. + + **The uncovered window is narrow, and worth stating precisely so it is not over- or under-sold.** + `measure()` brackets its counters around its own discovery, so a processor, group or NUMA change + during the middle read is already caught as `BracketOutcome::Changed`. What no counter reaches is + cache and efficiency-class structure. So the reachable case is a run where the cache structure + differs between the endpoint reads and the middle read while the processor, group and NUMA counts + stay identical -- near-impossible on real hardware, since caches do not change without processors + changing, but reachable on a hypervisor returning inconsistent `GetLogicalProcessorInformationEx` + results, which is exactly the population this probe exists to survey. + + Prefer **construction over comparison**: return the measured topology from `measure()` (as a sibling + function, so the six existing `measure()` callers are untouched) and build the banner with the + already-public `Fingerprint::from_topology`. The banner then describes the body's read *by + construction* and the contradiction becomes unrepresentable, rather than detected by a third + comparison that is itself new prose able to drift. The endpoint reads still earn their place: they + catch structural change across the wider window that the counter bracket cannot see. + + **This belongs to M2 rather than beside it:** "the banner describes the measured read" is a + correspondence invariant, so it should be expressed in the M2.1 oracle and checked on every rendered + report, not asserted once in a single test. + + Reviewer disagreement is recorded deliberately, because it is evidence about the instrument rather + 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.