Skip to content

perf(ep): rework how EP tuning picks and saves, remove small-token host overhead - #604

Merged
isytwu merged 42 commits into
mainfrom
perf/ep-small-token-host-overhead
Aug 30, 2026
Merged

perf(ep): rework how EP tuning picks and saves, remove small-token host overhead#604
isytwu merged 42 commits into
mainfrom
perf/ep-small-token-host-overhead

Conversation

@isytwu

@isytwu isytwu commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

Small-token (4/8/16) EP16 dispatch/combine on MI300X is bound by the host
submission path
, not by the kernels. The series started there and grew to
cover the tuning machinery that was supposed to measure it, which turned out to
be unable to pick a winner at all.

Runtime

  1. perf(ep) — memoize the six output tensor views that dispatch() /
    combine() rebuilt on every call. This is the −32% below. Opt out with
    MORI_EP_DISABLE_VIEW_CACHE=1.

Tuning correctness

  1. fix(ep) — the tuning margin was an absolute 1.0 GB/s. Where a whole
    sweep sits in the low single digits (4/8 tokens: 3.9 / 8.3 GB/s) nothing
    after the first candidate could ever clear it, so the sweep returned
    whatever it measured first; at 32 tokens (30.7 GB/s) it was under 4% and
    filtered almost nothing. Now relative, via MORI_EP_TUNING_MARGIN,
    defaulting to 0 (any improvement wins). The same bug and the same fix
    applied to the intra-node tuner, which shares the env var.
  2. fix(ep) — selection used a slowest-rank bandwidth while the summary
    printed a grand mean, so a config could win the sweep on a number the table
    it was chosen from never showed. Both are now the grand-mean latency,
    the quantity printed as the table's "Average" row.
  3. refactor(ep)the keep-best save gate is gone. It compared
    new_bw > old_bw across two tuning runs from different hosts, ROCm
    versions and machine load, and on a "keep" it said so at logger.info,
    which this library's WARNING-level logger never emits. A discarded run was
    completely silent. Saves are now unconditional and always print what
    changed; the JSON's git diff is where the call actually gets made.
  4. refactor(ep)bandwidth_gbps is now the grand mean too, matching
    the printed table, and rules carry bandwidth_metric recording that. A
    re-tune that replaces a rule written under the old definition prints both
    numbers without a percentage instead of reporting a definition change as a
    20%+ win.
  5. fix(ep)MORI_TUNING_SCOPE=quick together with
    --save-tuning-config is now rejected, in both tuners and both batch
    scripts. quick sweeps a reduced grid (inter-node: 3 warp_per_block against
    full's 5; intra-node: 3 against 9, and powers of two only for block_num),
    so its winner is not a result worth committing. This changed the documented
    workflow — see Behaviour changes for existing users.
  6. feat(ep)tools/verify_tuned_config.sh, because the sweep only
    times candidates and never compares output: nothing confirmed the geometry
    it wrote was correct, only that it was fast.

Supporting

  1. feat(ep)MORI_EP_ROUNDS (default 10) now sets the rounds for both
    --cmd bench and each tuning candidate, so the two measure on equal
    footing. Round 0 is dropped as warmup; values below 2 are rejected.
  2. feat(ep)MORI_EP_TUNING_VERBOSE=1 prints each candidate's full
    table; block_num 8 and 16 added to the sweep's candidate list;
    rdma_worst_rank / ll_worst_rank printed next to the winner as a
    straggler check (not saved, not used for selection).
  3. feat(ep) — tuned InterNodeV1LL EP16 configs for MI300X at hidden
    6144. Read the caveat in About the shipped configs before relying on
    these.
  4. feat(ep)test_low_latency.py's shape is configurable instead of
    hardcoded.
  5. Toolingdocker exec is probed rather than assuming sudo;
    --local-docker runs rank 0 in a container so the driver can stay on the
    host and its SSH keys never enter a container; --rdma-tc plumbs
    MORI_RDMA_TC through to both ranks (it was silently dropped —
    build_torchrun_cmd names the environment explicitly and the ranks start
    via docker exec/ssh, neither of which inherits the caller's); the
    cleanup pkill chain no longer kills itself and is scoped to this
    script rather than every torchrun the user owns on the node; run_all_internode_tuning.sh
    propagates a non-zero exit; /logs/ is gitignored.

No kernel changes — see What is not here.

Measurements

EP16 = 2 × 8 MI300X, v1_ll, hidden 6144, fp8_e4m3_fnuz dispatch / bf16
combine, topk 8, --num-qp 1. Medians over 3–5 runs; single runs are not
meaningful here (see Measurement caveats).

Final validation with test_low_latency.py --num-tokens 4 --hidden 6144:

pre-change this PR
end-to-end bench() avg_t 216.8 us 147.4 us −32%
end-to-end min_t 199.5 us 134.2 us −33%
bench_kineto dispatch 62.0 us 59.7 us unchanged
bench_kineto combine 100.2 us 96.3 us unchanged

The last two rows are the point: GPU kernel time does not move, the entire
gain is host-side. bench_kineto explicitly uses a large kernel plus a barrier
to "eliminate the unbalanced CPU launch overhead", so it is structurally
blind to this class of problem — which is why it went unnoticed.

Same picture from test_dispatch_combine_internode.py, A/B in a single
session:

pre-change this PR
combine 144.7 us 86.8 us −40%
wall/round 220.9 us 162.4 us −26%
host enqueue/round 204.3 us 142.7 us −30%
dispatch 81.8 us 74.2 us −9%

Reproduced on a second, independent node pair with the same numbers (combine
150→88, wall 222→164, enqueue 211→146), so this is not a property of one
machine.

Why the regime is overhead-bound

test result conclusion
hidden 1024 vs 6144 (6× data) 93→85 us / 147→145 us — unchanged not data-bound
grid 304 → 24 blocks 12.13 → 12.05 us not grid-bound
host enqueue vs wall 204 us vs 221 us per round host-bound

Latency is also flat across 4/8/16 tokens.

Behaviour changes for existing users

  • --tuning-scope quick can no longer write a config. The documented
    "quick sweep" workflow in MORI-EP-GUIDE.md did exactly that, and
    run_all_internode_tuning.sh defaulted to quick while always passing a
    config output — so with no arguments it would now have failed all 42 jobs.
    Its default is therefore flipped to full, the guide is updated, and both
    batch scripts reject the combination up front (in a second, rather than after
    a two-node launch) naming the two knobs that resolve it. quick remains
    available for exploration with --config-output ''.
  • Saving overwrites unconditionally. A re-tune that measures worse will now
    replace a good rule. That is deliberate — the alternative failed silently —
    but it means the printed delta is the thing to read, and a bad run should be
    discarded with git checkout rather than trusted to be filtered out.
  • Tuning rounds default 5 → 10 (MORI_EP_ROUNDS), so a sweep takes about
    twice as long. That buys a tuner that can distinguish configs at all.

About the shipped configs

The configs are a reproducible starting point, not a proven optimum. The
tuner measures in the same host-bound regime this PR is about, so at 4 tokens
its metric cannot cleanly separate geometries.

Re-tuning the same four shapes three times on one node pair reproduced 6 of the
8 saved geometries exactly and put the other two in a flat region (latency
within 1-4% of the saved rule). Two of the 24 measurements were whole-sweep
degradations of 25-31% that landed on a different shape each run -- in both
cases the fastest of all 105 candidates for that shape was the degraded one,
so the tuner picked correctly and the run itself was bad. The mandatory save
report is what made those visible; under the old gate they would have been
discarded in silence.

verify_tuned_config.sh reports, before running anything, which phases have a
rule that will actually be selected, and refuses to claim it verified a phase
that fell back to the built-in geometry:

  dispatch rules:  4 (dtypes: fp8_e4m3_fnuz) queried with fp8_e4m3_fnuz -> MATCH
  combine  rules:  4 (dtypes: bf16) queried with bf16 -> MATCH

Review notes on the memoization

The addresses come from _dispatch_out_ptrs / _combine_out_ptrs, which
__init__ already snapshots once from handle-owned symmetric shmem
(GetShmemDispatchOutTokMemObj() etc., selected by the immutable
config.kernelType, allocated at handle construction with no realloc path).
The pointer-stability assumption is pre-existing — this change caches a
view over pointers the code already treats as fixed, and does not add a
reallocation guarantee (a realloc would leave both the key and the view stale,
exactly as before).

  • Growth: bounded by the distinct (shape, dtype) an op is called with —
    one, for a fixed model.
  • Lifetime: entries alias handle-owned memory and live on the op, so
    nothing is retained beyond it.
  • Object identity: callers now get the same object rather than an equal
    one. Both alias the same buffer and the kernels overwrite it in place, so the
    only visible change is identity. Any caller that expected two dispatch()
    results to be independent snapshots was already broken.
  • Concurrency: worst case two threads build the same view and one wins;
    both are equivalent.
  • Opt-out: MORI_EP_DISABLE_VIEW_CACHE=1, read once in __init__
    probing the environment per call would reintroduce the cost this removes.

Correctness: --cmd test at 4 and 16 tokens, 500 rounds, on all 16 ranks,
completed without raising. Note that the error times: N line those runs print
is not the evidence — error_round is never populated anywhere in the file,
so that counter is always 0. Every comparison in run_test_once is a bare
assert, so the signal is that the run finished at all.

--num-qp: 1 is best, and here is why

Swept with the host off the critical path so the parameter can actually show,
5 runs each:

--num-qp 1 2 3 4
round (us) 149.8 160.1 168.4 170.8

Monotonic, and the distributions for 1 and 4 do not overlap (qp1 max 162.2 <
qp4 min 165.6). It holds at every size tested: −12.3% at 4 tokens, −3.1% at
128, −5.1% at 512.

The mechanism is in the combine cross-device barrier, which issues one RDMA
atomic per QP
:

for (int i = 0; i < config.numQpPerPe; i++) {
  shmem::ShmemAtomicTypeNonFetchThread<uint64_t>(..., core::AMO_ADD, proxyPe, i);
}

Meanwhile the data path picks a QP with
qpId = (tokenId / warpSize) % numQpPerPe, which is always 0 at 4 tokens. So
extra QPs buy nothing and cost extra RDMA atomic issues — span-profiling puts a
single GPU-initiated RDMA issue at ~13–16 us, which matches the ~21 us gap
between qp=1 and qp=4.

Two notes: measured plainly, this is invisible (158.4 vs 161.8 us, inside the
noise) because the host dominates. And MORI_NUM_QP_PER_PE (transport-level QP
count, default 4) is a different knob and had no measurable effect (152.7 vs
151.3 us); only the op config's numQpPerPe matters.

This is a usage recommendation; the default is left alone here.

Reproducing

Two nodes, $R = 0 / 1, start node 1 first.

export MORI_RDMA_TC=41 GPU_PER_NODE=8
export GLOO_SOCKET_IFNAME=<IFNAME> MORI_SOCKET_IFNAME=<IFNAME>

# baseline: pre-change behaviour
export MORI_EP_DISABLE_VIEW_CACHE=1
torchrun --nnodes=2 --node_rank=$R --nproc_per_node=1 \
  --master_addr=<HOST0> --master_port=1234 \
  examples/ops/dispatch_combine/test_dispatch_combine_internode.py \
  --cmd bench --kernel-type v1_ll --num-qp 1 \
  --max-tokens 4 --hidden-dim 6144 \
  --dtype fp8_e4m3_fnuz --combine-dtype bf16 --quant-type none \
  --block-num 32 --warp-per-block 4 --rdma-block-num 8

# this PR: drop the opt-out and rerun the identical torchrun command
unset MORI_EP_DISABLE_VIEW_CACHE

Tuning and verification, driver on the host with mori in containers on both
nodes:

bash tools/batch_internode_tuning.sh \
    --master-addr <HOST0> --peer-host <USER>@<HOST1> --ifname <IFNAME> \
    --local-docker <RANK0_CONTAINER> --docker <PEER_CONTAINER> --rdma-tc 41 \
    --kernel-type v1_ll --num-qp 1 \
    --dtype fp8_e4m3_fnuz --combine-dtype bf16 --quant-type none \
    --tokens-list "4,8,16,32" --hidden-dims "6144"

bash tools/verify_tuned_config.sh <same connection and shape flags>

End-to-end numbers above come from test_low_latency.py --num-tokens 4 --hidden 6144, which this series makes configurable.

Measurement caveats, all of which produced wrong conclusions during this
work:

  • The first run after a JIT-cache wipe or an idle period reads 40–80% slow.
    Discard one run before comparing.
  • Run-to-run spread is 10–20%, and roughly 1 run in 5 lands in a reproducible
    slow mode (~1.5×). Compare medians over ≥3 runs, never a single one.
  • EpDispatchCopyToStaging makes a good canary: nothing here touches it, so a
    reading above ~5 us means the environment is bad rather than the patch.

What is not here

No kernel optimization. Four kernel changes were implemented, verified
correct, and measured against per-kernel GPU time from the CUDA profiler — all
within noise (136.0/136.2 vs 135.5/135.5 us): a MultiWarpIter minimum slice,
skipping combine padding slots, an s_sleep poll backoff, and counting only
active blocks in the dispatch send barrier.

Where the GPU time actually goes, at 4 tokens / hidden 6144 (136 us total): the
two LL kernels are 82% of it, and fitting the token sweep gives dispatch ≈
50 us + 0.5 us/token and combine ≈ 46 us + 0.48 us/token. At 4 tokens
that intercept is 98% of the cost.

That fixed cost is not the fabric — ib_write_lat on the same NIC pair is 10.7
us for the 24 KB a 4-token dispatch moves. Span-profiling with
MORI_TRACE_SPAN locates it:

  • one warp spends 16.2 us issuing a single RDMA put while the other 127
    leave the send phase in 1.24 us; the recv phase's 17.25 us warp-ramp is
    exactly that warp arriving late
  • combine's median warp does 14.26 us of real workwarpsPerToken is
    pinned at 4, so each warp gathers 8 experts × 1536 elements over XGMI

The put cannot simply be trimmed: downgrading its four __threadfence_system()
calls to agent scope regresses everything (dispatch 55 → 131 us) because the
NIC stops seeing the payload.

Worth filing separately:

  1. ENABLE_PROFILER=ON does not build on mainintranode_ll.hpp
    references three Slot enumerators that are never declared, because the
    generator groups slots by source-file stem while Slot there aliases
    intranode.hpp's enum.
  2. Tuning configs frequently do not take effect, three independent reasons:
    MORI_EP_LAUNCH_CONFIG_MODE defaults to MANUAL, so the shipped JSONs are
    inert unless a caller opts in; tuning writes to the repo while the op reads
    the installed copy; on a miss _find_fallback_config silently substitutes
    another GPU model's file (observed running mi308x configs on MI300X),
    announcing it at logger.info. Each of these is a silent fallback with no
    warning a user will see.
  3. get_launch_config looks both phases up with config.data_type, so for a
    mixed-dtype config it returns nothing for combine. It is not on the runtime
    path -- dispatch()/combine() go through _resolve_launch_params with
    dtype=input.dtype, which is correct -- and nothing in the repository calls
    it, so this is latent. Documented in place rather than changed, since fixing
    it means deciding what its signature should be.
  4. error_round in test_dispatch_combine_internode.py is passed to
    run_test_once and never written, so every error times: line the test
    prints reads 0 regardless of outcome. It reads like a correctness summary
    and is not one.

@isytwu isytwu self-assigned this Aug 26, 2026
@isytwu
isytwu force-pushed the perf/ep-small-token-host-overhead branch from 77b1553 to 51d0cb5 Compare August 26, 2026 03:38
isytwu added 10 commits August 27, 2026 10:38
The winner threshold was an absolute 1.0 GB/s. That constant cannot work
across the operating range: at 4 tokens the whole sweep lives in 2.0-3.2
GB/s, so `bw > best + 1.0` never fires and the tuner returns candidate
[1/N] verbatim for every shape; at large token counts the same 1.0 GB/s
is under 2% and lets noise through. Dispatch happened to be fine because
candidate 1 is genuinely near-optimal for it, but combine wants the
opposite end of the space and was losing ~13% on EP16/MI300X.

Four changes, together aimed at "the saved config should be the best one
*and* should stop churning between re-tunes":

- relative margin (_BW_REL_MARGIN = 2%) instead of absolute GB/s
- score each config by the median over 9 rounds, not the mean over 5, so
  a single stalled round cannot decide a winner
- break ties inside the margin deterministically on block_num rather than
  letting whichever sample landed higher win -- two configs that are
  statistically indistinguishable must not alternate between runs
- require the same 2% margin before an existing rule is overwritten on
  save, so a re-tune that lands higher purely on noise leaves the
  checked-in JSON alone

Rounds per config go 5 -> 9, so a full sweep takes ~1.8x longer.

With this the tuner selects genuinely different geometries for dispatch
and combine, which it structurally could not do before.
dispatch() and combine() rebuilt six torch views over persistent shmem
buffers on every call. The addresses come from _dispatch_out_ptrs /
_combine_out_ptrs, which __init__ already snapshots once from handle-owned
symmetric memory, and the shapes are fixed by the config -- so every
rebuild produced an identical view, yet each one still walked
__cuda_array_interface__ and queried torch.cuda.current_device().

At small token counts that is not bookkeeping, it is latency. On EP16
(2x8 MI300X, hidden 6144, 4 tokens) host-side enqueue costs ~210us per
dispatch+combine round against ~222us of wall time, i.e. the host is what
paces the GPU, and cProfile attributes ~60us of that to this wrapping.
The regime is overhead-bound rather than data-bound: latency is flat from
hidden 1024 to 6144 and flat across 4/8/16 tokens.

Same-session A/B, 5 trials each:

  tokens   combine        wall/round     host enqueue/round
  4        150 -> 88 us   222 -> 164 us  211 -> 146 us
  8        146 -> 86 us   229 -> 162 us
  16       140 -> 93 us   232 -> 184 us

Cross-checked with the DeepEP-style bench (test_low_latency.py at 4
tokens): end-to-end 214 -> 157 us, while its kineto kernel-time metric is
unchanged -- that metric deliberately excludes CPU launch overhead, which
is why this went unnoticed.

Safety notes:

- The cache key includes the pointer, so a future reallocation misses the
  lookup and builds a correct view instead of returning a stale one.
- Entries are bounded by the distinct (shape, dtype) an op sees -- one for
  a fixed model. They alias handle-owned memory and share the op's
  lifetime, so nothing is retained beyond it.
- Callers get the same object rather than an equal one. Both alias the
  same buffer and the kernels overwrite it in place, so the only visible
  change is object identity.
- Stable output objects are also what makes the op capturable into a HIP
  graph; rebuilding views per call puts host logic inside the capture.
- MORI_EP_DISABLE_VIEW_CACHE=1 restores the old behaviour. It is read once
  in __init__ -- probing the environment per call would reintroduce the
  cost this removes.

Verified with --cmd test at 4 and 16 tokens, 500 rounds, 0 errors on all
16 ranks, and HIP graph replay bit-exact against eager on all 16 ranks.
test_low_latency.py had num_tokens / hidden / num_topk / num_experts
hardcoded in test_loop(), so measuring any shape other than the one baked
in meant editing the file. Expose them as arguments, along with the rank
count and the pressure-test loop that was a hardcoded `False`.

Defaults reproduce exactly the shape the test has always run
(128 / 7168 / 8 / 288), so existing invocations are unaffected.

  python examples/ops/dispatch_combine/test_low_latency.py \
    --num-tokens 4 --hidden 6144
Generated with MORI_TUNING_SCOPE=full (75 configs) on a 2x8 MI300X pair
for the small-token regime (4/8/16 tokens, hidden 6144, fp8_e4m3_fnuz
dispatch / bf16 combine, topk 8, num_qp 1).

                 dispatch          combine
  4 tokens       32 / 4 / 21       128 / 4 / 64    (block/warp/rdma_block)
  8 tokens       32 / 8 / 16       32 / 4 / 8
  16 tokens      32 / 8 / 16       32 / 4 / 16

Produced after the tuning-margin fix, so the two phases carry genuinely
different geometries -- previously every shape collapsed onto the first
swept candidate.

Known limitation, worth reading before trusting the 4-token row. The
tuner measures in the same host-bound regime this series is about, and
there its metric cannot separate good geometries: benchmarking these
against 32/4/8 and against 32/8/16 + 64/4/32 at 4 tokens gives 158.9 /
158.9 / 155.6 us, i.e. all three are inside the run-to-run spread. Take
the host out of the critical path and they separate clearly, with 32/4/8
ahead of what the tuner picked here. So these are a reproducible starting
point rather than a proven optimum, and the 4-token entry in particular
should be revisited once tuning can measure without the host in the way.

They only take effect with MORI_EP_LAUNCH_CONFIG_MODE=AUTO, and a
non-editable install reads the copy under site-packages rather than this
one.
The previous commit replaced an unusable absolute 1.0 GB/s threshold with
a 2% relative one. 2% is the right default for keeping a checked-in config
stable, but it is the wrong default for someone tuning to find the fastest
config: an improvement of 1% is discarded, and the margin also blocks
re-tunes from persisting a better result.

Default both margins to 0 -- take any improvement -- and expose them as
MORI_EP_TUNING_MARGIN for the cases that want stability back:

  MORI_EP_TUNING_MARGIN=0.02 <tuning command>

The two margins move together because they answer the same question at
different moments: which config wins inside a sweep, and whether a re-tune
may overwrite the saved rule.

The tradeoff is real and worth stating. Run-to-run spread on this workload
is 10-20%, so with no margin the winner is partly whichever config drew the
luckiest sample, and every re-tune is free to rewrite the JSON. The median
over 9 rounds introduced alongside the relative margin is the primary
defence against that and is unchanged here; the margin was only ever a
second, optional one. Exact ties still keep the earlier (smaller block_num)
candidate, so the sweep remains deterministic when two configs measure
identically.
…igurable

Customer data on a production MI308 rig showed the median-scored, no-margin
tuner (introduced two commits ago) regressing Worst and Average by 30-45%
at bs=16/32, while Best stayed essentially flat and the winning geometry
trended toward larger, more parallel configs (up to block=80 warp=16
rdma=53). That signature is not environment noise -- noise would not spare
Best while inflating Worst for one selection method only. It is what you
get when the scoring statistic cannot see a config's worst case: median
only asks "what does a typical round look like", so a geometry with a fine
median and a heavy tail (more blocks/warps contending for the same SMs and
RDMA queues) can win outright, whereas mean is pulled up hard by a single
bad round and would have penalized exactly that tail.

Revert the default to mean, matching what --cmd bench itself reports and
what main always used. Keep both knobs configurable without editing the
file:

  MORI_EP_TUNING_ROUNDS=<n>          # rounds per candidate, default 9
  MORI_EP_TUNING_STAT=mean|median    # scoring statistic, default mean

A full 75-config sweep at 4/8/16/32 tokens on an idle 2x8 MI300X pair (no
contention, different SKU from the customer's box) did not reproduce the
same magnitude -- mean's and median's picks came out within normal
run-to-run noise of each other there, sometimes in either direction. That
does not contradict the mechanism, it just means an idle, uncontended rig
does not stress the tail the way production load does; treat the fix as
warranted by the mechanism and the customer signature, not as bounded by
this sanity check.

Also regenerates the shipped MI300X EP16 configs with the new mean
default so the checked-in JSON matches what a fresh tune actually
produces now:

               dispatch          combine
  4 tokens     32/4/21           128/4/32
  8 tokens     64/4/32           128/6/64
  16 tokens    64/4/32           256/4/64

Verified with --cmd test at 4 and 16 tokens, 0 errors on all 16 ranks.
… history

Comments-only. The customer incident narrative belongs in the previous
commit's message, not as permanent inline prose everyone re-reads. Verified
the mean-branch computation is untouched and still matches main's formula
exactly: kept.mean(dim=0) -> same column indexing -> same * ll_scale ->
same .min().item(), for both dispatch and combine.
Selection compared a slowest-rank XGMI bandwidth figure
(min-across-ranks of each rank's own mean/median bandwidth) while the
"Average" column the final table -- and every re-run of --cmd bench --
actually shows is a different quantity: the grand mean of raw latency
across all ranks and all rounds. A config could clear the selection
bar on the first metric while looking worse than another candidate on
the second, because the two were never mathematically required to
agree. That gap is what let a customer ask "tuning says this config
won, so why does the printed bench table show it's worse" -- a
reasonable question, because the two numbers were never the same
thing to begin with.

Make the winner literally the config with the lowest
disp_stats["lat"][2] / comb_stats["lat"][2] -- the exact scalar
_build_phase_stats computes and _print_phase_table prints as
"Average". Verified directly: across a full 75-candidate sweep, the
winning config's printed Average (dispatch 49.19us, combine 73.54us)
equals the minimum seen across every candidate in the sweep (49.2us /
73.5us at the sweep's own print precision) -- selection and the
printed number are now the same value by construction, not just
usually close.

Consequences of tying selection to this specific quantity:

- Retires MORI_EP_TUNING_STAT (mean vs median). _build_phase_stats
  always reports a grand mean, never a median, so a "median" selection
  mode could no longer mean anything coherent once selection has to
  match what gets printed -- keeping the knob around would have meant
  it silently did nothing for this decision.
- _BW_REL_MARGIN is renamed _TUNING_MARGIN and now gates a
  lower-is-better latency comparison instead of a higher-is-better
  bandwidth one. MORI_EP_TUNING_MARGIN keeps its name and default (0)
  as a relative margin; only the quantity it's a margin *of* changed.
- The saved JSON's top-level bandwidth_gbps no longer comes from the
  selection variable (which is now a latency, not a bandwidth); it's
  computed post-hoc from the winning config's own already-computed
  stats (_headline_bw), so the field keeps meaning what it always
  meant -- LL bandwidth for LL/AsyncLL kernels, RDMA bandwidth
  otherwise -- decoupled from whatever criterion picked the winner.

Verified with --cmd test at 4 and 16 tokens, 0 errors on all 16 ranks.
…uner

The previous commit changes what the tuner selects (grand-mean latency,
matching the printed Average, instead of a slowest-rank bandwidth figure
that could diverge from it). The checked-in configs were tuned under the
old selection metric, so they no longer reflect what this branch's own
tuner would pick if run today -- regenerate them so the shipped JSON is
consistent with the code that ships alongside it.

               dispatch          combine
  4 tokens     64/4/42           64/4/32     (block/warp/rdma_block)
  8 tokens     64/4/42           32/4/21
  16 tokens    64/4/42           32/4/21

Verified with --cmd test at 4 and 16 tokens through the real
MORI_EP_LAUNCH_CONFIG_MODE=AUTO load path, 0 errors on all 16 ranks.
@isytwu
isytwu force-pushed the perf/ep-small-token-host-overhead branch from 86f7d86 to 5938837 Compare August 27, 2026 10:54
isytwu added 10 commits August 27, 2026 11:35
9 was a leftover from an abandoned median-scoring experiment (which wanted
extra samples to resist a single stalled round); that experiment was
reverted back to mean scoring, but the rounds default was never reverted
with it, silently paying a ~1.8x longer sweep for a defense mean scoring
doesn't need. Main has always used repeat=5.

Verified on an idle 2x8 MI300X pair: a full 75-candidate sweep at 4 tokens
with the new default finished in 12s and picked the same dispatch geometry
(48.2 -> 47.6us, within noise) and an equally good combine geometry (69.0 ->
69.4us, within noise) as the previous 9-round default.
… add 32-token

Full 75-candidate sweeps on an idle 2x8 MI300X pair (per-phase, per-token-count),
now running under the 5-round default instead of 9:

               dispatch                  combine
  4 tokens     32/4/21   -> 48.04us      64/4/32  -> 69.04us (unchanged)
  8 tokens     64/4/42   -> 48.10us (unchanged)   64/4/32  -> 69.11us
  16 tokens    64/4/42   -> 49.10us      32/4/21  -> 69.16us (unchanged)
  32 tokens    128/4/64  -> 52.44us (new)         64/4/32  -> 74.80us (new)

Every changed number is within this workload's documented run-to-run noise of
its 9-round predecessor; several entries didn't change at all because the
sweep re-picked the exact same geometry. Verified with --cmd test at 4/16/32
tokens, 0 errors across all 16 ranks.
… rounds/margin guards)

Four issues from review, all confirmed live:

- _headline_bw saved the grand-mean bandwidth (stats["ll"/"rdma"][2], the
  same number this file prints as "Average") as the JSON's bandwidth_gbps.
  save_tuning_result gates overwriting an existing rule on new_bw > old_bw,
  and every rule on disk -- including ones this PR never touches, e.g. other
  GPU models/kernel types -- was written under main's original definition,
  slowest-rank mean bandwidth. Grand mean is systematically >= slowest-rank
  mean, so the first re-tune with this code would silently overwrite a
  still-good rule with zero real improvement. Restored the slowest-rank
  semantics via two new _build_phase_stats fields (rdma_worst_rank,
  ll_worst_rank) so bandwidth_gbps means what it always meant; avg_latency_us
  and the avg_*_bandwidth_gbps fields (already grand-mean, and what _beats
  actually selects on) are untouched.

- _beats' "ties break on smaller block_num" branch was dead: the sweep visits
  block_num in ascending order, so a later candidate's block_num is never
  smaller than the incumbent's once one has won. It looked like a deliberate
  tie-break but was actually just first-found-wins via iteration order, which
  silently stops being deterministic if the sweep is ever reordered/
  parallelized. Switched to comparing the full (block_num, warp_per_block,
  rdma_block_num) tuple, which is a real tie-break independent of visitation
  order.

- MORI_EP_TUNING_ROUNDS had no lower bound. tuning_dispatch_combine drops
  round 0 as in-loop warmup (kept = all_data[1:]), so ROUNDS=1 leaves an
  empty tensor and _compute_stats' .min()/.max()/.mean() crash with a shape
  error that gives no hint the real cause is too few rounds. Now rejected
  at import time with a clear message.

- MORI_EP_TUNING_MARGIN="" (set but empty, not unset) reached float("") and
  crashed at import in both this file and tuning_config.py, since
  os.environ.get(k, default) only substitutes default when the key is
  absent. Both now fall back through `or "0.0"` so empty is treated the same
  as unset.

Regenerated the shipped MI300X EP16 JSON (dispatch+combine, 4/8/16/32
tokens) since bandwidth_gbps values shift under the corrected semantics;
avg_latency_us per entry is within noise of the prior commit's numbers.
Verified with --cmd test at all four token counts, 0 errors across all 16
ranks.
…gbps semantics

Companion to the previous commit's _headline_bw fix -- bandwidth_gbps now
reports slowest-rank mean bandwidth instead of the grand mean, so every
entry's number drops even though nothing about the winning geometry or its
actual latency changed. Regenerated fresh (not via a re-tune diff, which
would have been blocked from correcting the stale entries by the very bug
being fixed) on an idle 2x8 MI300X pair; avg_latency_us matches the prior
commit within noise. Verified with --cmd test at 4/8/16/32 tokens, 0 errors
across all 16 ranks.
Debug aid, no effect on selection or saved results: when set, prints every
candidate's full PrettyTable (the same _print_phase_table bench uses)
instead of just the one-line "disp sel=... comb sel=..." summary. Lets a
candidate's raw Best/Worst/Average be diffed directly against a --cmd bench
run of that same block/warp/rdma, which is what's needed to pin down why
tuning's self-reported latency for a config doesn't match bench's for the
same config.
The per-candidate progress header printed "block_num=X, warp=Y,
rdma_block_num=Z" while every table title (both the final Tuning Result and
the new MORI_EP_TUNING_VERBOSE per-candidate tables) prints "block=X warp=Y
rdma=Z" -- same fields, different names/punctuation, made grepping a
specific config across a sweep's output inconsistent. No behavior change.
The sweep's block_num candidates started at 32 (32, 64, 128, ... up to
sm_count) with no lower-range coverage. Measured directly on MI300X EP16
v1_ll:
  - --cmd test at block=2/4/6/8/16 (tok=4): all correct, 0 errors -- the
    kernel has no hard minimum block count, this was purely an unexplored
    region of the sweep.
  - --cmd bench across tok=4/8/16/32: block=2/4/6/8 lose everywhere (e.g.
    93.94us dispatch at block=2 vs ~48us for the eventual winner, at 4
    tokens). block=16 ties the winner at 4 tokens but is clearly worse from
    8 tokens up (56.29 vs 49.01us dispatch at 8 tokens).

Added only 8 and 16 (not 2/4/6), generically via sm_count rather than
hardcoded per architecture, so a re-tune on different hardware (e.g.
MI308's 80 CUs) empirically finds its own answer instead of assuming this
MI300X result transfers. A fresh full sweep with the wider candidate list
already found a new best for 4-token dispatch: block=16/warp=4/rdma=10 at
47.64us average, edging out the previous 64/4/42 at 48.46us. Verified: 0
crashes across a 105-candidate sweep (up from 75).
Re-tuned with the widened block_num candidate list (8/16 added). Adopted
only where an independent --cmd bench re-check confirmed the new pick
holds up:

  - 4 tokens: block 64->16 (dispatch), 128->16 (combine). Latency is a wash
    within noise (dispatch 47.99 vs 48.14us, combine 71.02 vs 70.93us on
    direct re-check) but block=16 uses a quarter to an eighth of the CUs
    for the same performance -- strictly better use of resources on a
    workload this small, with headroom to spare if the GPU is shared with
    other concurrent kernels.
  - 32 tokens: rdma_block_num 85->64 (dispatch), same geometry with
    refreshed numbers (combine). Not a real change, just a closer number
    from a repeat sweep.

8 and 16 tokens are deliberately left untouched: the widened sweep's own
picks for both (dispatch 16->51.69us at 16 tokens vs the existing 49.27us;
combine 32->76.75us at 16 tokens vs the existing 70.05us) turned out to be
a single noisy sample winning under 0-margin selection, not a real
improvement -- confirmed by re-benching both the old and new picks
independently and finding the existing config still faster. Re-tuning the
same hardware isn't guaranteed to reproduce or improve on a previous
result when the search space grows; each candidate here was verified
against the currently shipped config before being adopted, not taken from
the sweep's own printout.

Verified with --cmd test at 4/8/16/32 tokens, 0 errors across all 16 ranks.
…sion

Two bugs in bench's per-round debug print (--cmd bench, before the
Best/Worst/Average table):

- The print loop was phase-outer, round-inner: all dispatch rounds printed
  first (each correctly under its own "Round i"), then all combine rounds
  printed with no "Round i" header at all, since the header was gated on
  `cols is _labels[0][1]` (only true for dispatch). Combine's round 0 data
  ended up visually attached to dispatch's last round in the output, with
  no way to tell which combine line belonged to which round. Restructured
  to round-outer, phase-inner so one "Round i" header covers both phases'
  lines for that round, matching what the output already looked like it
  meant.
- The raw per-rank list used `.int().tolist()`, truncating every value to
  an integer (bandwidth/duration in the 1-10 range loses almost all its
  precision this way) even though the "avg" on the same line already kept
  2 decimals. Switched to round(v, 2) so the raw list and its average are
  consistent.
--cmd bench (repeat=10, hardcoded) and --cmd tuning (repeat=_TUNING_ROUNDS,
MORI_EP_TUNING_ROUNDS, default 5) measured on different round counts, which
was one of the two confirmed structural differences behind tuning's
self-reported latency for a config not matching an independent bench run
of that same config (the other being GPU DVFS/clock-ramp state, which is
an environment property, not something this change addresses).

Renamed to MORI_EP_ROUNDS, shared by both call sites, default 10 (bench's
old value). Tuning's default goes from 5 back to 10, doubling sweep time
again, but now the two are measured on equal footing by construction
instead of by coincidence. Verified MORI_EP_ROUNDS=1 still raises the
expected "must be >= 2" error (single-process check, no GPU needed --
the validation fires at import time before any distributed setup).
isytwu added 6 commits August 28, 2026 04:04
MORI_TUNING_SCOPE=quick narrows the sweep to 3 warp_per_block candidates
against full's 5, plus a narrower rdma_block_num set per block. A winner
picked from that reduced space is fine to look at, but it is not a result
worth committing as a saved config -- and nothing stopped the two being
combined, so a quick run with --save-tuning-config would write into the
repo's tuning_configs JSON as if it were a full sweep.

Raise instead of silently downgrading to full or warning and continuing:
the caller asked for two things that cannot both be honoured, and quietly
doing something other than what was asked is how the config would end up
wrong without anyone noticing. The check runs before the op is even built,
so it fails immediately rather than after a full sweep.
The tuning sweep only times candidates: run_bench_once launches dispatch
and combine and records durations, but never compares their output against
what it should be. So the (block_num, warp_per_block, rdma_block_num) it
writes to tuning_configs JSON is known to be fast and not known to be
correct.

Checking every candidate inside the sweep would answer that, but it would
also slow the sweep down substantially for a question the sweep does not
need answered while ranking configs. This runs the check once, afterwards,
against the winner: --cmd test under MORI_EP_LAUNCH_CONFIG_MODE=AUTO, so
each shape is built with whatever AUTO looks up from the JSON on disk, then
500 rounds compare dispatch/combine output element-wise.

Intended between batch_internode_tuning.sh and committing the JSON it
wrote. Exits nonzero if any rank reports nonzero error times, so it can
gate a commit. Argument surface and the SSH/docker/cleanup/timeout handling
mirror batch_internode_tuning.sh.
save_tuning_result only overwrote an existing rule when the new
bandwidth_gbps beat the old one by a margin. The intent was to stop a
shallow or noisy re-tune from replacing a good rule, but two tuning runs
come from different hosts, ROCm versions, machine load and points in time,
so their numbers were never comparable enough to decide that -- and the
metric being compared is not even what the inter-node sweep selects on
(it picks by grand-mean latency; bandwidth_gbps was derived separately).

The failure directions are also asymmetric. Wrongly overwriting shows up
in the JSON's git diff, where a reviewer can catch it. Wrongly keeping the
old rule was invisible: the "Kept existing" message was logger.info, and
this package's logger sits at WARNING by default, so it never printed. The
tuning run appeared to succeed and its result was discarded in silence.
The gate was guarding against the visible failure while manufacturing the
invisible one.

So: replace unconditionally, and print old vs new config and their
latency/bandwidth deltas. print() rather than logger.info for the same
reason the old message never reached anyone. Judging whether a re-tune is
trustworthy moves to the diff, which is the one place it can be judged with
the context to do it.

Nothing in the runtime lookup path reads bandwidth_gbps -- only block_num,
rdma_block_num and warp_per_block reach LaunchParams -- so this changes no
kernel launch behaviour.
bandwidth_gbps was deliberately the slowest-rank mean rather than the
grand mean this file prints as the table's "Average" row. The reason was
the save gate: it overwrote on `new_bw > old_bw`, and every rule already
on disk held a slowest-rank value, so writing a grand mean -- which is
>= the slowest-rank mean by construction -- would have read higher than a
same-performance old rule and overwritten still-good rules on the first
re-tune. With the gate gone, nothing numeric decides overwrites, and that
reason goes with it.

Report the grand mean instead, so a saved rule's bandwidth can be checked
against a --cmd bench run of that block/warp/rdma without re-deriving
anything -- the same property _beats already relies on for latency. Which
bandwidth is the meaningful one still depends on the kernel (LL moves its
payload over xGMI, v1 over RDMA), so that pick stays; it is now shared with
the summary print via _is_ll_kernel.

Rules written earlier keep their slowest-rank value until re-tuned, so the
field is only comparable within one tuning run's output. That was already
true for any cross-run comparison and is now documented in MORI-EP-GUIDE.

The slowest-rank figure is still computed and now printed next to each
winner's table. A large gap between it and the Average means the ranks
disagree -- that the Average is smoothing over a straggler rather than
describing every rank -- which is worth seeing while reading a sweep even
though nothing selects or saves on it.
_BW_NOISE_MARGIN was an absolute 1.0 GB/s a candidate had to clear to take
the lead. That cannot hold across the operating range: where a whole sweep
sits in the low single-digit GB/s, nothing after the first candidate can
ever clear +1.0, so the sweep silently returns whatever it measured first;
where bandwidth is in the hundreds, 1.0 GB/s is under 1% and stops
filtering noise at all. The inter-node tuner had the same bug -- there it
was measured leaving the sweep effectively inert -- and the same fix, so
both now read MORI_EP_TUNING_MARGIN and one setting covers both.

Also add a tie-break. Inside the margin two candidates are not
distinguishable, and this path had nothing to break the tie, so which one
won depended on which the sweep reached first; the lexicographically
smaller (block_num, warp_per_block) now wins, making a re-tune of the same
hardware reproducible.

The comparison direction is inverted relative to the inter-node _beats:
this path selects on bandwidth, where higher is better, not latency.
Kept as a named helper so that stays explicit at each of the three call
sites rather than being re-derived inline.

Selection metric is unchanged -- still bandwidth, matching what this path
writes as bandwidth_gbps. Switching it to latency for consistency with the
inter-node tuner would be a larger change with no bug forcing it.
Full sweep on a 2x8 MI300X pair, re-run under the reworked save path so
these rules carry bandwidth_metric and a grand-mean bandwidth_gbps rather
than the slowest-rank value earlier rules held.

Geometry moved for half the shapes:

  dispatch tok=4    (16,4,8)   -> (64,4,16)
  dispatch tok=8    (64,4,42)  -> (64,4,32)
  combine  tok=4    (16,4,10)  -> (128,4,32)
  combine  tok=8    (128,4,32) -> (64,4,32)

but the latency those were picked on barely did: the eight rules move by
+1.3, -1.0, +1.2, +0.8, -0.6, -4.0, -0.2 and +0.3 percent, all inside the
10-20% run-to-run spread measured on this hardware. So this is not a
performance claim -- it records what a full sweep selects on this pair
today, with the bandwidth field on the new definition.

Do not read the bandwidth_gbps deltas in this diff as gains. The previous
values are slowest-rank means and the new ones grand means, which is a
change of definition, not of speed; avg_latency_us is the field to compare
across this commit, and the only one the sweep actually selects on.
@isytwu
isytwu force-pushed the perf/ep-small-token-host-overhead branch from b8e9954 to 501f14a Compare August 28, 2026 05:12
isytwu added 15 commits August 28, 2026 14:32
…fig.sh

The script hardcoded `sudo docker exec` to reach the peer's container,
copied from batch_internode_tuning.sh, which has done the same since it
was introduced with the tuning config system. Nothing there explains the
sudo, and `docker exec` does not need root: it needs access to the docker
daemon socket, which membership in the `docker` group grants. On a host
that grants the operator no sudo rights at all -- which is the case on the
cluster this was written for -- `sudo docker exec` fails outright while
plain `docker exec` works, so the script could not run there.

Probe instead: try `docker exec`, fall back to `sudo -n docker exec`, and
use whichever answers. `-n` on the fallback so a host that would prompt
for a password fails fast rather than hanging on a read from a
non-interactive SSH session. When neither works the error names both
attempts and the three things that actually cause it, rather than
reporting it as an SSH failure -- the old message blamed SSH for what was
usually a permissions or container-name problem.

batch_internode_tuning.sh still hardcodes sudo and has the same latent
problem; left alone here to keep this change to the file it belongs to.
…tuning.sh

Same change as the preceding commit made to verify_tuned_config.sh, which
inherited this from here. `docker exec` needs access to the docker daemon
socket, granted by membership in the `docker` group, not by being root, so
hardcoding `sudo` makes the script unusable on a host that grants no sudo
rights -- there it fails at the SSH preflight and reports the failure as
an SSH problem.

Probe `docker exec` first and fall back to `sudo -n docker exec`, so both
kinds of host work without a flag. The remaining three call sites (peer
launch, per-combo kill, pre-run kill) use the probed prefix.
Without it the script assumes rank 0 runs wherever mori is importable. When
mori lives in a container that means running the driver inside the
container -- and since the script reaches the peer over SSH, the container
then needs SSH credentials to the other node, so a private key has to be
placed inside a container purely to let the script start a second process.

--local-docker reaches rank 0's container with `docker exec` the same way
--docker already reaches the peer's, letting the driver run on the host
where usable SSH credentials already exist. Both sides are now symmetric
and neither needs a key inside a container.

`timeout` kills the `docker exec` client rather than the torchrun running
inside the container, so rank 0 gets an explicit kill after each combo too;
otherwise one hung shape leaves processes behind that poison the next one.

Verified on a 2-node MI300X EP16 setup against the committed v1_ll config
(hidden 6144, tokens 4/8/16/32): 4/4 shapes reported correct with exit 0,
and a deliberately short --timeout produced the TIMEOUT path, the hung-combo
summary, and exit 1.
…ing tools

Rejecting --save-tuning-config under MORI_TUNING_SCOPE=quick broke the
documented workflow: run_all_internode_tuning.sh defaulted to quick and
always passed a config output, so it failed all 42 jobs with no arguments,
and both "Quick sweep" commands in MORI-EP-GUIDE.md plus all three usage
examples in batch_internode_tuning.sh failed the same way. The guard was
also inter-node only -- bench_dispatch_combine.py still saved quick-scope
winners silently, and quick is if anything narrower there (3 warp_per_block
against 9, and only powers of two for block_num against a step-8 grid).

Saving now always requires the full sweep, consistently:

  - bench_dispatch_combine.py refuses quick+save, as the inter-node script
    already did.
  - Both batch scripts validate the combination up front, so it fails in a
    second with the two knobs that resolve it rather than after a two-node
    launch, once per combo.
  - run_all_internode_tuning.sh defaults to full and gains a --config-output
    passthrough, so quick stays usable for exploration via
    --tuning-scope quick --config-output ''. Without the passthrough that
    combination was unreachable through the wrapper.
  - batch_intranode_tuning.sh omits --save-tuning-config when the output is
    empty, matching the inter-node script, so --config-output '' means "do
    not save" instead of passing an empty path.
  - Docs and usage examples updated to match.
The kill chain ran as `bash -c 'pkill -9 -f torchrun; pkill -9 -f
test_dispatch_combine_internode; ...'`, whose own command line contains
every pattern it searches for. The first pkill therefore matched the shell
running it, killed it, and the remaining two never executed -- so the
python worker processes, which carry only the script path and not the word
"torchrun", survived every cleanup.

Reproduced in a container: `pgrep -af torchrun` returned exactly one
process, the `bash -c` wrapper. A verify run that hit its per-combo timeout
left the workers alive on both nodes; the next combo then contends with
them, and on this hardware leftover GPU processes have previously required
a node swap to clear.

Fix with the standard bracket form -- `"[t]orchrun"` matches a process
named torchrun but not the literal pattern text in the shell's own cmdline.
After the change the same timed-out run leaves zero strays on both nodes.

batch_internode_tuning.sh carried the same two kill strings and the same
bug; fixed there too.
The batch/run_all tuning scripts and verify_tuned_config.sh all write into
$REPO_ROOT/logs, so running any of them leaves the working tree dirty. The
existing *.log entry does not cover the directory itself or the results
markdown written alongside.
--local-docker mirrors what the preceding commit added to
verify_tuned_config.sh, for the same reason: without it the driver has to
run inside the container that has mori, and since the peer is reached over
SSH, that container then needs SSH credentials to the other node. Reaching
rank 0's container with `docker exec` locally keeps the keys on the host.
The two scripts are siblings with the same structure, so leaving the option
on only one of them made the pair inconsistent.

--rdma-tc plumbs MORI_RDMA_TC through to both ranks, alongside the
MORI_RDMA_SL that --rdma-sl already handled. build_torchrun_cmd names the
environment explicitly and the ranks start via `docker exec`/`ssh`, neither
of which inherits the caller's environment, so setting MORI_RDMA_TC on the
command line reached the wrapper and nothing else -- a tuning run would
silently use the default traffic class while appearing to honour the
setting. Found while re-tuning MI300X EP16 v1_ll, where the shipped config
was produced with MORI_RDMA_TC=41 and the run reproducing it could not pass
that value in.
Three defects in the report that replaced the keep-best gate. Since that
report is now the only thing standing between a bad tuning run and a
committed config, each of them undoes the reason the gate was removed.

Latency and bandwidth print on adjacent lines in one format, and the
percentage had one polarity for both. A 30% latency regression rendered as
"+30.0%" directly above a bandwidth line where "+" means better -- and
latency is what the inter-node tuner now selects on, so that is the line a
reviewer reads to judge a run. Label it ("+30.0%, worse") rather than
flipping the sign, which would just move the ambiguity.

`if not old:` treated an old value of 0.0 the same as no old value and
returned a bare number, making a REPLACED line byte-identical to an ADDED
one -- the old value silently dropped, which is exactly the failure this
report exists to prevent. It also short-circuited the "different metric,
not comparable" warning for those rules. `_stats_avg` in the inter-node
writer returns a literal 0.0 when the stats dict is empty, so this is
reachable rather than theoretical. Test against `is_number` instead, and
give 0.0 its own message since no ratio exists for it.

`new is None` returned "n/a" and discarded `old`, claiming no value exists
on a line where one did; print `123.45 -> n/a`. `_is_number` also rejects
a string from a hand-edited JSON, which previously reached the arithmetic
and raised TypeError.
`_beats`/`_bw_beats` said the tie-break made re-tuning deterministic
"regardless of sweep order" and would "keep working if the sweep is ever
parallelized". The opposite is true. The caller overwrites best_lat/best_bw
with the winner's score, so a tie-break win at MORI_EP_TUNING_MARGIN > 0
installs a *worse* score as the baseline and the next comparison is made
against it: the incumbent ratchets steadily worse, each step individually
"within the margin". It cannot fire today because the sweep visits
block/warp/rdma in ascending order, which makes new_cfg > best_cfg always --
so this is a trap for whoever reorders it, not a live bug. Say that, and
name what has to change first: split the margin baseline from the selected
candidate.

The view cache said the pointer in the key means "if a buffer were ever
reallocated the lookup misses and a correct view is built". That protection
is vacuous -- the pointers are snapshotted once in __init__ and never
refreshed, so a realloc leaves the key stale too. The no-realloc assumption
is pre-existing and the cache does not weaken it; claiming a guarantee it
does not have is worse than stating the assumption.

`_headline_bw` said the slowest-rank bandwidth is "printed per candidate";
it is printed once, next to the winner.
…not do

AUTO mode fails open. If the JSON for the gpu/kernel/ep is missing,
unparseable, or has no rule matching the dtype the op queries with,
dispatch_combine.py falls back to its built-in geometry, logs at DEBUG
(this library's logger sits at WARNING), and the run passes -- so the
script printed "Verified correct" having verified nothing.

That is not hypothetical. Measured against the JSON this branch ships:

  dispatch lookup(fp8_e4m3_fnuz) -> LaunchParams(64, 16, 4)
  combine  lookup(fp8_e4m3_fnuz) -> None

get_launch_config looks both phases up with config.data_type, while the
tuner saves combine rules keyed on the combine dtype, so every combine rule
misses and runs on the fallback. The script was reporting those shapes as
verified.

Add a preflight that resolves the rules through the real
TuningConfigManager and reports per phase whether the queried dtype has any
rule at all; carry the answer into the summary. Only that necessary
condition is checked, not a replay of lookup()'s ceiling/topk matching, so
it cannot drift out of step and silently start passing.

Also correct two claims in the header. The exit-status contract cited
"error times" as the failure signal -- that counter is never populated in
the test script, so it is always 0 and the branch reading it is
unreachable; the real signal is the non-zero exit from the bare asserts in
run_test_once. And the summary now states that only ranks 0-N are graded:
the peer's output is not captured and its exit status is discarded, so a
peer-side failure surfaces only as a hang.

--rdma-tc added for parity with batch_internode_tuning.sh, so a config
tuned under a traffic class is verified under it.
…e_tuning.sh

The wrapper ran all six groups and then exited 0 unconditionally, so a
caller could not tell a clean matrix from one where every group died. That
became easy to hit with the new up-front rejection: `--tuning-scope quick`
leaves CONFIG_OUTPUT at its "auto" default, batch_internode_tuning.sh
rejects each group in under a second, and the wrapper printed "Failed: 6"
and returned success.

It also forwarded neither --local-docker nor --rdma-tc, and its argument
parser rejects unknown flags -- so the two options added for containerised
two-node runs were unusable for the six-group matrix, which is the case
that most needs them. --rdma-sl, --master-port and --gpu-per-node were
missing for the same reason.

Docs: document verify_tuned_config.sh, which had none; explain
--local-docker, --rdma-tc and --rdma-sl where the docker line only
mentioned --docker/--ssh-key; and fix the JSON example, which named fields
(`rdma_bandwidth_gbps`) the writers have never emitted and omitted
avg_latency_us and the new bandwidth_metric.
…ctly

Without --local-docker the driver runs on the host, where pkill matches
every process the invoking user owns on that node -- so `pkill -9 -f
torchrun` during cleanup would take out an unrelated concurrent torch job.
Anchor both patterns on the test script's name instead. The bracket form is
kept for the separate reason it was introduced: it stops the kill chain
matching its own command line. Re-tested on a forced per-combo timeout,
which is the path that actually needs cleanup: exit 1 and zero surviving
processes in both containers, same as before the narrowing.

`${var,,}` lowercases MASTER_ADDR to master_addr and leaves the underscore,
so the three scripts told the user to pass `--master_addr`, which their own
parsers reject. Substitute dashes.
…all shapes only

Each combo runs 500 rounds and run_test_once compares output with per-token
Python loops, so cost scales with max_tokens -- measured ~40s for 500 rounds
at 4 tokens. The default --tokens-list reaches 4096, where 600s is nowhere
near enough, and the resulting message says "possible kernel hang", which
sends the reader after a nonexistent bug. Say so in the header and in the
message itself.
The check added in "stop verify_tuned_config.sh reporting a verification it
did not do" queried both phases with the dispatch dtype and so reported
combine as NO MATCH on every mixed-dtype config -- a false alarm, and worse
than the gap it was meant to close, because it tells the operator a live
config is dead.

The runtime looks each phase up with the dtype of its own input tensor:
dispatch() and combine() both call _resolve_launch_params with
dtype=input.dtype, and combine's input has already been converted to the
combine dtype (_convert_for_combine). So a combine rule saved under bf16 is
matched by a bf16 lookup and does take effect. Pass --combine-dtype through
to the preflight and check against it.

What misled the check is get_launch_config, which does use config.data_type
for both phases. It is not on the runtime path and nothing in the
repository calls it; its docstring now says so rather than leaving the next
reader to make the same inference.
Tuning already picks the two independently and saves a block/warp/rdma
triple for each, but bench shared one set, so a saved result whose two
triples differ could not be replayed. Add --dispatch-*/--combine-*; the
old flags now set both phases and the per-phase ones override them.
Flag names match the intranode benchmark, which gains the same shared
shorthand.
@isytwu isytwu changed the title perf(ep): fix the EP tuning margin and remove per-call host overhead in small-token dispatch/combine fix(ep): rework how EP tuning picks and saves, remove small-token host overhead Aug 30, 2026
@isytwu isytwu changed the title fix(ep): rework how EP tuning picks and saves, remove small-token host overhead perf(ep): rework how EP tuning picks and saves, remove small-token host overhead Aug 30, 2026
…them

Only --cmd bench forwards the block/warp/rdma flags; test, test_sentinel
and sweep construct their own ops and let the op resolve the geometry. The
flags were accepted and dropped in silence, and the preceding commit took
the count from three to nine.

This is not theoretical. Reviewing this branch, a run of
`--cmd test --block-num 8 --warp-per-block 8 --rdma-block-num 2` was used
to isolate a suspected bad geometry, and a second run at warp=6 as its
control -- both actually measured the default config, so the "control"
differed from the subject in nothing at all. A one-line warning would have
caught it immediately.

Fires before spawn, once, only when a flag was explicitly given and the
command is not bench.
@isytwu
isytwu merged commit cafdaad into main Aug 30, 2026
10 checks passed
@isytwu
isytwu deleted the perf/ep-small-token-host-overhead branch August 30, 2026 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant