nxmem: bind the Phase 6 fixes to behaviour, not to their own counters by justinchuby · Pull Request #1454 · justinchuby/onnx-genai

justinchuby · 2026-08-19T11:01:40Z

Part of #1186 — Phase 6 only.

Supersedes #1448 and #1440, both of which stay open as records. Branches from fb9a72c4 (head of #1448), so all of that work is included.

The reviewer's position on #1448 was that the production code is correct and that not a line of it needed to change — it was rejected because five mutations against the new code survived the suite. The specific failure was that each of the six fixes was verified through a counter the fix increments, rather than through the behaviour the fix performs. Deleting a counter turned tests red. Deleting the behaviour and keeping the counter did not.

This PR is therefore almost entirely test-side.

The one production change

crates/onnx-runtime-memory-host/src/loader.rs, +30 lines, no logic:

impl Drop for PluginModule bumping a MODULES_UNMAPPED static.
pub fn PluginModule::modules_unmapped() -> u64.

This is deliberately not another leak counter. PluginModule declares library last and Drop::drop runs before fields, so reaching this drop means dlclose is unconditionally about to run — it observes the unmap rather than restating the decision to leak. A leaked strong reference shows up as this count not moving, whatever the platform then does with the mapping. (Confirmed adding Drop here does not re-trigger the E0509 that forced the try_unload restructure.)

Findings

A1 (High) — `Drop for MemoryPlugin` self-retention

Mutated: deleted core::mem::forget(Arc::clone(&self.module)), kept FORCED_MODULE_LEAKS.fetch_add.
Was: 101 passed / 0 failed. Now: 108 passed / 3 failed.

Red test	Assertion
`dropping_a_plugin_with_live_objects_keeps_the_module_mapped`	"a module dropped with live objects must never reach dlclose, even after those objects are gone"
`unload_is_refused_when_only_the_plugin_still_owns_something`	"the module must not reach dlclose while the plugin still owns an allocator"
`a_plugin_that_cannot_report_readiness_is_neither_unloaded_nor_unmapped`	"a module the host could not clear must stay mapped"

The modules_unmapped() delta must be measured across a region that includes drop(allocator) — otherwise the allocator's own live Arc masks the mutation and the test passes for the wrong reason. an_idle_plugin_unloads asserts the delta is 1, so the mutation is caught in both directions: it cannot be satisfied by never unmapping anything.

A2 (High) — `AllocatorCore::drop` callback-table pinning

Mutated: core::mem::forget(context) → drop(context), kept LEAKED_CALLBACK_TABLES.
Now: 74 passed / 1 failed — the low pass count is itself the evidence: the new end-to-end test takes SIGBUS and destroys the whole 36-test nxmem_abi binary.

This was the finding the original rejection named and neither revision closed. report_completion had exactly one caller, reachable only synchronously from a host call, and there was no thread::spawn anywhere in the test plugin — so no test ever had the plugin touch the host callback table after AllocatorCore::drop returned. The guarantee at abi/src/lib.rs:49 was verified as a property of this plugin, not of the ABI.

New callback-after-drop mechanism + exported NxmemTestpluginReportParkedCompletion hook: parks the NxmemHostCallbacks pointer at open, and on demand reports a completion through it from a real std::thread (joined before returning, so the test is deterministic).

The binding assertion is Arc::strong_count, not the crash. Opening an allocator takes exactly two Arc<PluginModule> refs — one in AllocatorCore.module, one in HostBridge.module. mem::forget(context) leaks the box and its contents, so the bridge's ref is never released; drop(context) releases it:

assert_eq!(Arc::strong_count(plugin.module()), pins_before - 1,
    "only the core's module reference may be released; the leaked callback table still owns its own");

in nxmem_abi_callback_pinning.rs. That is safe, deterministic, and green/red without relying on UB. The SIGBUS in a_plugin_thread_reports_a_completion_after_its_allocator_is_gone is reported as corroboration only.

Miri does not cover this. Miri cannot execute a dlopen'd cdylib, so the entire host integration suite is outside it. I am not claiming sanitiser coverage of this path. Miri coverage is -p onnx-runtime-memory-abi --lib only.

B (Medium) — multi-pass drain loop

Two separate tests, because one does not kill both mutations.

Mutation	Red	Assertion
`MAX_DRAIN_PASSES_ON_DROP: 16 → 1`	8 tests	`dropping_an_allocator_drains_in_as_many_passes_as_the_plugin_needs`: "the host must come back once per release the plugin was willing to retire"
drain budget `u64::MAX → 1`	6 tests	`dropping_an_allocator_offers_an_unbounded_budget_per_pass`: "an unbounded budget must let a willing mechanism retire the whole queue in one pass"
remove the `\|\| retired == 0` progress guard	1 test	`dropping_an_allocator_with_a_queued_release_leaks_its_callback_table`: "a pass that retires nothing must end the loop, not restart it"

