Repository navigation
perf(ir): keep is_dense off the heap for ordinary ranks - #1433
Merged
Merged
Conversation
Issue #1077 asks why our per-Run dispatch overhead is higher than ORT's. Answering that repeatedly has meant hand-editing `Instant` probes into the hot path, measuring, and tearing them out — which is slow, unreviewable, and leaves nothing behind that would catch a regression. This adds `dispatch_probe`, a counter and phase-timing module gated behind a non-default Cargo feature, and wires it through the CPU dispatch path: ORT callback entry, input metadata queries, tensor binding, output allocation, dispatch lookup, kernel invocation, and status/ABI crossing. It also tallies FFI round trips, heap allocations and `OrtStatus` creations, which are the three things actually worth removing. The counters are kept twice. The thread-local copy is precise, and is what makes exact-count assertions possible without a neighbouring test bleeding into the measurement. The global mirror exists because ORT picks the thread that runs `Compute`, and the e2e harness reading these numbers back over the C ABI is not necessarily on it; a thread-local-only probe would report zero there and read as "we made no FFI calls" rather than "you are on the wrong thread". Without the feature the whole thing is a `Drop`-less zero-sized guard and a set of `#[inline(always)]` empty functions, so production builds pay nothing. `probe_is_compiled_out_in_production` asserts that directly rather than trusting it. Timing is gated a second time, on `ONNX_GENAI_PROFILE_DISPATCH=1`, because a test asserting call counts does not want two `Instant::now()` calls added to every phase it measures. The counts themselves are pinned by a hand-built `OrtApi` in `kernel_ctx`, which answers just enough of the C API to run `read_inputs` without a live ONNX Runtime. Reading one input costs eight round trips and five allocations today; three inputs cost 1 + 3x7 and 1 + 3x4, so the model is linear rather than a single data point. Any change that adds a round trip or an allocation to the per-Run path now has to come here and edit a number in a diff a reviewer can see. Two of the five allocations are already visibly waste: the `format!("input {i}")` label is built eagerly on every input even when validation succeeds, and `compute_execute` formats a staging log line unconditionally before the function that early-returns when logging is off. Those are removed in a follow-up, which this change exists to measure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`probe_is_compiled_out_in_production` compared `snapshot()` after some counting against `snapshot()` before it. Both are calls to the same constant function in a disabled build, so the assertion held no matter what the probe did — verified by mutation: a version whose disabled `count_n` recorded into a static and whose `snapshot` returned a non-zero `Counters` passed the test unchanged. Compare against `Counters::default()` instead, which is an absolute claim rather than a self-consistency one, and fails against that mutation. Also assert `!needs_drop::<PhaseGuard>()`, so a `Drop` impl added to the production guard — the one way it could regain a cost while staying zero-sized — is caught. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… one Review of #1387 found the module's central claim was false. It documented `OrtFfiCall` as "how many ORT FFI calls a Run made" and called it the headline number for #1077, but only `read_inputs` and `allocate_output` were instrumented. On the happy path of a single-node Run, `device_mem_info`, `ort_input_mem_info`, `mem_info_is_device`, `alloc_scratch`, `host_pool::install` and `CreateStatus` all reach into ORT and none of them were counted. A probe that under-counts while presenting itself as exact is worse than no probe: it invites the reader to conclude the dispatch path is cheaper than it is, which is precisely the conclusion #1077 exists to prevent. Every leaf call through an `OrtApi` function pointer in this crate is now counted. Rather than assert that in a comment, `ffi_coverage` enforces it: it scans the source of each file that reaches into `OrtApi`, extracts the API members it names — ORT's C API is CamelCase and our own fields are snake_case, so `.CamelCase` isolates them cleanly — and fails if that set or the instrumentation count moves. Adding an uninstrumented call now fails with the member's name in the message. Verified by mutation: adding a stray `GetTensorShapeElementCount` reference failed with exactly that. A second test pins that the scan finds real members, so the guard cannot degrade into a heuristic that matches nothing and passes forever. `DispatchAlloc` gets the opposite treatment, because hand-placed counts genuinely cannot be exhaustive: `Vec::new()` does not allocate, `Vec::with_capacity(0)` does not allocate, and whether a `collect` allocates once or twice is a property of `size_hint` rather than of anything visible at the call site. It is now documented as a lower bound that is exact only for `read_inputs`, and callers who need the true whole-Run figure get `CountingAllocator`, a `GlobalAlloc` wrapper they can install in a test or benchmark. This crate deliberately does not install it — a library that defines a `#[global_allocator]` takes that choice away from every dependent. Also from review: - A node with no inputs was charged one allocation for a `Vec::with_capacity(0)` that never happens. Now conditional, and pinned by a test. - The phase timings are documented as what they are: non-exhaustive on the success path, and nesting on the error path, where `StatusCrossing` opens inside whichever phase was live because a guard closes at scope exit rather than at the early return. `DispatchLookup` is entered twice per node and reports the total. Summing them does not yield Run time in either direction, and a reader who assumed it did would be misled. - `probe_is_compiled_out_in_production` cannot prove `count` has no side effect — a disabled build has no storage to observe one — so it no longer implies it does. What it can prove, it does: the guard is a ZST and does not need dropping. Validation: clippy clean and `--lib` green in both feature configurations (253 without, 260 with), and 55/55 `plugin_ort_e2e` against real ORT. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Stacked on #1387 (base `squad/resch-dispatch-probe`). The instrumentation there exists to make changes like this one provable; this is the first of them. ## Why Two heap allocations sat on the per-`Run` dispatch path purely to serve diagnostics that are switched off. **1. `staging_log(&format!(…))`.** `staging_log` returns immediately when the transfer trace is disabled — but the argument is evaluated first. Every one of these builds a `String` and formats each interpolated value on the way to a function that drops it on the floor. One of them is in `compute_execute`, before any real work happens: ```rust staging_log(&format!( "[plugin/staging #982] compute_execute enter: entries={} routing={} device_staging={}", exported.entries.len(), exported.routing.is_some(), exported.device_staging.is_some() )); ``` So every dispatch in every production run has been paying for a debug line nobody asked for. **2. `format!("input {i}")` in `read_inputs`.** Built for every input on every `Run`, to label an error that is almost never raised. ## What A `staging_log!` macro that checks `staging_trace_enabled()` *before* formatting, so a disabled trace costs one relaxed `OnceLock` load and nothing else. Thirteen call sites convert. `validate_dims` now takes `impl Display` instead of `&str`, so the caller passes `format_args!("input {i}")` — which borrows its arguments and formats only if a message is actually produced. Same treatment for `transfer.rs`'s `CopyTensors[{i}]`. ## Evidence The pinned counts in `dispatch_cost` (added by #1387) move: | | before | after | |---|---|---| | allocations, 1 input | 5 | **4** | | allocations, 3 inputs | 1 + 3×4 | **1 + 3×3** | That is the instrumentation earning its keep. The improvement is a number changing in a test — reproducible on any machine, by anyone, at any time — rather than a claim resting on a benchmark. That matters here in particular: the box is currently shared with other work and sitting at ~74% idle against my ≥93% gate, so a wall-clock A/B taken right now would be noise. I will post interleaved p50/p90 once the machine quiesces, but the correctness of the change does not depend on it. ## Correctness - **The trace still works.** Ran the e2e suite with `ONNX_GENAI_PLUGIN_TRANSFER_TRACE=1` and confirmed the `compute_execute enter` line still appears. A macro that silently disabled the trace would otherwise be indistinguishable from one that made it cheap. - **No side-effecting expression moved inside a now-conditional macro.** I checked every converted site; the only mutation in the neighbourhood (`to_stage.push(p)`) is outside the macro call. - **Emitted text is unchanged** — the format strings and arguments are copied verbatim, only the gate moved. - clippy clean and `--lib` green in both feature configurations (253 / 260), and **55/55 `plugin_ort_e2e`** against real ORT. Relates to #1077. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…uted-loop fixes (2.31 -> 1.71 at depth 100) (#1397) > **Note on scope.** Enabling auto-merge across the stack caused each stacked PR to merge into its *parent feature branch* (those branches are unprotected; only `main` requires checks), and GitHub retargeted the children. No `main` protection was bypassed and no work was lost, but this PR now carries the whole dispatch-overhead stack below #1387, not just the memory-info change: > > - #1394 — the #1077 benchmark grid > - **this PR** — `device_mem_info`, 7 → 4 FFI calls per `Run` > - #1401 — routed-loop instrumentation, the O(depth²) retirement scan, lazy absent strides, and the 8-element dispatch-isolating cases > > Each was independently reviewed before it merged; the sections below cover the memory-info change, and #1394/#1401 retain their own descriptions and review records. #1387 (probe + lazy formatting) remains a separate PR to `main`. ## Stop asking ORT three times where the inputs live Part of #1077. Stacked on #1394 (which measured the cost this removes). ### The finding Every `Compute` call resolves where a routed subgraph's intermediate tensors should be allocated. On a host EP — and the CPU plugin is always a host EP — that resolution cost **7 FFI calls** for a single-input node to arrive at the answer "host memory". Three of them were asking ORT for something already in hand. `device_mem_info` scans the inputs looking for a device-resident one: 1. per input: `GetValue` → `GetTensorMemoryInfo` → `mem_info_is_device` 2. on a host EP it never finds one, so the scan runs to completion 3. it then falls through to "use input 0's memory info" — and **fetches input 0 again**, two more calls for a value it had already looked at and discarded 4. the caller then asked `mem_info_is_device` on the result — a **third** query for a fact the function had just established internally and thrown away ### The change The scan remembers input 0's memory info as it walks, and returns `(mem_info, is_device)` so the caller does not have to re-derive the second half. The fallback fetch stays, for the one case that genuinely needs it: no input count available, so the scan never ran and nothing was remembered. ### Why this is behaviour-identical - The scan visits the same inputs, in the same order, and returns on the same condition. - The remembered value is exactly what the re-fetch would have produced: both are input 0's `OrtMemoryInfo`, and it is immutable for the lifetime of the value. - The `recon_mem_info` branch reports `is_device = true` without asking. That branch is only reachable when the scan found a device-resident input, so it is device-resident by construction — which is precisely what the removed call was confirming. ### Evidence `mem_info_cost` (new, in `compute.rs`) drives `device_mem_info` through a hand-built `OrtApi` and pins the call count. | scenario | before | after | |---|---|---| | host EP, 1 input | **7** | **4** | | device-resident input (short-circuit) | 3 | 3 | | no input count (fallback path) | 2 | 2 | | zero inputs | 0 | 0 | The "before" column is not a recollection — it is the same test run against the previous implementation. Three FFI calls per `Run`, off the fixed per-`Run` cost that #1394 measures at **~0.9 µs worse than ORT's**. ### Why counters and not a stopwatch This machine has not been quiet enough all session for a trustworthy wall-clock A/B (`mpstat` shows cores at 75–78% idle against a ≥93% gate). An FFI-call count is deterministic and load-independent, so it is reproducible now and will still be reproducible on someone else's machine. That is the case for #1387 existing. ### Also pinned The three non-host paths, so a future change cannot quietly break them while the host count stays pretty: device-resident short-circuit, missing input count, and a node with no inputs (reports no memory info rather than inventing one). ### Validation - `cargo test -p onnx-runtime-ep-plugin --lib` — 253 pass (debug; `--release` silently disables `debug_assert!`) - same with `--features dispatch_probe` — 264 pass - `plugin_ort_e2e` — **55/55**, including `every_assigned_node_is_also_executed_by_this_ep` - clippy clean in both feature configurations No ORT CPU fallback. No MLAS. --- ## Independent review (Opus 4.8, adversarial) — **APPROVE WITH NITS**, all findings addressed Commissioned specifically to attack "behaviour is unchanged in every case". It held for every EP that ships, with one crack worth fixing. **Finding 1 (MINOR) — the recon branch's `is_device = true` was convention, not construction.** Upheld, and fixed properly. My comment claimed the reconstruction is "device-resident by construction". The reviewer traced it: `recon_mem_info` is built from the EP's reported `device_type`, and `device_staging` is attached whenever an EP supplies a `HostToDeviceCopier` — and **nothing in that trait requires a copier-providing EP to be non-CPU**. Every one today is (CUDA→GPU, QNN→NPU), which is exactly what made the hardcode look safe. A CPU-typed EP that grew a copier would build a host-typed memory info and the hardcode would hand it to the device scratch path — host pointers treated as device pointers, the failure this code exists to prevent, inverted. The old code had no such exposure because it asked ORT. Rather than paper over it with a `debug_assert`, the invariant is now structural. `mem_info_is_device` is exactly `device_type != CPU`, and the reconstruction is created by passing that same `device_type` to `CreateMemoryInfo_V2` — so recording it at construction reproduces the old query **in every case, including the one that does not exist yet**, and still costs no FFI call. **Finding 2 (MINOR) — the one branch whose reporting changed was untested.** Correct: all four original tests passed `staging = None` and never reached it. Three tests added — CPU-typed reconstruction is not reported as a device, device-typed one is (4 calls, not 5), and a real device input still wins over the fallback. Falsified: restoring the hardcoded `true` fails **exactly one** test, the one naming the case; the other six correctly stay silent. **Finding 3 (NIT) — test 2's `== 4` does not distinguish new from old.** Accurate. The device-input path cost 4 before as well; the real guard in that test is the tuple value, not the count. Left as-is (it pins the device path against future regressions), noted here so nobody reads it as evidence for this PR. ### Claims the reviewer tried to break and could not These are now load-bearing rather than merely unchallenged: - **Pointer stability** — the remembered input-0 memory info cannot differ from what the re-fetch produced: same `OrtValue`, straight-line code with no intervening mutation, and the pointer's lifetime window is identical in both versions. - **The fallback path does not hardcode anything** — it still queries, so a device-resident input on that path still reports `true`, exactly as before. - **Zero-input nodes** — identical (`None`, not an invented memory info). - **Caller equivalence** — `Option<(*const _, bool)>` is `Copy`; no move, no shadowing, no ordering change. - **`mem::zeroed::<OrtApi>()`** — sound; all fields are `Option<extern fn>`, zero is the null-fn niche, and the sentinel pointers are never dereferenced. - **The `ffi_coverage` tripwire stays honest** — 9 members / 9 `ort_call()` sites, unchanged and correct: this PR removes *dynamic* calls, not *static* sites. - **Concurrency** — entirely stack-local; no new shared mutable state. - **No vacuous test** — the reviewer was briefed that I have twice shipped a test that passed against the broken version, and specifically hunted a third. None found. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Issue #1077 has ~7 allocations per node left and no way to say where they are. This adds per-phase allocation attribution and gets it reporting from a live routed dispatch. The allocator hook charges each allocation to the innermost open phase. PhaseGuard now saves the enclosing phase on enter and restores it on drop, so nested phases -- which the dispatch path really does produce, because a guard closes at scope exit rather than at an early return -- attribute to the inner one rather than losing the outer. Reading it back needs the counters to come from the library ORT dlopens, not from the test binary: the cdylib has its own global allocator, so an allocator installed in the harness never sees an allocation made inside Compute. The counting allocator is therefore installed in the cdylib under a new dispatch_probe feature, and the harness reads totals through the existing C export. Also fixes a resolver bug this uncovered. cdylib_resolve rebuilds the cdylib with a fixed feature list, and dispatch_probe was missing from it, so the rebuild silently replaced the instrumented library with a default-feature one and the probe reported zeros for every phase. The file already warned about this hazard for mlas. Nothing fails loudly when a feature is omitted, so the comment now says so explicitly. Production is unaffected: without the feature record_alloc is an inline no-op, no global allocator is installed, and the harness path is additionally gated on NXRT_MM_BENCH_PROBE=1. Falsified: reverting the outer-phase restore fails exactly the two nesting tests; the five attribution tests install the counting allocator for real rather than calling record_alloc by hand. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ing them The first live attribution run said four allocations per node and gave no way to tell whether that was all of them. It was a third of them. Allocations made with no phase open were being discarded rather than counted, so the per-phase rows could not be checked against any total. They now land in an explicit UNATTRIBUTED bucket, which makes the columns add up: 12.11 allocations per node on a 100-node chain, of which 4 are inside named phases and 8.03 are in the routed loop between them. The hand-placed DispatchAlloc tally independently reads 12.19, which is the cross-check that this is now complete. The harness also kept its own copy of the phase order, and the copy was wrong: it labelled TensorBind's allocations "OutputMeta" and Allocate's "TensorBind". Every number in that table was correct and attached to the wrong phase, which is the kind of error that survives review. The names now come from the library through a new nxrt_dispatch_probe_phase_name export, so there is one source of truth, and a test pins the export against Phase::name. Falsified: renaming one exported phase fails the pinning test; reverting the unattributed bucket fails the outside-a-phase test, which now asserts the allocation is visible rather than absent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The allocation table said 12.11 allocations per node, and only 4 of them were inside named dispatch phases. The other 8 were these six vectors -- ort_outputs, buf_writes, absent_scratch, slot_kinds, new_bufs and node_ort_operands -- being created and dropped once per node, plus a shape clone. They are hoisted above the loop and cleared at the top of each iteration, so node N reuses node N-1's capacity. Clearing at entry rather than at exit means no node can observe another node's contents even on an early return. buf_writes no longer clones the output shape. It stores the output slot instead and indexes output_shapes, which is already live for the whole iteration; the clone was one more allocation per buffer-bound output for data that already existed. Measured on the 8-element Relu chain at depth 100, 1 thread: allocations per node 12.11 -> 8.17 bytes per Run 102288 -> 45792 ratio vs ORT CPU 1.713 -> 1.416 per node 0.39us -> 0.31us (ORT 0.22us) Depth 1 is unchanged at 1.517, which is the expected signature: this touches per-node cost only, not the fixed per-Run floor. Dynamic shapes, optional/absent slots, concurrency and retained outputs are unaffected -- the vectors were already per-node scratch with no cross-node meaning. 259 lib tests and all 55 e2e conformance tests pass, including every_assigned_node_is_also_executed_by_this_ep. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The plan cache promotes every hit to the front so a hot signature stays ahead of a one-off prefill shape once the cache is full. In the steady state -- one shape, every node, every Run -- the hit is already at the front, so the promotion is identity, but remove + insert still memmoves the whole vector twice to achieve nothing. Guarding the promotion on idx != 0 costs one compare and removes both memmoves on the common path. Callgrind on the 100-node 8-element Relu chain, differenced by iteration count so one-time partitioning cancels exactly: lookup drops from 141 to 101 instructions per node, -28%. The absolute counter is bit-reproducible across three independent runs, and a null control (identical binary, two runs) moves it by zero. Move-to-front was entirely untested before this change: deleting the promotion outright passed all 259 tests. Added a falsifier that drives the cache past capacity while re-dispatching one signature and asserts on observed planning calls; it fails both with the promotion deleted and with the new guard forced always-false. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The docstring claims to be a move-to-front falsifier, but the loop ran to CAPACITY - 1, which with the hot signature fills the cache to exactly capacity and never evicts. It therefore passed with or without the promotion and proved nothing -- one of the two reasons move-to-front went untested. Flood past capacity so eviction actually happens. Mutation-verified: deleting the promotion now fails this test as well as the new one, where before it only failed the new one. Also correct the mechanism described in the lookup comment. In the profiled benchmark the cache holds a single entry, so the instructions the guard removes are the call, bounds checks and length bookkeeping of remove + insert, not a multi-element move. The measurement quoted is unchanged; only the explanation of it was imprecise. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
is_dense collected the non-trivial (stride, size) extents into a Vec on every call. It runs once per operand per node on the CPU plugin dispatch path, so a rank-1 tensor of eight floats paid a malloc/free pair to have four numbers inspected. perf sampling of the 100-node 8-element Relu chain, our EP side alone, steady state: is_dense and the Vec it built together accounted for 5.25% of dispatch time (3.32% in the collect, 1.93% in the function), against roughly 3% for the elementwise arithmetic the whole graph exists to do. Rank <= 8 covers every tensor ONNX produces in practice, so those extents now live on the stack. Higher ranks keep the heap path rather than impose a limit the signature does not have. Both paths hand the same slice to the same routine, so there is exactly one implementation of the predicate and the two cannot drift. Paired A/B on the same host and session, three interleaved repetitions, 100-node 8-element Relu chain, one thread, CPU fallback disabled: control ours p50 0.0324 / 0.0330 / 0.0329 ms ratio 1.367 / 1.375 / 1.368 with fix ours p50 0.0307 / 0.0307 / 0.0309 ms ratio 1.261 / 1.299 / 1.316 -6.4% on our side, non-overlapping across repetitions and well clear of this host's ~4% wall-clock noise floor. The new differential test runs the pre-optimisation implementation as an oracle over ranks 0..=10 -- straddling the inline bound in both directions -- with contiguous, permuted, zero-sized, negative-strided and length-mismatched cases, and asserts the corpus actually reached the dense arm rather than degenerating into rejections. Mutation-verified: it fails if the inline array is not truncated to the filled length, if the d > 1 filter is dropped, or if the stride is not taken as absolute. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Independent review found the `checked_dense > 100` guard was satisfied four times over by rank-0 shapes alone. Those return true from the empty-extents early exit without reaching the sort-and-product logic the guard names, so it asserted nothing about that arm. Count only cases with a dimension above 1: the corpus supplies 836 of them. Add a deterministic test for a rank of exactly INLINE_RANK with every dimension non-trivial. That is the single input where the filled length equals the whole stack array, so the truncation to `..len` is a no-op and the corresponding mutation cannot be caught -- it was reached once by chance in the random corpus and should not depend on the seed. It also covers dense-but-not-row-major, which the corpus only reaches by accident. Record that the smallest-stride check is redundant. Mutation testing shows removing it changes no result, because the loop's first iteration makes the same comparison. Kept for intent, now labelled so a reader does not take it for load-bearing. Fix the length-mismatch comment: at rank 0 popping a stride leaves two empty slices, which is not a mismatch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
is_contiguous built the entire row-major stride vector on the heap and compared slices against it. Accumulating the expected stride while walking backwards answers the same question with no allocation and short-circuits on the first mismatch. This is a simplification, not a measured win: on the dispatch benchmark it is a null result, because our elementwise fast path gates on is_dense and reaches is_contiguous barely at all (compute_contiguous_strides is 0.04% of sampled self time). Paired A/B, three repetitions, 100-node 8-element Relu: 0.0310 / 0.0311 / 0.0306 ms against 0.0307 / 0.0307 / 0.0309 -- indistinguishable. It is kept because it removes an allocation and a line of indirection from a predicate used across the dtype conversion, elementwise and host ABI paths that this benchmark does not exercise, not because it moved this number. Short-circuiting means the walk cannot overflow where the materialising version could, since that one built every stride before comparing any of them. Strictly more defensive, and unreachable either way for shapes whose element count fits in memory. Differential test against the previous implementation over ranks 0..=6, with exactly-contiguous, one-axis-disturbed and arbitrary strides plus ragged lengths, asserting the corpus actually reached the accepting arm. Mutation-verified: it fails if the product advances on the wrong index, if the walk runs forwards, or if the length check is dropped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Main landed several dispatch-path changes that overlap this stack. Resolved in favour of keeping both sides' wins rather than either side wholesale: - staging_log! doc comment: main's wording (the macro bodies were identical). - Scratch memory-info resolution: main moved it inside the routed branch so the single-node path stops paying an ORT memory-info query per kernel-context input per Run. Kept that placement, but on this stack device_mem_info already returns (mem_info, is_device) so the follow-up mem_info_is_device call is not made a second time. SubgraphFallback:: Deferred maps the tuple back to the pointer. - Single-node output views: kept this stack's absent_slot_strides, which builds one stride entry per *absent* slot and allocates nothing when there are none, over main's variant that clones every output shape whenever any slot is absent. Took main's lazy ort_view_iter, which drops the collect- then-drain Vec of views per call. The DispatchAlloc counter that named that Vec goes with it. - Single-node operand list: took main's OrtOperands::Slots, which removes the per-call node_ort_operands Vec. Kept the DispatchLookup phase marker. Gates: onnx-runtime-ep-plugin lib 259 passed; plugin_ort_e2e 55 passed (including every_assigned_node_is_also_executed_by_this_ep); clippy clean on onnx-runtime-ep-plugin and onnx-runtime-ir with --all-targets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Both conflicts are squash artifacts and both took main's side; this branch touches neither the probe module nor the cdylib manifest. Refs #1077 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Owner
Author
Pre-merge validation (local, comprehensive)Retargeted onto A/BTiny (8-element) Depth 100, clean reps: 1.179 / 1.172 / 1.166 / 1.162, median 1.169, against Two further reps were discarded, not averaged in: they read 1.039 and 1.091, but their ORT arms read 0.0509 and 0.0442 ms against a usual 0.0265 ms, so both were contaminated by external load on this shared box. Discarding them makes the claim smaller. Every rep kept was gated on ~99% idle across the four pinned cores. Gates
Carries an independent Opus review, APPROVE WITH NITS, nits fixed. |
justinchuby
added a commit
that referenced
this pull request
Aug 19, 2026
…de (#1472) Removes one heap allocation and one `Drain` drop per **routed node, per `Run`**. ## What The routed node loop collected `TensorMut`s for every ORT-bound output into `ort_out_views`, then immediately `drain(..)`ed it to feed the map that builds `all_output_views`. The intermediate vector was never read as a vector — every element went straight through to the next collect. `main` already removed exactly this shape of code on the **single-node** path. This does the same on the routed path, which is where it is paid *per node* rather than once per `Run`. ## Why it is safe `ort_outputs.push(..)` and `slot_kinds.push(RoutedSlotKind::Ort)` happen together in the one `NodeOutputSink::Ort` arm, so there is exactly one `Ort` slot kind per ORT output. The lazy iterator therefore yields precisely the same views, in the same order, and is exhausted identically. `Buffer` and `Absent` slots are untouched. `TensorMut` construction has no side effects, so not building views for outputs that are never consumed is unobservable — and by the lockstep argument there are none. The mutable borrow of `ort_outputs` now *starts later* (at the block rather than before the `new_bufs` loop) and ends at the same place, so this is strictly less restrictive to the borrow checker, not more. ## Measurement — this is a null result, and is labelled as one Paired interleaved A/B, `grid_relu_100_tiny` (100-node 8-element `Relu` chain), 800 iters, 100 warmup, 4 threads, pinned to cores 9/11/13/15 verified ≥99% idle, 3 reps: | | ratio vs ORT CPU | |---|---| | before | 1.193 / 1.190 / 1.180 | | after | 1.207 / 1.192 / 1.056\* | \* third rep contaminated (both sides inflated ~1.75×, host interference). Medians: **1.193 → 1.192**. Indistinguishable against a ~4% wall-clock noise floor. `perf record` self time attributes ~2% to the collect and its drain (`Vec<TensorMut>` collect 1.22%, `Drain<TensorMut>::drop` 0.79%), which this grid cannot resolve. This is the **third** independent confirmation of the finding recorded in #1077 §5e: removing allocations one or two at a time does not move this number. **So it is not offered as a performance fix.** It is offered as a simplification: it deletes a binding and a drain, removes an allocation the profile does show, and makes the routed path agree with the single-node path. That is what separates it from #1422, which was closed unmerged because it bought an equally null result at the cost of *added* representation complexity. Recorded in #1077 as a null result and **not counted toward the parity target**. ## Validation - `onnx-runtime-ep-plugin` lib: 261 passed - `plugin_ort_e2e`: 55 passed, including `every_assigned_node_is_also_executed_by_this_ep` (assigned == executed holds) - clippy `--all-targets` clean on the touched crate - No ORT CPU fallback, no runtime/default MLAS Refs #1077. Stacked on #1433 — auto-merge deliberately **off** while the base is an unprotected branch. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 19, 2026
Resolves 12 conflicts from the six squash landings of the #1077 stack (#1387, #1409, #1412, #1430, #1433, #1472). compute.rs (6): took main's side throughout. Main is the superset -- #1397's `device_mem_info` now returns `(mem_info, bool)`, #1401 builds one stride entry per *absent* slot rather than cloning every output shape, and the entry/lookup probe spans are new. This branch's `has_absent` guard is subsumed by the per-absent-slot construction. kernel_ctx.rs (6): kept this branch's one-call `GetTensorElementTypeAndShapeDataReference` structure and interleaved main's probe counters into both the borrowed and the legacy path, so `Event::OrtFfiCall` stays a total. Took main's `impl std::fmt::Display` for `validate_dims` (a superset of this branch's `Arguments<'_>`). Kept the inline-rank output-dims path, with the allocation counter moved to the heap-only branch where the allocation actually happens. Two instrumentation updates the merge made necessary: - dispatch_probe's `every_ort_entry_point_is_accounted_for` fired exactly as designed: this branch adds a 12th ORT API member. Table updated to 12 members / 12 `ort_call()` sites. - The pinned costs `1 + 7` calls and `1 + 3` allocations describe the *legacy* path: `fake_api()` zeroes the struct, so the reference hook is null there and the fallback runs. The fast path -- the one real ORT 1.27 takes, and the entire point of this PR -- had no cost pin at all. Added `the_reference_hook_path_costs_exactly_three_ort_calls_per_input` (pins 1 + 3, down from 1 + 7) and `the_reference_hook_path_allocates_less_than_the_legacy_path`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 19, 2026
) ## What this is `read_inputs` runs **once per input per `Run`**, and it asked ORT for the element type and the dimensions through the classic five-call sequence: ``` GetTensorTypeAndShape → GetTensorElementType → GetDimensionsCount → GetDimensions → ReleaseTensorTypeAndShapeInfo ``` The first of those makes ORT **heap-allocate** an `OrtTensorTypeAndShapeInfo` and copy the shape into it; the last frees it. Five FFI crossings and one allocation on ORT's side of the boundary, per input, per `Run`, to read data the `OrtValue` already owns. `GetTensorElementTypeAndShapeDataReference` (ORT C API, since 1.24) returns the element type **and a reference to the value's own shape array** in one call, allocating nothing. This PR routes `read_inputs` through it. Stacked on #1244 (since merged; this branch now targets `main` directly). No kernel is touched, no numerics change, and node assignment and execution ownership are unchanged: the CPU EP still executes every node it claims, locally, with ORT CPU fallback off. ## The three cuts **1. Five ORT calls and one ORT-side allocation per input → one call, no allocation.** The plugin already fails closed below API 27, so the hook is always present in practice. The five-call sequence is kept as a fallback for a host that leaves it null, and a host offering **neither** now fails closed with a message naming both — it does not silently read garbage shapes. The borrowed pointer is used only to build the owned `shape`/`strides` of the `OwnedInput`; nothing retains it. ORT spells a scalar as a **null** pointer with count 0, and `slice::from_raw_parts` is UB on null, so that case is guarded explicitly — see *Correctness* below for how that guard is actually enforced. **2. `validate_dims` takes a lazily-formatted label, not `&str`.** Every call site passed `&format!("input {i}")`, i.e. a heap `String` built on the success path purely to label an error that almost never happens. Callers now pass `format_args!(..)` and the string is only materialised inside the error branches. One `String` per input per `Run` gone. All call sites converted, including `transfer.rs` and the unit tests; no message text changed. (The merge with `main` took main's `impl std::fmt::Display` signature here, which is a superset of this branch's original `Arguments<'_>` and keeps both callers working unchanged.) **3. `allocate_output` converts the output shape to `i64` on the stack.** `let dims: Vec<i64> = shape.iter().map(|&d| d as i64).collect()` was a heap allocation per output per `Run` whose entire lifetime was the `KernelContext_GetOutput` call below it. Rank ≤ 8 now goes to a stack array; a taller tensor still works, through the `Vec`. For a 1-in/1-out elementwise node that is **5 fewer ORT FFI calls**, one fewer ORT-side allocation, and **2 fewer heap allocations** per `Run`, on top of #1244. ## A/B, one thread `taskset -c 8-15`, this branch vs its merge parent (`origin/main` + #1244), **interleaved** (A,B,A,B,… so drift hits both arms equally), 400 iterations after 50 warmup, **five rounds**. Ratio is **ours/ORT, lower is better**; each column is the median of the five rounds. "ORT drift" is the control: ORT's own p50 measured in the two arms, which must not move for the comparison to mean anything. | case | before p50 | after p50 | before p90 | after p90 | ORT drift | rounds won | |---|---|---|---|---|---|---| | `sqrt_f32_4k` | 1.023 | **0.952** | 1.031 | **0.964** | +2.9 % | 4/5 | | `leakyrelu_f32_4k` | 1.202 | **1.097** | 1.210 | **1.104** | 0.0 % | 5/5 | | `hardsigmoid_f32_4k` | 1.213 | **1.144** | 1.226 | **1.149** | 0.0 % | 5/5 | | `thresholdedrelu_f32_4k` | 1.316 | **1.209** | 1.314 | **1.220** | 0.0 % | 5/5 | | `sigmoid_f32_4k` | 1.328 | **1.252** | 1.333 | **1.258** | 0.0 % | 5/5 | | `tanh_f32_4k` | 1.368 | **1.287** | 1.369 | **1.291** | 0.0 % | 5/5 | | `erf_f32_4k` | 1.383 | **1.327** | 1.387 | **1.329** | 0.0 % | 5/5 | | `celu_f32_4k` | 0.430 | **0.411** | 0.435 | **0.418** | 0.0 % | 4/5 | | `elu_f32_4k` | 0.437 | **0.416** | 0.443 | **0.422** | −1.8 % | 4/5 | | `selu_f32_4k` | 0.467 | **0.441** | 0.471 | **0.446** | 0.0 % | 5/5 | | `log_f32_4k` | 0.727 | **0.700** | 0.728 | **0.706** | 0.0 % | 5/5 | | `mish_f32_4k` | 0.268 | **0.264** | 0.271 | **0.266** | 0.0 % | 4/5 | 12 of 12 cases improve at p50 and p90; 56 of 60 individual arm-pairs favour the change, and no case regressed in its median. In absolute terms it is **0.2–0.3 µs off every `Run`**: `tanh_f32_4k` 4.20 µs → 3.90 µs against ORT's 3.10 µs, `thresholdedrelu_f32_4k` 3.10 µs → 2.80 µs against ORT's 2.30 µs. That is the shape of a fixed per-call cost being removed, which is what it is. `sqrt_f32_4k` crosses below 1.00 — the plugin path now dispatches that op faster than plain ORT does. ## A/B, four threads Same protocol, `NXRT_MM_BENCH_THREADS` = `ONNX_GENAI_MLAS_THREADPOOL_THREADS` = `RAYON_NUM_THREADS` = 4, three rounds. | case | before p50 | after p50 | before p90 | after p90 | |---|---|---|---|---| | `leakyrelu_f32_4k` | 1.069 | **0.966** | 1.075 | **0.976** | | `hardsigmoid_f32_4k` | 1.152 | **1.050** | 1.164 | **1.060** | | `thresholdedrelu_f32_4k` | 1.212 | **1.091** | 1.228 | **1.106** | | `sqrt_f32_4k` | 1.121 | **1.036** | 1.130 | **1.045** | | `sigmoid_f32_4k` | 1.346 | **1.264** | 1.359 | **1.263** | | `tanh_f32_4k` | 1.434 | **1.319** | 1.453 | **1.325** | | `erf_f32_4k` | 1.581 | **1.502** | 1.588 | **1.510** | | `celu_f32_4k` | 0.460 | **0.437** | 0.476 | **0.445** | | `elu_f32_4k` | 0.481 | **0.448** | 0.486 | **0.468** | | `selu_f32_4k` | 0.500 | **0.458** | 0.482 | **0.464** | | `log_f32_4k` | 0.766 | **0.731** | 0.781 | **0.734** | | `mish_f32_4k` | 0.332 | **0.322** | 0.345 | **0.339** | 36 of 36 arm-pairs favour the change. The cost is per call, not per element or per worker, so the gain is the same absolute microseconds at every thread count. ## Drift control: the 1 Mi grid does not move Same protocol, `_f32_1m`, 200 iterations after 30 warmup, five interleaved rounds, one thread. A 1 Mi tensor amortises a fixed per-call cost away, so **these must not move** — and they don't: | case | before | after | | case | before | after | |---|---|---|---|---|---|---| | `relu_f32_1m` | 1.024 | 1.028 | | `sqrt_f32_1m` | 0.313 | 0.309 | | `exp_f32_1m` | 0.580 | 0.576 | | `log_f32_1m` | 0.264 | 0.258 | | `sigmoid_f32_1m` | 0.588 | 0.584 | | `mish_f32_1m` | 0.100 | 0.099 | | `tanh_f32_1m` | 0.618 | 0.620 | | `celu_f32_1m` | 0.127 | 0.129 | | `erf_f32_1m` | 0.651 | 0.668 | | `elu_f32_1m` | 0.129 | 0.129 | | `gelu_exact_f32_1m` | 0.585 | 0.584 | | `selu_f32_1m` | 0.136 | 0.138 | | `gelu_tanh_f32_1m` | 0.602 | 0.646 | | `hardsigmoid_f32_1m` | 0.455 | 0.453 | | `fastgelu_f32_1m` | 0.608 | 0.639 | | `leakyrelu_f32_1m` | 0.409 | 0.417 | | `quickgelu_f32_1m` | 0.429 | 0.438 | | `thresholdedrelu_f32_1m` | 0.567 | 0.573 | The per-case round-win counts here are 0/5–4/5 with a median of 2/5, i.e. a coin flip — the signature of noise, not of an effect. The two largest movers, `gelu_tanh` (+7 %) and `fastgelu` (+5 %), are the two cases whose **ORT** side also drifted most in the same rounds (−4.4 % and −2.8 %), so the ratio moved because the denominator did. This host's floor is roughly ±5 % on the 1 Mi grid and I am not claiming anything below it. ## Correctness **Five new tests, three with a verified falsifier**, driving `read_inputs` and `allocate_output` against a hand-built `OrtApi` whose hooks count their calls. | test | what it pins | falsifier | |---|---|---| | `input_shapes_come_from_one_call_when_ort_offers_the_reference_hook` | 1 reference call, **0** legacy calls, and the shape/strides/dtype that come out | forcing the legacy route fails it with `left: 0, right: 1` (run) | | `the_five_call_fallback_produces_the_same_input_as_the_reference_hook` | the fallback is exercised and its `OwnedInput` matches the reference path field for field | — (it *is* the parity check) | | `a_borrowed_scalar_shape_never_dereferences_null` | ORT's documented scalar spelling — null pointer, count 0 — takes the borrowed route and yields rank 0 | deleting the null guard makes **Miri** report `out-of-bounds pointer use: null pointer is a dangling pointer` at `kernel_ctx.rs:280` (run) | | `read_inputs_fails_closed_when_no_shape_route_exists` | the error names **both** routes; no silent garbage shapes | — | | `output_dims_are_identical_on_the_inline_and_heap_ranks` | ranks 0/1/8/9/12 all reach ORT with every dimension intact | dropping the heap arm delivers rank 9 truncated to 8 dims (run) | **The null guard is now actually enforced, not just asserted.** Natively, `from_raw_parts(null, 0)` returns an empty slice, so a test asserting the resulting shape passes with or without the guard — the guarantee only exists if Miri sees it. `onnx-runtime-ep-plugin` was not in the Miri matrix, so this PR adds `kernel_ctx::` as a lane in `.github/workflows/miri.yml` (the crate's other modules dlopen ORT and are not Miri-tractable, which is why the lane is scoped to the module rather than the crate). 25 tests pass under Miri in 4.15 s; with the guard deleted the lane fails. That pairing is what makes the test non-vacuous. **Suites.** `-p onnx-runtime-ep-plugin`: **252** unit tests pass (247 before, +5 new). `-p onnx-runtime-ep-cpu-plugin` with `NXRT_REQUIRE_ORT_TESTS=1`: every suite green, including all **55** `plugin_ort_e2e` cases — `every_assigned_node_is_also_executed_by_this_ep`, `no_supported_node_is_ever_left_to_the_ort_cpu_ep`, `no_matmul_family_node_escapes_to_the_ort_cpu_ep` and `every_fixture_loads_with_cpu_fallback_disabled` all pass, so **assigned still equals executed** with ORT CPU fallback disabled. `cargo clippy --release --all-targets -p onnx-runtime-ep-plugin` and `cargo fmt` are clean. **Build identity.** Pure native CPU EP: no MLAS at runtime, no ORT CPU EP fallback, no new dependency. AVX2/FMA host (`avx2 fma f16c`, no AVX-512), so ORT and we are on the same instruction footing. ## Independent review Reviewed by **Claude Opus 4.8**, read-only, briefed with the exact ORT contract for the borrowed pointer and asked specifically to hunt UB and vacuous tests. Verdict **APPROVE**, no blockers. It confirmed the null/scalar guard is unreachable-by-construction for `from_raw_parts`, that the borrow cannot outlive the `OrtValue` (its only consumer copies), that `ReleaseTensorTypeAndShapeInfo` still runs on every legacy error path — and noted that moving `DataType::from_onnx` after the match incidentally closes a **pre-existing** leak of `type_shape` on the unsupported-dtype path. It raised two test-quality defects, both real and both **fixed** in `6027f6e16`: 1. The three counting tests shared two process-wide `AtomicUsize`es while asserting exact counts, and libtest runs them in parallel — one test's reset could land inside another's assertion window. They now take a shared `SHAPE_COUNTER_LOCK` that resets both counters under the guard. 2. `a_borrowed_scalar_shape_never_dereferences_null` was **vacuous** with respect to its name, for exactly the reason above, and the crate was not under Miri. Hence the Miri lane, and the test now also asserts the scalar went through the borrowed route so it cannot pass on the fallback. It also flagged that `OrtStatus` is not released on error paths in this file. That is pre-existing and repo-wide in `kernel_ctx.rs` — the old five-call code leaked identically — so it is not touched here; it belongs in its own change. ## What is left After #1244 and this PR, a 1-in/1-out elementwise `Run` is down to roughly `KernelContext_GetInputCount` + `GetInput` + the one shape reference + `GetTensorData` + `GetOutput` + `GetTensorMutableData`. What remains: * **`OwnedInput`'s `shape` and `strides` `Vec`s**, `kernel_inputs`, `infer_shapes`'s `Vec<Vec<usize>>`, `slot_map`, `output_views`, and the `Box<HostPool>` in `host_pool::install` — each worth ~25–35 ns. Removing them needs an inline-capacity vector type or per-session caching of the parts that cannot change between `Run`s. Both are worth doing; neither belongs here. * **The `HostPool` box** specifically is @sebastian's 16-thread scheduling file and should come from him or after his PRs land. --- ## Refreshed against `main` @ `6a855d5e0`, and measured as a stack `origin/main` moved a long way while this sat in the CI queue (#1346, #1352 and #1361 on the quality lane; #1154, #1232, #1238 on the CPU side). Merged in normally — no rebase — and re-measured from scratch against the new baseline. Production pure-native A/B, plain ORT as the control arm. No MLAS, no ORT CPU fallback, no deferral. `taskset -c 8-15`, one thread, 400 iterations, five interleaved rounds out of two worktrees, started only once cores 8-15 were >=93% idle. **Ratio is ours/ORT, lower is better.** `before` is `main` at `6a855d5e0`; `after` is #1244 + #1246 together, since #1246 is stacked on #1244 and the pair is what a user gets. | case | ratio p50 main | ratio p50 stack | Δ | ratio p90 main | ratio p90 stack | ours us | ORT drift | rounds won | |---|---|---|---|---|---|---|---|---| | `thresholdedrelu_f32_4k` | 1.512 | **1.216** | -19.6% | 1.519 | **1.216** | 3.6 → **2.8** | -4.2% | 5/5 | | `tanh_f32_4k` | 1.511 | **1.279** | -15.4% | 1.513 | **1.284** | 4.6 → **3.9** | +0.0% | 5/5 | | `sigmoid_f32_4k` | 1.475 | **1.248** | -15.4% | 1.484 | **1.256** | 4.7 → **4.0** | +0.0% | 5/5 | | `erf_f32_4k` | 1.470 | **1.363** | -7.3% | 1.489 | **1.364** | 7.5 → **7.0** | +0.0% | 5/5 | | `hardsigmoid_f32_4k` | 1.416 | **1.127** | -20.4% | 1.426 | **1.133** | 3.5 → **2.8** | +0.0% | 5/5 | | `leakyrelu_f32_4k` | 1.361 | **1.094** | -19.6% | 1.371 | **1.104** | 3.5 → **2.8** | +0.0% | 5/5 | | `sqrt_f32_4k` | 1.141 | **0.946** | -17.1% | 1.150 | **0.955** | 4.0 → **3.4** | -2.8% | 5/5 | | `log_f32_4k` | 0.767 | **0.698** | -9.0% | 0.776 | **0.703** | 7.9 → **7.2** | +0.0% | 5/5 | | `selu_f32_4k` | 0.505 | **0.440** | -12.9% | 0.512 | **0.444** | 5.6 → **4.9** | +0.0% | 5/5 | | `elu_f32_4k` | 0.480 | **0.416** | -13.3% | 0.488 | **0.421** | 5.3 → **4.6** | +0.0% | 5/5 | | `celu_f32_4k` | 0.470 | **0.411** | -12.6% | 0.478 | **0.418** | 5.7 → **5.0** | +0.0% | 5/5 | | `mish_f32_4k` | 0.276 | **0.264** | -4.3% | 0.278 | **0.268** | 17.3 → **16.6** | +0.0% | 5/5 | **Every case, every round.** The two rows with a moving control (`sqrt` -2.8%, `thresholdedrelu` -4.2%) are reported rather than dropped; both won 5/5 anyway and their absolute time fell by the same ~0.7 us as everything else. That constant ~0.7 us is the point. It is not proportional to tensor size — the same absolute amount comes off `hardsigmoid` (3.5 -> 2.8 us) as off `mish` (17.3 -> 16.6 us) — which is what a fixed per-`Run` cost looks like when you remove some of it. It moves the cheap ops the most because they had the least to hide it behind, and `sqrt` crosses from 1.141 to **0.946**, from a loss to a win. ### Where the remaining time goes Measured directly, by instrumenting `compute_execute` segment by segment on top of this stack (temporary probe, not committed; `perf` is unavailable on this host — `perf_event_paranoid=4`). Per `Run`, one-in/one-out elementwise node, 4096 `f32`, microseconds: | segment | us | note | |---|---|---| | `KernelContext_GetOutput` | 0.35 | ORT's own API — ours to call, not to optimise | | `read_inputs` | 0.15 | 4 ORT FFI calls, already one shape call after #1246 | | rest of `allocate_output` | 0.13 | `GetTensorMutableData` + strides | | `prepare_workspace` | 0.09 | metadata vector + plan-cache lookup, for a kernel needing 0 bytes | | `host_pool::install` | 0.05 | @sebastian's, not touched | | `infer_shapes` | 0.05 | | | `kernel_inputs` | 0.04 | | | `output_views` | 0.04 | | Non-kernel node cost is **~1.25 us and near-constant across all twelve operators** (0.28 to 15.2 us of kernel time), which is the direct confirmation that small-node ratios on this EP are dispatch-bound rather than kernel-bound. There is no single large item left — the biggest, `KernelContext_GetOutput`, is ORT's. The rest is a long tail of 0.04-0.15 us items, which is what #1358 (`InlineVec`) starts on. ### And nothing breaks at 1 Mi Same harness, 1048576 elements, 120 iterations, 3 rounds. A fixed per-`Run` cost should be invisible here, and it is: | case | ratio p50 main | ratio p50 stack | ours us | ORT drift | |---|---|---|---|---| | `celu_f32_1m` | 0.141 | 0.140 | 382.1 → 377.9 | -0.0% | | `elu_f32_1m` | 0.138 | 0.136 | 347.7 → 343.6 | +0.1% | | `erf_f32_1m` | 0.671 | 0.670 | 595.2 → 594.6 | +0.1% | | `exp_f32_1m` | 0.606 | 0.590 | 247.0 → 240.5 | +0.1% | | `fastgelu_f32_1m` | 0.643 | 0.648 | 415.3 → 413.2 | -1.7% | | `gelu_exact_f32_1m` | 0.572 | 0.592 | 706.1 → 710.6 | -2.8% ⚠ | | `gelu_tanh_f32_1m` | 0.657 | 0.651 | 414.8 → 410.5 | +0.3% | | `hardsigmoid_f32_1m` | 0.376 | 0.372 | 88.0 → 69.6 | -0.9% | | `leakyrelu_f32_1m` | 0.421 | 0.430 | 90.1 → 87.3 | -2.2% ⚠ | | `log_f32_1m` | 0.270 | 0.274 | 616.4 → 612.4 | +1.7% | | `mish_f32_1m` | 0.105 | 0.105 | 1626.0 → 1626.0 | -0.2% | | `quickgelu_f32_1m` | 0.455 | 0.453 | 321.5 → 320.0 | -0.5% | | `relu_f32_1m` | 1.034 | 1.022 | 131.7 → 130.2 | +0.0% | | `selu_f32_1m` | 0.147 | 0.148 | 374.1 → 371.8 | -1.5% | | `sigmoid_f32_1m` | 0.471 | 0.606 | 231.6 → 229.0 | -23.4% ⚠ | | `sqrt_f32_1m` | 0.314 | 0.302 | 148.1 → 143.9 | +1.1% | | `tanh_f32_1m` | 0.644 | 0.631 | 226.2 → 221.9 | -0.2% | | `thresholdedrelu_f32_1m` | 0.500 | 0.486 | 70.9 → 68.6 | -0.3% | Flat, as predicted — 0.7 us against 70-1626 us of work. Absolute time is equal or better in 16 of 18 cases. The two ⚠ rows had the control move more than the effect: `sigmoid` is unusable (ORT itself moved -23.4%; our own absolute went 231.6 -> 229.0 us), and `gelu_exact`'s +0.6% absolute sits inside its -2.8% control. Reported rather than dropped. This is the coverage claim for the change: it buys ~0.7 us at every size, which is 20% of a small node and nothing at all of a large one, and it costs nothing anywhere. --- ## Status after merging latest `main` (2026-08-19) Validated on latest `main` after the six-PR #1077 stack landed (#1387, #1409, #1412, #1430, #1433, #1472). Full evidence in the PR comment below; the measured summary: **Deterministic counters, `relu_1_tiny`** — `OrtFfiCall` **10 → 6 per `Run`**, `DispatchAlloc` **24 → 20**. Four fewer round trips for one input, i.e. the predicted 7 → 3 per input. **Timing A/B**, production build, 3 clean reps (a 4th discarded for contention — it read 0.973, which would have flattered us): | case | main | this PR | |---|---|---| | `relu_1_tiny` | 1.397 | **1.271** | | `relu_10_tiny` | 1.250 | **1.173** | | `relu_100_tiny` | 1.163 | **1.110** | The gain shrinks as depth grows — the signature of a fixed-cost fix, since this removes FFI calls per `Run`, not per node. Fixed per-`Run` overhead **1.38 → 1.23** against ORT; per-node slope unchanged. **Test coverage gap this merge exposed and closed:** the pinned costs (`1 + 7` calls, `1 + 3` allocations) run against a zeroed `fake_api()`, where the reference hook is null — so they describe the *legacy fallback*, not the fast path this PR exists to add. Added `the_reference_hook_path_costs_exactly_three_ort_calls_per_input` (pins `1 + 3`) and `the_reference_hook_path_allocates_less_than_the_legacy_path`. `dispatch_probe`'s FFI-coverage table updated to 12 members / 12 `ort_call()` sites, which is the guard that flagged the new API member in the first place. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Resch <resch@squad.local>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1433 +/- ##
===========================================
- Coverage 82.10% 80.14% -1.97%
===========================================
Files 12 377 +365
Lines 5471 165131 +159660
Branches 5471 165131 +159660
===========================================
+ Hits 4492 132340 +127848
- Misses 780 27956 +27176
- Partials 199 4835 +4636
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #1077.
What
is_densecollected the non-trivial(stride, size)extents into aVeconevery call. It runs once per operand per node on the CPU plugin dispatch path,
so a rank-1 tensor of eight floats paid a
malloc/freepair to have fournumbers inspected.
Rank <= 8 covers every tensor ONNX produces in practice, so those extents now
live on the stack. Higher ranks keep the heap path rather than impose a limit
the signature does not have. Both paths hand the same slice to the same
routine (
dense_extents), so there is exactly one implementation of thepredicate and the two cannot drift.
Why this function
perf recordsampling of the 100-node 8-elementReluchain, our EP sidealone (
NXRT_MM_BENCH_SIDE=ours), 120k iterations so the steady statedominates rather than model construction:
compute_execute::{closure#0}malloc/_int_free/cfreeis_dense+ theVecit builtWorkspacePlanCache::lookup__memcmp_avx2_movbeis_densecost more than the kernel work it was gating.Measurement
Paired A/B, same host and session, three interleaved repetitions, 100-node
8-element
Reluchain, one thread on both sides, CPU fallback disabled,assigned == executedenforced by the suite:-6.4% on our side. The two sets are non-overlapping across every
repetition and the effect is well clear of this host's ~4% wall-clock noise
floor. Per-node slope (depth 100 minus depth 1, divided by 99) moves from
0.307 to 0.285 us/node against ORT's 0.223.
Correctness
is_densehad no tests in its own crate -- the only coverage was sevenassertions in a consumer crate. The new differential test runs the
pre-optimisation implementation verbatim as an oracle over ranks 0..=10,
straddling the inline bound in both directions, across contiguous, permuted,
zero-sized, negative-strided and length-mismatched cases. It also asserts the
corpus actually reached the dense arm, so it cannot silently degenerate into
"everything was rejected and both agreed".
Mutation-verified -- the test fails for each of:
d > 1filter dropped on the inline pathINLINE_RANKlowered to 2cargo test -p onnx-runtime-ir69 passed;cargo test -p onnx-runtime-ep-cpu --lib1447 passed;plugin_ort_e2e55 passed includingevery_assigned_node_is_also_executed_by_this_ep; clippy clean.Also in this PR:
is_contiguous(a null result, kept as a simplification)is_contiguoushad the identical shape of defect -- it built the entirerow-major stride vector on the heap and compared slices against it. It now
accumulates the expected stride while walking backwards, with no allocation and
a short-circuit on the first mismatch.
This is not claimed as a performance win. On the dispatch benchmark it is a
null result, because our elementwise fast path gates on
is_denseand reachesis_contiguousbarely at all --compute_contiguous_stridesis 0.04% ofsampled self time. Paired A/B, three repetitions:
is_densefix onlyis_contiguousIndistinguishable. It is kept because it removes an allocation and a line of
indirection from a predicate used across the dtype-conversion, elementwise and
host-ABI paths this benchmark does not exercise -- negative complexity, unlike
#1422, which was closed unmerged for being a null result that added
representation complexity.
Short-circuiting also means the walk cannot overflow where the materialising
version could, since that one built every stride before comparing any of them.
Differential test against the previous implementation over ranks 0..=6, with
exactly-contiguous, one-axis-disturbed and arbitrary strides plus ragged
lengths, asserting the corpus actually reached the accepting arm.
Mutation-verified against three corruptions: product advancing on the wrong
index, walking forwards, and dropping the length check.
Review follow-ups (applied)
Independent adversarial review returned APPROVE WITH NITS, no correctness
defects, and confirmed by diff that the differential oracle is a faithful
verbatim copy of the pre-change implementation. Two nits were real and are
fixed:
checked_dense > 100guard was satisfied four times over by rank-0shapes, which return
truefrom the empty-extents early exit withoutreaching the sort-and-product logic the guard names. It now counts only cases
with a dimension above 1; the corpus supplies 836 of them.
INLINE_RANKwith everydimension non-trivial -- the single input where the filled length equals the
whole array, so the truncation mutation cannot be caught. It was reached once
by chance in the random corpus and should not depend on the seed. It also
covers dense-but-not-row-major.
Mutation testing additionally showed the
pairs[0].0 != 1early rejection isredundant -- the loop's first iteration makes the same comparison. It is
preserved verbatim from the original and now labelled so a reader does not take
it for load-bearing.
Stacking
Based on
squad/resch-plan-cache(#1430). Auto-merge deliberately offwhile the base is a non-
mainbranch.