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-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}" + ); +} diff --git a/crates/windows-platform-probes/CHECKLIST.md b/crates/windows-platform-probes/CHECKLIST.md index a69a23693..3efef01fd 100644 --- a/crates/windows-platform-probes/CHECKLIST.md +++ b/crates/windows-platform-probes/CHECKLIST.md @@ -53,3 +53,87 @@ 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. + +- [ ] **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. 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..fa66244c9 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,422 @@ 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. + +## 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) | 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..b4fb5f734 100644 --- a/crates/windows-platform-probes/src/tests.rs +++ b/crates/windows-platform-probes/src/tests.rs @@ -693,3 +693,3606 @@ 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":[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] +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" + ); + } +} + +#[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}" + ); +} + +#[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.rs b/crates/windows-platform-probes/src/topology.rs new file mode 100644 index 000000000..3a9353ca3 --- /dev/null +++ b/crates/windows-platform-probes/src/topology.rs @@ -0,0 +1,1460 @@ +// 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", + // 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". + 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)), + ] + } + + /// 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" + )); + } + + // 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 \ + 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 fn(&DomainKind) -> bool, + |kind| matches!(kind, DomainKind::Core { .. }), + |kind| 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..ea2b64b68 --- /dev/null +++ b/crates/windows-platform-probes/src/topology_report.rs @@ -0,0 +1,615 @@ +// Copyright (c) Mike Grier. + +//! The topology probe's report, as text. +//! +//! 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 division 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(_) | PartitioningCache::SummaryMissing(_) + ) + { + // A parse that is short or disputed is exactly how the level that would + // 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" + ); + 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(), + // 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(","), + // 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 + // 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 +}