New drip mechanism retires exactly one release per call however large a budget it is offered, so a queue of n genuinely takes n passes. The budget mutation needs 20 queued releases against lazy — with 3 it survives, because 3 ≤ 16 passes.

C (Medium) — per-allocator `outstanding_releases`

Mutated: the check in AllocatorCore::drop → self.module.host_live_counts().queued_releases. Red: a_queued_release_on_one_allocator_does_not_pin_anothers_table — "a release queued against a different allocator must not leak this allocator's callback table".

Its own binary (nxmem_abi_allocator_scoped_pinning.rs), because sticky poisons process-global module counters.

D (Low) — count-before-call ordering

Mutated: moved outstanding_releases.fetch_add after enqueue_release (D1), and separately the module-wide release_queued() (D2). Both red on a_completion_arriving_inside_enqueue_never_sees_an_underflowed_count.

The naive version of this test does not work: AtomicU64::fetch_sub wraps rather than panicking even in debug, so a single reentrant completion goes 0 → u64::MAX → 0 and the final value is identical. The kill requires reading the counter during the window — a new reentrant-completion mechanism reports the completion from inside enqueue_release, and a MidCallObserver snapshots host accounting from its request_reclaim:

"the reentrant completion decremented a count the host had already incremented; a count taken afterwards would read u64::MAX here"

The observer uses OnceLock, not Mutex — this path re-enters the host from inside a plugin call, and a held host lock would deadlock rather than fail.

E (Medium) — `live_views`

Mutated: deleted counters().live_views.fetch_add(1, …). Was: 38 green. Now: 15 red.

New a_committed_shared_prefix_is_reported_as_a_live_view mirrors the live_capabilities coverage: delta assertions over commit, the refusal path, a view outliving its prefix handle, and retirement at block removal. The plugin-side model and retiring-at-block-removal are unchanged, as directed.

F (Low) — MSVC probe order

find_cc now tries cl first on cfg!(target_os = "windows") (after $CC), extracted as probe_msvc(). Without this, a Windows runner with LLVM on PATH silently checked clang and never MSVC — which defeats the purpose, MSVC being the compiler most likely to disagree with Rust on #[repr(C)] packing.

I cannot verify this. This host is macOS/arm64; the changed branch is cfg-gated to Windows and was never executed here. It is a code-reading change only.

Mutations I invented against code I did not write

19 mutations against the #1448 and pre-existing code, especially the F2 pinning and F4 Drop. Four survived and are now closed; one is proven equivalent; one is disclosed as probably equivalent.

#	Mutation	Result
O1	`Drop for MemoryPlugin` gate ignores the plugin's own report	killed (2)
O2	degraded unload report `Err(_) => u64::MAX` → `=> 0`	SURVIVED → closed, see below
O3	remove the drop gate's early return (always leak)	killed (15)
O4	remove `retire_queued_releases_on_drop()` entirely	killed (9)
O5	pinning check `== 0` → `<= 1`	killed (15)
O6	remove `outstanding_releases.fetch_sub(1)` in `host_release_completed`	killed (4)
O7	move that `fetch_sub` to the top of `host_release_completed`	killed (4)
O8	`try_unload`'s `Arc::try_unwrap` always `Ok`	killed (1)
O9	remove the `offset_of!(Self, allocate)` floor in `read_prefix_unvalidated`	SURVIVED → closed, see below
O10	`AllocatorCore::drop` skips draining `self.live`	SURVIVED → closed, see below
O11	remove the `.min(negotiated_minor)` clamp	killed (1)
O12	neuter the `AbandonOnDrop` guard	killed (13)
O13	`structured_release_slot` drops its minor guard	SURVIVES — proven equivalent, see below
O14	remove `self.factories.clear()` before `drop(self)` in `try_unload`	SURVIVES — probably equivalent, disclosed below
O15	never free the callback table	killed (11)
O16	remove the `insert_allocation` rollback after a failed `enqueue_release`	SURVIVED → closed, see below
O17	remove `vtable.release_allocation = None` in `read_prefix_unvalidated`	killed (1)
O19	test plugin publishes `release_allocation` regardless of negotiated minor	survives alone; see O13
O18	O17 + O19 + O13 together (all three layers)	killed (1)

The four that survived, and what closed them

O2 — the degraded unload report. try_unload's degraded path uses Err(_) => u64::MAX so a plugin that cannot answer "what do you still own?" is treated as owning everything. Changing that to 0 — i.e. unloading a module whose state is unknown — was invisible. Closed by nxmem_abi_degraded_report.rs (its own binary; the mechanism poisons a process-global static): a plugin made to fail unload_readiness is asserted to be neither unloaded nor unmapped, the second half via modules_unmapped.

O9 — the allocate offset floor. read_prefix_unvalidated refuses anything shorter than offset_of!(Self, allocate) (48 bytes) before consulting the sender's abi_minor. Every existing short-struct test used MIN_STRUCT_SIZE_MINOR_0 - 1, which is far above the floor, so the floor itself was never exercised. New a_vtable_too_short_to_declare_its_own_minor_is_refused_without_reading_it:

an 8-byte declaration claiming abi_minor = 1 must be refused against the baseline requirement, and the message must not contain "minor 1" — a reader that believed the claimed level would have read abi_minor from a struct too small to contain it;
a 4-byte heap allocation with struct_size = 4, where reading abi_minor is genuinely out of bounds. Plain cargo test cannot see that; this one is under Miri, which does cover the abi crate's --lib tests.

O10 — the drop-time drain of leaked allocations. Making AllocatorCore::drop skip draining self.live was invisible, and the obvious assertion doesn't help: the module-wide tally self-heals, because the plugin's own release_state drains its maps at refcount zero. Asserting live_allocations == 0 is therefore a property of this plugin, not of the ABI — precisely the weakness class that caused the rejection. Closed with a TERMINAL_RELEASES counter incremented on entry into deallocate/release_allocation, which separates "the host handed these back" from "the plugin cleaned up after it":

assert_eq!(terminal_releases(&plugin) - before, 2,
    "dropping an allocator must hand back every allocation its caller never freed");

O16 — the enqueue_release failure rollback. No mechanism refused an enqueue, so the rollback path was dead code under test. New refusing-deferred mechanism refuses before touching any state. The binding assertion is deliberately not a counter: a host that unwound only its tallies would have forgotten the address, so the caller's eventual release would be refused as a stray pointer and the bytes stranded with no way to name them.

let outcome = unsafe { allocator.release(ptr, 4096, 64) };
assert!(matches!(outcome, AllocationReleaseOutcome::Complete { .. }),
    "an allocation the plugin refused to take must still be releasable, got {outcome:?}");

O13 — proven equivalent, not a coverage gap

structured_release_slot's guard if self.negotiated_minor < 1 || self.vtable.abi_minor < 1 { return None; } can be deleted with the suite fully green, and no test can close it, because it is unreachable:

Every NxmemAllocatorVtable reaching an AllocatorCore comes from read_prefix or read_prefix_unvalidated (allocator.rs:1520, :1665; read_prefix delegates to the unvalidated form).
Both set effective_minor = declared_minor.min(negotiated_minor) and then if effective_minor < 1 { vtable.release_allocation = None; }.
So vtable.abi_minor <= negotiated_minor, and either disjunct of the host guard implies effective_minor < 1, which implies the slot is already None.

It is defence in depth, and correct as such. I verified this rather than assuming it, by mutating each layer:

O17 (remove the ABI nulling) → red, a_vtable_above_the_negotiated_ceiling_is_clamped_not_refused.
O19 (test plugin publishes the slot regardless of minor) → survives alone; layer 1 still nulls it.
O17 + O19 → still green; the host guard catches it.

O17 + O19 + O13, all three layers → red:

panicked at nxmem_abi.rs:262:
assertion `left == right` failed: a baseline host must not enter a slot it did not
negotiate, whatever the sender happens to publish

So the new assertion is genuinely behavioural — it fires exactly when the behaviour breaks — and I have not claimed a kill I do not have. an_older_host_range_still_drives_the_current_plugin previously asserted its central claim only in a comment ("using the minor-0 deallocate slot rather than structured release"); it now asserts it, via a STRUCTURED_RELEASES counter, that the structured delta is 0 and the terminal delta is 1.

O14 — disclosed as probably equivalent

Removing self.factories.clear() before drop(self) in try_unload survives. I believe this is an equivalent mutant rather than a gap: drop(self) runs Drop for MemoryPlugin::drop (gate check, early return, gate open) and then drops fields including factories, which is the very next thing anyway. The test plugin does not count factory objects in its unload report, so clearing early is observationally identical for this plugin.

I am flagging rather than closing it, because it is not unreachable the way O13 is: it would be a real ordering defect for a plugin that reports factory objects in live_capabilities, and a test for that would be testing a plugin behaviour this plugin does not have. I would rather disclose it than manufacture a mechanism to paper over it.

Test plugin additions

Three new mechanisms — callback-after-drop, reentrant-completion, drip, refusing-deferred (four; MECHANISM_NAMES goes 12 → 16, and every_factory_is_released_exactly_once now derives its count from MECHANISM_NAMES.len() instead of a hardcoded 12) — plus exported counters NxmemTestpluginDrainCalls, NxmemTestpluginTerminalReleases, NxmemTestpluginStructuredReleases, and the hooks NxmemTestpluginReportParkedCompletion / NxmemTestpluginRefuseUnloadReport.

Two new test binaries (nxmem_abi_degraded_report.rs, nxmem_abi_allocator_scoped_pinning.rs). The multi-binary split is unchanged in principle and was extended for the same reason it exists: module counters are statics inside the loaded .so and are process-global, so a mechanism that poisons them needs its own process.

Validation — macOS/arm64, no CUDA

cargo test -p onnx-runtime-memory-abi -p onnx-runtime-memory-host -p onnx-runtime-memory-testplugin

Binary	Baseline `fb9a72c4`	This PR
`onnx_runtime_memory_abi` (lib)	50	51
`header_contract.rs`	4	4
`onnx_runtime_memory_host` (lib)	9	9
`nxmem_abi.rs`	29	36
`nxmem_abi_allocator_scoped_pinning.rs`	—	1 (new)
`nxmem_abi_callback_pinning.rs`	1	1
`nxmem_abi_degraded_report.rs`	—	1 (new)
`nxmem_abi_plugin_gate.rs`	1	1
`nxmem_abi_unload_gate.rs`	1	1
`onnx_runtime_memory_testplugin` (lib)	6	6
Total	101 / 0 failed, 8 binaries	111 / 0 failed, 10 binaries

cargo clippy -p onnx-runtime-memory-abi -p onnx-runtime-memory-host -p onnx-runtime-memory-testplugin --all-targets -- -D warnings — clean.
cargo fmt --check — clean.
cargo doc --no-deps — 0 warnings.
cargo +nightly miri test -p onnx-runtime-memory-abi --lib — 51 passed / 0 failed / 0 ignored (baseline 50/0/0).

Miri scope, stated plainly: Miri cannot execute the dlopen'd cdylib, so every host integration test — including the A2 use-after-free path — is outside it. The only Miri coverage is the abi crate's own --lib tests, which is where the O9 out-of-bounds case lives.

Pre-existing failures elsewhere in the repo (completions.rs rustfmt drift, matmul_nbits_marlin_numerics, Windows CUDA unnecessary_unwrap, macOS MLAS, onnx-runtime-memory-governor rustdoc links) are untouched and unrelated; nothing in these three crates touches CUDA.

Scope

Unchanged, as directed: the read_prefix_unvalidated/read_prefix split, the AbandonOnDrop guard, the try_unload restructure forced by E0509 and its degraded path, bounded-drain-then-leak as a policy, live_views retiring at block removal, the read_prefix clamp design, and the three-way test-binary split.

These crates do not depend on onnx-runtime-memory-governor, so the Phase 1–5 lock graph is untouched by construction. No host lock is held across a call into the plugin — a live concern for the new callback-after-drop path, which re-enters the host; a held lock there would deadlock rather than fail, which is why MidCallObserver uses OnceLock. The three accounting axes remain distinct and no quarantine path refunds.

Criterion 8's execution-provider/context wiring remains unimplemented. It is deliberately out of scope for Phase 6 and tracked separately; onnx-runtime-ep-cuda/src/deferred_release.rs is untouched. This half is not done.

Not for merge — this stack stays open for human review. Please do not enable auto-merge or enqueue.

PR #1448's fixes were correct but five mutations against them survived the suite, because each was verified through a counter the fix increments rather than through the thing the fix does. Deleting a counter turned tests red; deleting the behaviour and keeping the counter did not. No production logic changes. The one production addition is observability: `PluginModule` gains a `Drop` that counts modules reaching `dlclose`, exposed as `PluginModule::modules_unmapped`. Dropping the `library` field *is* the `dlclose`, so this observes the unmap rather than restating the decision to leak — a leaked strong reference shows up as this count not moving. Everything else is test-side: - A1 `Drop for MemoryPlugin` — `modules_unmapped` deltas: 0 across the forced leak paths (measured across `drop(allocator)`, or the still-live Arc masks it), 1 across a clean unload. - A2 callback-table pinning — a `callback-after-drop` mechanism parks the host callback table and reports a completion from a real `std::thread` after the allocator is gone. Bound deterministically by `Arc::strong_count` on the module, since `mem::forget(context)` leaks the bridge's module reference. - B multi-pass drain — a `drip` mechanism retiring one release per call (kills the 16 -> 1 pass bound) and 20 queued releases against `lazy` (kills the unbounded-budget mutation, which survives at 3). - C per-allocator counter — two allocators on one module, a release queued on one only. - D count-before-call — a `reentrant-completion` mechanism plus an observer that reads the counter from inside `enqueue_release`. `fetch_sub` wraps, so the final value is unchanged; only a mid-call read distinguishes them. - E `live_views` — delta assertions over commit, refusal, and retirement. - F `find_cc` prefers `cl` on Windows targets, so an MSVC runner with LLVM on PATH no longer silently checks clang. Also closes four gaps found by mutating code this change did not write: the degraded unload report's `u64::MAX`, the `offset_of!(Self, allocate)` floor in `read_prefix_unvalidated`, the drop-time drain of leaked allocations, and the `enqueue_release` failure rollback. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

codecov · 2026-08-19T15:27:35Z

Codecov Report

❌ Patch coverage is 81.38981% with 782 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.48%. Comparing base (c65cf03) to head (33864d9).

Files with missing lines	Patch %	Lines
crates/onnx-runtime-memory-host/src/allocator.rs	70.79%	313 Missing and 12 partials ⚠️
crates/onnx-runtime-memory-testplugin/src/lib.rs	75.02%	282 Missing and 22 partials ⚠️
crates/onnx-runtime-memory-abi/src/vtable.rs	92.81%	43 Missing and 3 partials ⚠️
crates/onnx-runtime-memory-host/src/loader.rs	88.52%	38 Missing and 4 partials ⚠️
crates/onnx-runtime-memory-abi/src/version.rs	90.73%	25 Missing and 4 partials ⚠️
crates/onnx-runtime-memory-abi/src/status.rs	87.34%	19 Missing and 1 partial ⚠️
crates/onnx-runtime-memory-abi/src/types.rs	96.92%	9 Missing ⚠️
crates/onnx-runtime-memory-abi/src/lib.rs	85.71%	4 Missing and 2 partials ⚠️
crates/onnx-runtime-memory-host/src/error.rs	98.33%	1 Missing ⚠️

Additional details and impacted files

@@                              Coverage Diff                               @@
##           justinchuby-phase-5-process-memory-manager    #1454      +/-   ##
==============================================================================
- Coverage                                       80.01%   79.48%   -0.53%     
==============================================================================
  Files                                             366      375       +9     
  Lines                                          164950   168967    +4017     
  Branches                                       164950   168967    +4017     
==============================================================================
+ Hits                                           131980   134299    +2319     
- Misses                                          28150    29807    +1657     
- Partials                                         4820     4861      +41

Flag	Coverage Δ
mlas	`85.09% <ø> (-0.14%)`	⬇️
offline	`79.37% <81.38%> (-0.54%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
crates/onnx-runtime-memory-host/src/error.rs	`98.33% <98.33%> (ø)`
crates/onnx-runtime-memory-abi/src/lib.rs	`85.71% <85.71%> (ø)`
crates/onnx-runtime-memory-abi/src/types.rs	`96.92% <96.92%> (ø)`
crates/onnx-runtime-memory-abi/src/status.rs	`87.34% <87.34%> (ø)`
crates/onnx-runtime-memory-abi/src/version.rs	`90.73% <90.73%> (ø)`
crates/onnx-runtime-memory-host/src/loader.rs	`88.52% <88.52%> (ø)`
crates/onnx-runtime-memory-abi/src/vtable.rs	`92.81% <92.81%> (ø)`
crates/onnx-runtime-memory-testplugin/src/lib.rs	`75.02% <75.02%> (ø)`
crates/onnx-runtime-memory-host/src/allocator.rs	`70.79% <70.79%> (ø)`

... and 15 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2026-08-19T16:58:43Z

🔴 Benchmark Regression Detected

Comparison of criterion micro-benchmarks: PR head vs merge-base, measured on the same runner in the same job (base first → PR second).

ℹ️ Absolute times are informational only — they vary with runner load. The % change column is the reliable signal because both sides ran under identical conditions.

Status	Scenario	Base	PR	Change
🔴	`gather/large_f32_threads=1-internal/131072`	21.52 µs	39.18 µs	+82.1%
🔴	`add/large_f32_threads=1-internal/4194304`	564.21 µs	849.50 µs	+50.6%
🔴	`gather/large_f16_threads=1-internal/131072`	12.12 µs	17.29 µs	+42.7%
🔴	`block_quantized_matmul_cached_dense/mxfp4_preexpanded_dense_oncelock_like_proxy/1x1024x1024`	53.01 µs	73.92 µs	+39.4%
🔴	`block_quantized_matmul_cached_dense/mxfp4_cached_dense_repeated_call/1x1024x1024`	50.65 µs	69.98 µs	+38.2%
🔴	`gather/medium_f32_threads=1-internal/32768`	3.47 µs	4.64 µs	+33.8%
⚠️	`add/small_bf16_threads=1-internal/1024`	410.6 ns	512.3 ns	+24.8%
⚠️	`matmul/medium_generic_bf16_threads=8/32x512x512`	437.71 µs	543.43 µs	+24.2%
⚠️	`matmul/small_generic_bf16_threads=8/1x256x256`	34.21 µs	40.95 µs	+19.7%
⚠️	`gather/medium_bf16_threads=1-internal/32768`	2.23 µs	2.58 µs	+15.6%
✅	`gather/small_f16_threads=1-internal/4096`	438.8 ns	495.9 ns	+13.0%
✅	`reduce_mean/medium_f32_threads=1-internal/65536`	232.39 µs	261.89 µs	+12.7%
✅	`gather/small_f32_threads=1-internal/4096`	617.5 ns	695.9 ns	+12.7%
✅	`add/small_f16_threads=1-internal/1024`	419.6 ns	471.9 ns	+12.4%
✅	`matmul/large_generic_f32_threads=1/32x1024x1024`	9.92 ms	11.00 ms	+10.9%
✅	`reduce_mean/large_f32_threads=1-internal/262144`	940.13 µs	1.03 ms	+9.9%
✅	`add/large_f16_threads=1-internal/4194304`	1.64 ms	1.80 ms	+9.6%
✅	`add/large_bf16_threads=1-internal/4194304`	1.65 ms	1.80 ms	+9.2%
✅	`gather/medium_f16_threads=1-internal/32768`	2.48 µs	2.67 µs	+7.6%
✅	`add/small_f32_threads=1-internal/1024`	199.4 ns	212.0 ns	+6.3%
✅	`add/medium_f32_threads=1-internal/262144`	25.34 µs	26.82 µs	+5.8%
✅	`matmul/small_generic_f16_threads=8/1x256x256`	34.43 µs	36.42 µs	+5.8%
✅	`gather/small_bf16_threads=1-internal/4096`	514.5 ns	543.2 ns	+5.6%
✅	`reduce_mean/small_f32_threads=1-internal/4096`	15.53 µs	16.23 µs	+4.5%
✅	`matmul/small_generic_bf16_threads=1/1x256x256`	35.88 µs	37.08 µs	+3.4%
✅	`add/medium_bf16_threads=1-internal/262144`	105.55 µs	108.81 µs	+3.1%
✅	`block_quantized_moe_cached_dense/mxfp4_uncached_expert_dequant_each_call/rows=1,H=256,I=256,E=4,top_k=1`	402.42 µs	407.39 µs	+1.2%
✅	`qwen3_sampling_processors/top_k_top_p_fast`	691.24 µs	695.13 µs	+0.6%
✅	`matmul/medium_generic_f32_threads=1/32x512x512`	2.47 ms	2.48 ms	+0.2%
✅	`logit_processing/seven_processor_chain_per_step`	332.58 µs	325.64 µs	-2.1%
✅	`matmul/small_generic_f16_threads=1/1x256x256`	36.25 µs	35.26 µs	-2.8%
✅	`kv_cache/alloc_dealloc_pages`	40.74 µs	39.31 µs	-3.5%
✅	`qwen3_sampling_processors/top_k_full_sort_baseline`	2.22 ms	2.14 ms	-3.7%
✅	`matmul/large_generic_bf16_threads=1/32x1024x1024`	2.08 ms	2.00 ms	-3.8%
✅	`sampling_latency/top_k_per_token`	53.04 µs	50.52 µs	-4.8%
✅	`matmul/large_generic_f16_threads=8/32x1024x1024`	91.40 µs	86.83 µs	-5.0%
✅	`qwen3_sampling_processors/top_p_fast_after_top_k`	545.59 µs	516.65 µs	-5.3%
✅	`sampling_latency/top_p_per_token`	400.58 µs	379.30 µs	-5.3%
✅	`grammar_masking/llguidance_compute_mask/32`	91.60 µs	85.53 µs	-6.6%
✅	`qwen3_sampling_processors/top_k_top_p_full_sort_baseline`	5.97 ms	5.56 ms	-6.8%
✅	`sampling_latency/greedy_per_token`	3.39 µs	3.15 µs	-7.1%
✅	`qwen3_sampling_processors/top_p_full_sort_after_top_k_baseline`	3.70 ms	3.42 ms	-7.4%
✅	`tokenization/decode_tokens_per_second`	6.76 ms	6.25 ms	-7.6%
✅	`matmul/medium_generic_f16_threads=1/32x512x512`	41.05 µs	37.39 µs	-8.9%
✅	`block_quantized_moe_cached_dense/mxfp4_cached_dense_expert_repeated_call/rows=1,H=256,I=256,E=4,top_k=1`	97.74 µs	88.52 µs	-9.4%
✅	`sampling_latency/min_p_per_token`	228.28 µs	206.36 µs	-9.6%
✅	`qwen3_sampling_processors/top_k_partial_selection`	151.95 µs	134.14 µs	-11.7%
✅	`matmul/large_generic_f32_threads=8/32x1024x1024`	4.45 ms	3.85 ms	-13.4%
🟢	`gather/large_bf16_threads=1-internal/131072`	16.97 µs	14.38 µs	-15.2%
🟢	`block_quantized_matmul_cached_dense/mxfp4_uncached_dequant_each_call/1x1024x1024`	689.40 µs	579.72 µs	-15.9%
🟢	`matmul/medium_generic_f32_threads=8/32x512x512`	1.36 ms	1.14 ms	-16.0%
🟢	`matmul/large_generic_bf16_threads=8/32x1024x1024`	1.76 ms	1.43 ms	-18.7%
🟢	`add/medium_f16_threads=1-internal/262144`	138.52 µs	108.36 µs	-21.8%
🟢	`tokenization/encode_tokens_per_second`	488.43 µs	380.87 µs	-22.0%
🟢	`matmul/medium_generic_bf16_threads=1/32x512x512`	731.99 µs	559.55 µs	-23.6%
🟢	`matmul/large_generic_f16_threads=1/32x1024x1024`	102.86 µs	75.80 µs	-26.3%
🟢	`matmul/small_generic_f32_threads=1/1x256x256`	52.33 µs	37.08 µs	-29.1%
🟢	`matmul/small_generic_f32_threads=8/1x256x256`	56.12 µs	39.67 µs	-29.3%
🟢	`matmul/medium_generic_f16_threads=8/32x512x512`	48.18 µs	32.08 µs	-33.4%

Visual flags: ⚠️ ≥ 15% slower, 🔴 ≥ 30% slower — calibrated against measured runner noise (~27% worst-case on multi-threaded matmul)

Host info

CPU: Apple M1 (Virtual)
Cores: 3
OS: Darwin 25.5.0 arm64
Rust: rustc 1.97.1 (8bab26f4f 2026-07-14)
Load avg: { 3.38 3.28 5.65 }

What this cannot catch

Regressions in code paths not covered by these benchmarks (e.g., end-to-end decode with a real model)
Sub-threshold regressions that compound over multiple PRs
Performance changes that only manifest under GPU execution
Latency changes in the ORT integration path (these benchmarks exercise the native Rust kernels)

> **A100 safety update (head `13c37a7f`):** expected shared-prefix admission failures are now all-or-private: K/V commit is transactional, a failed V share rolls K back before enqueue, and only a successful rollback permits private-KV fallback. Fatal kernel faults remain process-fatal; the new cross-process VMM probe proves a worker fault does not affect an actively executing peer, while the current server is still in-process and does not yet consume physical shared-prefix metadata. See the [final verified conclusion](#1579 (comment)). Collapses phases 1–7 of the #1186 memory architecture rework onto current `main` as a single merge. The stack forked 270 commits ago (176 of them touching `crates/`), so rebasing it layer by layer would mean solving the same 17 conflicts fourteen times, with no reviewer ever looking at the intermediate states. Closes the stack: #1252 #1263 #1279 #1283 #1301 #1341 #1349 #1426 #1440 #1448 #1454 #1462 #1465 #1468 #1533. **A review guide is in the first comment.** It is the part worth reading — this diff is 103 files, but only three decisions in it are ones a compiler cannot check. ## What was already reviewed, and what wasn't Every phase was reviewed and approved by a session that was not its author, under the rejection-lockout rule (a rejected author never writes the next revision). | Phase | PR | Verdict | |---|---|---| | 1–5 | #1252 → #1426 | approved | | 6 | #1440, #1448, #1454 | **rejected** ×3 | | 6 | #1462 | approved | | 7 | #1465 | **rejected** | | 7 | #1468 | approved | | 7 test fixes | #1533 | approved, A100-verified | The four rejected rounds were not replaced — the later rounds are stacked **on top of** them. So the tree here is the approved state, but the history contains the rejected commits. **#1440 / #1448 / #1454 / #1465 should not be reviewed individually**; they close automatically. **Not reviewed anywhere:** the conflict resolutions themselves. That is what this PR is for. ## Verification Run here, on this merged tree: - `cargo check --workspace --all-targets` — clean - `cargo check -p onnx-genai-engine --features cuda,native-backend --all-targets` — clean. Worth calling out: the default feature set does **not** compile the `cfg(cuda)` code, which is where the riskiest edits are. Checking only the default set would have missed a real break (see the guide). - `cargo test` over 7 crates — **1105 passed, 2 failed, 88 ignored** - `cargo clippy --workspace --all-targets` - `cargo fmt` **The 2 failures and the 1 clippy error are pre-existing and proven so, not merge damage:** - `platform_capacity::{disk_capacity_is_measured_for_the_working_directory, an_explicit_byte_limit_is_honored_without_a_device_query}` — `platform_capacity.rs` is byte-identical to `main` (`git diff origin/main HEAD -- ` that file is empty). The cause is a macOS-only FFI layout bug: `fsblkcnt_t` is 4 bytes on macOS (confirmed: `sizeof(struct statvfs)=64`, `sizeof(fsblkcnt_t)=4`) while the Rust struct declares `f_blocks`/`f_bfree`/`f_bavail` as `u64`. CI is Linux, where it is 8 bytes. Left alone: it is a real bug but not this PR's. - `optimizer.rs` clippy `approx_constant` — present on both parents; the merged file is byte-identical to `main`. - Three files had `rustfmt` drift already present on `main` (`matmul_nbits.rs`, `normalization.rs`, `optimizer.rs`). Reverted rather than swept in, to keep the diff readable. **Final A100 revalidation (head `13c37a7f`):** full CUDA-memory GPU suite passed; CUDA EP default-parallel lib suite passed (**488 passed / 17 ignored**); real fp16 GQA covered three requests × two interleaved decode steps with two shared peers and one transactional private fallback, byte-identical to independent GPU and CPU references. A device-started/event-gated worker remained healthy through a peer process `CUDA_ERROR_ILLEGAL_ADDRESS`, owner exit, replacement worker, and process restart. Governor tests, targeted Miri, CI Clippy, fmt, and CUDA honesty also passed. ## Still open after this merges - `memory-plugin-provider-wiring` — the back half of Phase 6 criterion 8 (provider/context pinning), which fell in the gap between phases. **#1186 must not be closed until it lands.** This merge makes it more tractable, not less: see decision 3 in the guide. - `memory-deferred-invariant-asserts` — two surviving mutants found by #1462's reviewer, adjudicated NON-BLOCKING and not defects. - #1533's CUDA honesty guard warns on a legitimately portable anchor. Should be moved out of the CUDA test binary or allow-listed rather than left warning. --------- Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <copilot@github.com> Copilot-Session: 46c5d75b-8146-489c-b82f-08ee29c27ce4 Copilot-Session: 39ff6824-d35f-4d3b-8f5f-043a7119a100 Copilot-Session: c80f8522-983c-47f7-8241-2155a823aabe

justinchuby · 2026-08-22T02:15:24Z

Closing: superseded by #1579, which collapsed phases 1–7 into a single merge — landed as a36964280 (squash, 126 files, +43956 −3286).

This PR was rejected under the lockout rule and superseded by the approved revision that was stacked on top of it (not by a replacement of it). Per #1579's own guidance, it was never meant to be reviewed individually.

Containment verified, not assumed:

$ git merge-base --is-ancestor 33864d94a ebceab071   # ebceab071 = #1579 head
→ ancestor

Every one of the 15 stack heads (#1252 → #1533) is a literal ancestor of the merged head, and I separately confirmed the merged head's content reached main: of the 126 files #1579 touched, exactly one differs from main — crates/onnx-runtime-session/src/executor/tests.rs, where main carries 120 extra lines from #1703 (Expand-broadcast mask tests). That is the conflict resolution correctly preserving the other side, not content loss.

Why this didn't close itself: #1579's body says Closes the stack: #1252 #1263 …, which GitHub does not parse — the closing keyword must be immediately followed by the reference, and in any case closing keywords only ever close issues, never pull requests. So all 15 stayed open by mechanism, not by intent.

#1186 stays open: #1579 explicitly gates it on memory-plugin-provider-wiring (the back half of Phase 6 criterion 8, provider/context pinning), which has not landed.

This was referenced Aug 19, 2026

test(memory): anchor the dlclose proxy and the negotiated-level premise (Phase 6) #1462

Closed

CUDA 机器验证计划:#1186 内存重构 Phase 1–7 全栈测试指引 #1474

Open

justinchuby changed the base branch from justinchuby-phase-5-process-memory-manager to justinchuby-memory-phase-6-revision August 20, 2026 14:53

justinchuby mentioned this pull request Aug 20, 2026

Merge the #1186 memory stack (phases 1-7) onto main #1579

Merged

justinchuby closed this Aug 22, 2026

Navigation Menu

Repository navigation

nxmem: bind the Phase 6 fixes to behaviour, not to their own counters - #1454

nxmem: bind the Phase 6 fixes to behaviour, not to their own counters#1454
justinchuby wants to merge 1 commit into
justinchuby-memory-phase-6-revisionfrom
justinchuby-memory-phase-6-test-hardening

justinchuby commented Aug 19, 2026

Uh oh!

codecov Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 19, 2026

Uh oh!

justinchuby commented Aug 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

justinchuby commented Aug 19, 2026

The one production change

Findings

A1 (High) — Drop for MemoryPlugin self-retention

A2 (High) — AllocatorCore::drop callback-table pinning

B (Medium) — multi-pass drain loop

C (Medium) — per-allocator outstanding_releases

D (Low) — count-before-call ordering

E (Medium) — live_views

F (Low) — MSVC probe order

Mutations I invented against code I did not write

The four that survived, and what closed them

O13 — proven equivalent, not a coverage gap

O14 — disclosed as probably equivalent

Test plugin additions

Validation — macOS/arm64, no CUDA

Scope

Uh oh!

codecov Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Aug 19, 2026

🔴 Benchmark Regression Detected

Uh oh!

justinchuby commented Aug 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

A1 (High) — `Drop for MemoryPlugin` self-retention

A2 (High) — `AllocatorCore::drop` callback-table pinning

C (Medium) — per-allocator `outstanding_releases`

E (Medium) — `live_views`

codecov Bot commented Aug 19, 2026 •

edited

Loading