Skip to content

perf(umbp): cut per-range overhead in local BatchGetRanges - #596

Merged
isytwu merged 15 commits into
refactor/umbp-backend-agnosticfrom
perf/umbp-ranged-single-node
Aug 25, 2026
Merged

isytwu merged 15 commits into
refactor/umbp-backend-agnosticfrom
perf/umbp-ranged-single-node

Conversation

@isytwu

@isytwu isytwu commented Aug 24, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

Follow-up to #589, which cut what a remote ranged get moves. This PR targets
the case where nothing is remote: a single standalone process whose inner
backend is distributed, with every key resident on this node. That is the shape
the sglang direct linker runs, and no RDMA is involved anywhere in this change.

A single-node deployment has a fair question to ask of that mode: what does it
cost over plain local mode, which has no master, no peer and no arena? Before
this series, a real amount — measured on DeepSeek-V4-Pro geometry, distributed
mode took 1.8× the TTFL of local mode at one rank. After it, distributed is
ahead of local at every rank count. The numbers are in Test Result.

The reason it was behind is that a BatchGetRanges call was spending most of
its time on per-descriptor bookkeeping rather than on moving bytes. Measured on
DeepSeek-V4-Pro's smallest pool (1,728 B fragments, 32 keys, served entirely
from the local medium):

total=149.7us
  resolve    14.9us  10.0%   per-key medium lookup
  classify   27.3us  18.2%   hipPointerGetAttributes, once PER RANGE
  build      15.1us  10.1%   TransferItem assembly
  xfer       78.7us  52.6%   plan 33.0 (22.0%)  submit 44.4 (29.7%)
  other      13.8us   9.2%   validation, metrics

The copy is under a third; per-descriptor cost is 60%. It is that high
because a layer of a page is small — a model does not have one uniform pool, and
DeepSeek-V4-Pro's three span 37,440 B, 8,448 B and 1,728 B.

The metric being optimised is TTFL: the wait for the first layer group, which is
the one no forward compute overlaps, so it lands directly in TTFT.

Technical Details

Seven commits, each independently measurable.

1. test(umbp): attribute a ranged call's time finely enough to act on

The existing UMBP_RANGED_CALL_DEBUG buckets could not answer the question they
were reached for. resolve covered the whole per-key loop, mixing the medium's
index lookup (per key) with describing the caller's buffers (per range); xfer
covered Plan, Submit and Wait together, so it could not say whether a batch of
many small ranges was paying to plan the move or to make it.

resolve splits into resolve / classify / build, xfer into plan / submit /
wait via a StepTiming the engine fills in, and the report gains items-per-call.
Sub-phases are timed nested and subtracted at the call site so they stay
disjoint. Every timer is inert when the switch is off — a null sink never reads
the clock.

Also adds --local-only to the bench, which seeds the reading node instead of
the peer so every call is served locally. Without it the bench forces every read
remote and the local path's costs are buried under the wire.

2. perf(umbp): let a ranged copy reach the non-temporal path

HostCopyBlock only used the non-temporal AVX2 copy at 256 KiB and above, a
threshold chosen when one key was one ~MiB page (the comment at the top of that
file says exactly that). A ranged copy moves one layer, so every ranged copy
fell under it and took the ordinary memcpy, paying read-for-ownership on a
destination it completely overwrites. Now 2 KiB.

3. perf(umbp): fan a local ranged copy out across threads

One memcpy stream reached the same rate whether 1 rank or 8 were copying
concurrently, so the ceiling was the stream, not the memory system. Local mode
already knew this — DRAMTier spreads a batch read over
UMBP_DRAM_READ_THREADS and its comment reads ">1 breaks the single-core memcpy
ceiling". LocalCopyEngine never did, because it was written when one key was
one whole page and the batch executors above it supplied the parallelism; ranged
I/O invalidated that by handing a single thread thousands of small copies inside
one Submit.

Two things this cost before it paid, both found by measurement and both recorded
in the commit message:

  • Spawning threads per call is worse than not parallelizing at all. Creating and
    joining threads maps and unmaps their stacks, which serializes on the process
    address space, so the damage does not scale down with helper count — capping
    the whole process to four concurrent spawned helpers still cost 25% of TTFL on
    a remote restore. Helpers are now a persistent pool.
  • The default is 2, not local mode's 4. Past 2 the local copies outrun the
    NIC's DMA into the scratch arena on a mixed workload, and on the real geometry
    4 does not pay even locally.

The asynchronous locality installer marks itself background and stays inline:
nobody is blocked on it, so borrowing cores for it only slows whoever is.

4. perf(umbp): describe a ranged caller buffer by its registration

The largest single win. BuildLocalRangeTransfers described each range with
ClassifiedUserBytes, which asks hipPointerGetAttributes what the pointer is —
a HIP runtime call per range. It is avoidable: in standalone-process mode the
server registers every client region it imports, so location and device are
already in the registration table. ClassifiedUserBytes' own comment says
registered buffers never reach it; this path just never looked.

Using the registration pays a second and larger time. A registered ref names the
region base, so every range landing in one caller buffer shares a (src, dst)
pair and LocalCopyEngine::Plan folds them into one plan whose adjacent
fragments coalesce. Describing each range by its own address made every range
its own plan, with its own four vectors, and made the coalescing that Plan
exists to do unreachable.

The registration table is now sorted by base and searched with upper_bound,
and a whole batch resolves under one shared lock instead of one exclusive lock
per range. Linear scanning was fine when lookups were per key; against a caller
holding one buffer per layer it is quadratic.

Unregistered memory still goes through ClassifiedUserBytes — for a pointer
nobody declared, interrogating it is the only correct answer.

5. perf(umbp): resolve a ranged batch per backend, report its bytes once

Two batch APIs were being called one element at a time.

BatchResolve ran inside the per-key loop as BatchResolve({keys[i]}): a
temporary vector<string> (copying the key) and a temporary result vector per
key, and one acquisition per key of the backend mutex that Allocate, Commit
and Evict also take — the mutex where every rank sharing this client
serializes. Now once per backend for the whole batch, with only unclaimed keys
carried to the next backend, so registry order and first-hit-wins are unchanged.

The byte counters were updated per key. AddCounter takes a lock and builds a
metric key from name and labels, and the help text is a hundred-odd bytes
materialized on every call. Bytes are now accumulated and reported once; totals
are identical, only the number of updates changes. Flushing is a scope guard,
not a line before each return, because this function has four exits that can
all have moved bytes.

6. perf(umbp): stop reallocating while assembling a ranged batch

local_items grew by doubling, and a TransferItem carries two TransferRefs
each holding a MemoryDesc, so every reallocation relocates a lot of bytes. The
range count is known up front and is a tight lower bound on the item count, so
the vector is reserved. MakeReadRanges returned a fresh vector per key for a
buffer dead by the next iteration; it now fills one the loop reuses.

7. test(umbp): run the ranged bench against local mode too

The bench could only drive a distributed PoolClient, so the question this PR
exists to answer had no measurement behind it. --local-mode runs the whole
bench against StandaloneClient instead — no master, no peer, no arena, no
network. Both go through one RangedClient interface so the measurement loop is
literally the same code; a second copy of the loop would make any difference
between the two unfalsifiable.

Not to be confused with --local-only, which keeps the distributed client and
merely puts every key on the reading node.

The local client is configured DRAM-only, because UMBPConfig enables an SSD
tier by default and the distributed side has DRAM only — otherwise the
comparison is between two storage stacks rather than two deployments. Measured
both ways; it makes no difference on these shapes, but the result should not
depend on that.

Where the time goes now

Same call as in Motivation — DeepSeek-V4-Pro's smallest pool, 32 keys:

              before    after
  resolve     14.9us    5.0us
  classify    27.3us    3.0us
  build       15.1us    8.5us
  plan        33.0us    2.7us
  submit      44.4us   20.7us    <- coalescing, not a faster copy
  other       13.8us    4.3us
  total      149.7us   44.4us

The copy is now the largest item, which is where a data path should be spending
its time.

A note on how this was measured

The first three commits were originally tuned against a uniform 73,728 B layer
shape and reported considerably larger wins. That shape is larger than any
pool the model actually has
, and this path is fragment-size bound — 5.4 GB/s
at 1,728 B against 37.5 GB/s at 37,440 B. Every number here was re-measured on
the real geometry after #589 landed --pools, and the commit messages were
rewritten to match. The non-temporal threshold tuned on the synthetic shape
(64 KiB) sat above all three real pools and would never have fired in
production.

Knobs

default
UMBP_DRAM_NT_COPY_MIN_BYTES 2 KiB non-temporal copy threshold
UMBP_DRAM_COPY_THREADS 2 best everywhere measured, not a compromise; no reason for a deployment to set it
UMBP_RANGED_CALL_DEBUG off the phase breakdown above

Test Plan

Image lmsysorg/sglang-rocm:v0.5.16-rocm720-mi30x-20260730, host mlx5 RoCE.
Baseline is a separate build of the merge base (bb259b50) with the same bench
binary. This machine's run-to-run spread reaches ±20%, so every reported figure
is a median of 3.

cmake -S . -B build -GNinja -DBUILD_UMBP=ON -DBUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Release
cmake --build build --target bench_umbp_pool_client_ranges -j

# this container's RC queues are smaller than mori-io's defaults
export MORI_IO_QP_MAX_SEND_WR=256 MORI_IO_QP_MAX_RECV_WR=256 MORI_IO_QP_MAX_CQE=1024
POOLS='c4:30:37440:2/2;c4_indexer:30:8448:2/2;c128:31:1728:0,1,3,5,7,9,11,13,15,17,19,21,23,25,27,29,31,33,35,37,39,41,43,45,47,49,51,53,55,57,59'
B=./build/tests/cpp/umbp/distributed/bench_umbp_pool_client_ranges

# distributed mode, every key on the reading node
$B --pools "$POOLS" --ranks 1,2,4,8 --pages 32 --requests 2 --offload-ranks 0 --local-only
# local mode, same loop and shapes -- the deployment comparison
$B --pools "$POOLS" --ranks 1,2,4,8 --pages 32 --requests 2 --offload-ranks 0 --local-mode
# must not regress: the remote path
$B --pools "$POOLS" --ranks 1,2,4,8 --pages 32 --requests 2 --offload-ranks 0
# byte check: recomputes every restored page against a position-dependent seed
$B --pools "$POOLS" --ranks 8 --pages 32 --requests 2 --offload-ranks 0 --local-only --verify

Correctness was checked on six shapes (real geometry distributed-local,
distributed-remote and local-mode, uniform local, --layer-group 1, and the
8,448 B small-object pool), and the ranged suite was also run with the
non-temporal path disabled and with the fan-out off, to confirm neither knob is
load-bearing for correctness.

Test Result

Single-machine, DeepSeek-V4-Pro geometry, 32 pages/request, layer groups of 8.
restore includes a fixed 9.15 ms of simulated compute (61 layers × 150 us).

ranks restore ms TTFL us
1 11.51 → 9.70 1.19× 1327 → 509 2.61×
2 12.24 → 9.74 1.26× 1395 → 562 2.48×
4 12.70 → 9.78 1.30× 1465 → 597 2.45×
8 13.54 → 9.98 1.36× 1771 → 769 2.30×

Subtracting the fixed compute, the load itself goes 2.36 → 0.55 ms at 1 rank
and 4.39 → 0.83 ms at 8 — 4.3× and 5.3×.

Against local mode

Same loop, same shapes, only the deployment differs (restore ms / TTFL us):

ranks local mode distributed, before distributed, after
1 9.93 / 722 11.51 / 1327 9.70 / 509
2 10.10 / 908 12.24 / 1395 9.74 / 562
4 10.74 / 1220 12.70 / 1465 9.78 / 597
8 17.57 / 1711 13.54 / 1771 9.98 / 769

Distributed mode used to cost 1.8× the TTFL of local mode at one rank. It is now
ahead of local at every rank count, by 1.4× at one rank and 2.2× at eight.

Local mode did not move — it is untouched by this PR — and its own profile
(UMBP_DRAM_PROFILE=1) says why it is now behind. At 8 ranks its DRAMTier
spends 92% of a batch in the copy, at 3.0 GB/s per stream, plus 7% classifying
pointers, with zero lock wait. Those are the same three costs this series
removed from the distributed path — a non-temporal threshold set for whole-page
copies, a HIP pointer query per range, and threads spawned per call. Local mode
still has all three; carrying the fixes across is separate work, and commit 7 is
what makes it measurable.

Worth noting for anyone reading commit 3: local mode is where the idea of
fanning the copy out came from, but its implementation spawns threads per call,
which measured worse than not parallelizing. The distributed path uses a
persistent pool instead, which is why it now beats the thing it borrowed from.

Remote path (not the target; must not regress):

ranks restore ms TTFL us
1 16.56 → 12.07 2539 → 2456
2 30.60 → 26.34 3384 → 3289
4 49.27 → 47.37 6569 → 5311
8 85.10 → 86.01 13010 → 12988

Improves or holds. The 8-rank restore reads 1% worse, which is inside this
machine's spread — repeated runs of that cell landed between 80.2 and 86.0 ms
on both sides.

Suites — all pass:

test_umbp_pool_client_ranges 24
test_umbp_range_map 13
test_umbp_ssd_ranged_io 6
test_umbp_cross_node_smoke 19
test_umbp_pool_client_batch_put 9
test_umbp_medium_selection 9
test_umbp_client_metrics 9
test_component_metrics 19
test_standalone_shm_ipc 9
test_umbp_dram_tier_ranges 13

test_umbp_pool_client_ranges additionally passes with
UMBP_DRAM_NT_COPY_MIN_BYTES set past every fragment size and with
UMBP_DRAM_COPY_THREADS=1.

Byte verification: 0 mismatched pages on all six shapes, local mode included.

Note that test_standalone_shm_ipc is not matched by CI's
ctest -R '^(umbp_|test_dummy_ssd_tier|test_prefix_aware_eviction|test_ssd_tier)'
filter, since its registered name does not start with umbp_. It exercises the
registration path commit 4 changes, so it was run by hand; renaming it is
outside this PR.

Not in this PR

Put. BatchPutRanges is untouched and does not benefit: measured, a local
ranged put spends 81% in per-key BatchAllocate and 18% in the unconditional
BatchRoutePut RPC, and 0.2% in the code this PR changes. Fixing it means
batching the backend calls and letting a local allocation answer before the
master is asked — the latter is safe (Router::BatchRoutePut is read-only; the
global index is populated by the commit-time KvEvent::ADD) but gives up
cross-node dedup and placement balancing, so it wants its own PR and a config
gate.

That work needs the bench fixed first: with the default offload load the node's
medium fills, every BatchAllocate returns no-space, and the puts fail
silently — the bench only accumulates successful bytes, so it reports
put_gibps=0 rather than an error. Anyone reading the put half of this bench's
output today is reading nothing.

Local mode's copy path. The three fixes in commits 2–4 apply just as well to
DRAMTier, which still has the 256 KiB non-temporal threshold, a
DetectPointerLocation per copy job, and a spawn-per-call fan-out. That is a
different file with different callers and belongs in its own change.

TransferRef weight. The 8.5 us of build that remains is mostly copying
MemoryDesc (three std::strings plus two 64 B arrays) twice per item. Holding
it by shared_ptr would roughly halve that, but weighted by call count build
is about 12% of the total, so the payoff is ~6% — against changing a struct the
RDMA path shares, whose only coverage is a same-host two-client smoke test. Not
worth it here.

Submission Checklist

🤖 Generated with Claude Code

isytwu and others added 4 commits August 24, 2026 08:28
The existing UMBP_RANGED_CALL_DEBUG buckets could not answer the question they
were reached for.  `resolve` covered the whole per-key loop, so it mixed the
medium's index lookup with describing the caller's buffers -- one cost per KEY,
the other per RANGE, and a ranged call carries thousands of ranges.  `xfer`
covered Plan, Submit and Wait together, so it could not say whether a batch of
many small ranges was paying to plan the move or to make it.

Splits `resolve` into resolve / classify / build, gives put its own validate
and commit, and splits `xfer` into plan / submit / wait via a StepTiming the
engine fills in.  Sub-phases are timed nested and subtracted at the call site
rather than re-timed, so they stay disjoint and `other` keeps meaning what it
says.  Also reports items-per-call, which is what makes an item-per-range
explosion visible next to the times.

Classification moves out of the page walk and into one pass over the ranges.
Same calls in the same order; a range can produce several page fragments, and
classification is a property of the range, so the walk was the wrong place to
do it whether or not anyone is timing it.

--local-only seeds the reading node instead of the peer.  Every call is then
served by the local medium and the remote phase never runs, which is the
single-machine shape: a standalone process whose backend is distributed but
whose keys all live here.  Without it the bench forces every read remote and
the local path's per-key and per-range costs are buried under the wire.

Every timer is inert when the switch is off -- a null sink never reads the
clock.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
HostCopyBlock only used the non-temporal AVX2 copy at 256 KiB and above.  That
threshold was picked for whole-object copies, where distributed mode makes one
key one ~MiB page -- the comment at the top of this file says exactly that.  A
RANGED copy moves one LAYER of one page, and layers are small: DeepSeek-V4-Pro
splits its stack across pools of 37,440 B, 8,448 B and 1,728 B per layer.
Every ranged copy therefore fell under the threshold and took the ordinary
memcpy, paying read-for-ownership to fetch destination lines it was about to
overwrite completely.

The threshold has to be set against a real model's fragment sizes.  A uniform
73,728 B shape is larger than anything the model actually has, so a value
tuned there can look convincing and still never fire in production -- which is
what happened to a first attempt at this: 64 KiB is above all three real pools.

2 KiB, measured on DeepSeek-V4-Pro's geometry with everything served from the
local medium (8 ranks, 32 pages, medians of 3): restore 12.04 -> 11.43 ms and
TTFL 1,529 -> 1,353 us.  Below 2 KiB the remaining movement is inside this
machine's run-to-run spread.  UMBP_DRAM_NT_COPY_MIN_BYTES overrides it, since
the right threshold is a property of the machine's cache.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
After the non-temporal fix the copy was still most of a local ranged call, and
one memcpy stream reached about the same rate whether 1 rank or 8 were copying
concurrently -- so the limit was the stream, not the memory system.

Local mode already knew this.  DRAMTier spreads a batch read over
UMBP_DRAM_READ_THREADS and says so in a comment: ">1 breaks the single-core
memcpy ceiling".  LocalCopyEngine never did, and its header explains why -- it
was written when one key was one whole page and the batch executors above it
supplied the cross-key parallelism.  Ranged I/O invalidated that: a layer-wise
reader hands a single thread thousands of small copies inside one Submit.

Submit now flattens its plans and runs the copies off a shared cursor, which
matters because the fragments are not equal-sized -- a range crossing a page
boundary is split, and a model's pools differ by more than 10x anyway -- so an
up-front split would not be an even one.

Two things this cost before it paid, both found by measurement:

Spawning threads per call is worse than not parallelizing at all.  Creating and
joining threads maps and unmaps their stacks, which serializes on the process
address space, so the damage does not scale down with the number of helpers:
capping the WHOLE PROCESS to four concurrent spawned helpers still cost 25% of
TTFL on a remote restore.  The helpers are now a persistent pool and fan-out
costs a condvar wake.

Fanning out past two threads does not pay.  Measured on DeepSeek-V4-Pro's real
pool geometry (medians of 3, restore ms / TTFL us):

                    threads=1        threads=2        threads=4
  local  1 rank   10.78 / 1340     10.28 / 1093     10.30 / 1118
  local  8 ranks  13.03 / 1810     11.24 / 1492     11.56 / 1669
  remote 1 rank   15.29 / 2404     13.61 / 2486     13.64 / 2342
  remote 8 ranks  81.92 / 10812    83.78 / 10787    84.16 / 11790

2 is best everywhere here, not a compromise between workloads.  Going further
costs twice: it starves a concurrent remote half, where the NIC's DMA into the
scratch arena loses to the copies and the first layer group -- the one nothing
overlaps -- waits; and it does not pay even locally, because fragments this
small do not keep four threads usefully busy.  An earlier uniform-73,728 B
shape did show 4 winning locally, but that shape is larger than any pool the
model actually has.

The asynchronous locality installer marks itself background and stays inline:
nobody is blocked on it, so borrowing cores for it only slows whoever is.

Together with the non-temporal fix, against the previous behaviour on the real
geometry: local restore 11.55 -> 10.15 ms at 1 rank and 14.14 -> 11.94 at 8,
TTFL 1,383 -> 958 and 1,958 -> 1,673.  Subtracting the bench's fixed 9.15 ms of
simulated compute, that is 2.40 -> 1.00 ms of load at 1 rank and 4.99 -> 2.79
at 8.  The remote path improves too and no shape regresses.

Bytes verified on three shapes; ranged, range-map, SSD-ranged, cross-node,
batch-put, medium-selection, metrics and DRAM-tier suites all pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BuildLocalRangeTransfers described each range with ClassifiedUserBytes, which
asks hipPointerGetAttributes what the pointer is.  That is a HIP runtime call
PER RANGE, and a layer-wise reader carries thousands per call.  It is also
avoidable: in standalone-process mode the server registers every client region
it imports, so the answer -- location and device -- is already sitting in the
registration table.  ClassifiedUserBytes' own comment says registered buffers
never reach it; this path just never looked.

Using the registration pays a second time, and the second time is larger.  A
registered ref names the REGION base, so every range landing in one caller
buffer now shares a (src, dst) pair and LocalCopyEngine::Plan folds them into
one plan whose adjacent fragments coalesce into single memcpys.  Describing
each range by its own address made every range its own plan, with its own four
vectors, and made the coalescing that Plan exists to do unreachable.

The table is now sorted by base and searched with upper_bound, and a whole
batch resolves under one shared lock rather than one exclusive lock per range.
Linear scanning was fine when lookups were per key; against a caller holding
one buffer per layer and thousands of ranges it is quadratic.

Per call on DeepSeek-V4-Pro's smallest pool (1,728 B fragments, 32 keys):

              before   after
  classify    27.3us    3.0us
  plan        33.0us    2.6us
  submit      44.4us   17.3us     <- coalescing, not a faster copy
  total      149.7us   58.4us

End to end, everything served from the local medium (medians of 3):

              restore ms      TTFL us
  1 rank     10.15 -> 9.79    952 -> 599
  8 ranks    11.90 -> 10.38  1549 -> 1001

TTFL is the metric this path exists for -- the first layer group is the one no
compute overlaps -- and it drops about a third.  The remote path improves or
holds.

Unregistered memory still goes through ClassifiedUserBytes: for a pointer
nobody declared, interrogating it is the only correct answer.

Ranged, range-map, SSD-ranged, cross-node, batch-put, medium-selection,
metrics, DRAM-tier and shm-IPC suites pass, the ranged suite also with the
non-temporal path disabled and with the fan-out off; bytes verified local and
remote.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@isytwu isytwu self-assigned this Aug 24, 2026
isytwu and others added 8 commits August 24, 2026 09:49
Two per-key costs left on the local ranged get path, both of them a batch API
being called one element at a time.

BatchResolve ran inside the per-key loop as BatchResolve({keys[i]}).  Each call
built a temporary vector<string> (copying the key) and a temporary result
vector, and took the backend mutex -- the same mutex Allocate, Commit and Evict
take.  In standalone-process mode every rank on the node shares one PoolClient,
so that mutex is where they serialize; a thousand-key call took it a thousand
times.  It now runs once per BACKEND for the whole batch.  Backends are still
tried in registry order and the first hit still wins: only keys nothing has
claimed yet are carried forward, so a second medium is asked about exactly what
the first one missed.

The outbound and inbound byte counters were updated per key.  AddCounter takes
a lock and builds a metric key from the name and labels, and the help text is a
hundred-odd bytes that has to be materialized on every call.  The bytes are now
accumulated over the batch and reported once.  Counter totals are unchanged;
only the number of updates is.

Flushing happens from a scope guard rather than before each `return`, because
this function has four exits that can all have moved bytes -- the same reason
ScopedBatchBandwidth and ScopedRangedReport are destructor-based.  It is
declared after both so it runs before either reads local_bytes.

Per call on DeepSeek-V4-Pro's smallest pool (1,728 B fragments, 32 keys):

              before   after
  resolve      8.3us    4.0us
  other       12.9us    4.3us
  total       55.8us   43.4us

End to end, everything served from the local medium (medians of 3):

              restore ms       TTFL us
  1 rank      9.79 -> 9.75     602 -> 561
  8 ranks    10.26 -> 9.95    1082 -> 745

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two allocations that scale with the batch rather than being paid once.

local_items grew by doubling. A TransferItem carries two TransferRefs, each
holding a MemoryDesc, so every reallocation relocates a lot of bytes.  The range
count is known before the loop and is a tight lower bound on the item count --
one range is one item unless it straddles a page -- so the vector is reserved up
front.

MakeReadRanges returned a fresh vector per key.  A call carrying thousands of
ranges over hundreds of keys paid one allocation per key for a buffer that is
dead by the next iteration; it now fills a vector the loop reuses, so the call
pays one.  Renamed FillReadRanges to say so.  The write side keeps
MakeWriteRanges: its result is stored per request rather than consumed
immediately, so there is nothing to reuse.

Per call on DeepSeek-V4-Pro's smallest pool (1,728 B fragments, 32 keys):

  build   12.6us -> 8.5us

End to end, everything served from the local medium (medians of 3): TTFL
561 -> 521 us at 1 rank, unchanged at 8 where the copy dominates.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bench could only drive a distributed PoolClient, so "how much does the
distributed deployment cost over the local one on a single machine" had no
answer -- and that is the question a single-node deployment actually has.

--local-mode runs the whole bench against StandaloneClient instead: no master,
no peer, no scratch arena, no network.  Both deployments go through one
RangedClient interface so the measurement loop is literally the same code; a
second copy of the loop would make any difference between them unfalsifiable.

Not to be confused with --local-only, which keeps the distributed client and
merely puts every key on the reading node.

The local client is configured DRAM-only.  UMBPConfig enables an SSD tier by
default, which brings a demotion pipeline and its own threads, and the
distributed side of the comparison has DRAM only -- leaving it on would compare
two storage stacks rather than two deployments.  (Measured both ways; it makes
no difference on these shapes, but the comparison should not depend on that.)

On DeepSeek-V4-Pro geometry, 32 pages/request, groups of 8 (medians of 3,
restore ms / TTFL us):

  ranks   local mode      distributed
   1     9.93 / 722      9.69 / 502
   2    10.10 / 908      9.75 / 561
   4    10.74 / 1220     9.81 / 624
   8    17.57 / 1711    10.01 / 793

Distributed is now ahead everywhere, which it was not before this series: at
the merge base it read 11.51/1327 at 1 rank against local's 9.93/722.

Local mode's own profile says why it did not move.  At 8 ranks its DRAMTier
spends 92% of a batch in the copy, at 3.0 GB/s per stream, plus 7% classifying
pointers, with zero lock wait.  Those are the same costs this series removed
from the distributed path -- a non-temporal threshold set for whole-page
copies, a HIP pointer query per range, and threads spawned per call -- and
local mode still has all three.  Fixing them there is separate work; this
commit is what makes it measurable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nning

Two defects in the pool added by "fan a local ranged copy out across threads",
both of which a 224-core idle box hides and a real inference process does not.

The pool was sized to `helpers`, which with the default of one helper per call
meant ONE thread for the entire process.  The second concurrent caller onward
got no help at all -- and still paid the queue lock, the wake and the wait.
Worse, a worker popped a batch on claim, so even with more threads exactly one
of them could ever work on a given batch.  The pool now grows against the idle
count up to a process cap, leaves a batch queued while it is being drained so
several helpers share its cursor, and lets the publisher remove it.

Completion was a `std::this_thread::yield()` spin.  A spin costs nothing when
there is a spare core and the process has nothing else to do, which describes
the bench and not a server that is also running the model's forward pass, the
HIP runtime and every other rank.  It is now a condition variable, notified by
whichever thread finishes the last item under the mutex the waiter checks its
predicate on.

Fixing these changes the tuning conclusion.  The default was 2 because 4 looked
like it cost a concurrent remote half its first layer group, which I attributed
to the copies outrunning the NIC's DMA.  That was wrong: it was the starved
pool and the spin.  With them fixed, on DeepSeek-V4-Pro geometry (medians of 3,
restore ms / TTFL us):

                    threads=1        threads=2        threads=4        threads=8
  local  1 rank    9.88 / 690       9.71 / 518       9.61 / 425       9.57 / 381
  local  8 ranks  10.06 / 853      10.03 / 723       9.83 / 611       9.94 / 682
  remote 1 rank   13.23 / 2294     12.06 / 2229     11.50 / 2304     11.51 / 2284
  remote 8 ranks  84.44 / 12399    90.55 / 12860    82.58 / 11384    79.78 / 11600

4 is now best or tied-best everywhere, and matches local mode's
UMBP_DRAM_READ_THREADS.  8 buys a little at one rank and gives it back at
eight.

Found because an end-to-end run regressed where the bench did not.  The bench
cannot see either defect by construction: it is the only thing running, on far
more cores than it uses.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ExecuteLocalPutRangesBatch called BatchAllocate, BatchAbort and BatchCommit one
element at a time -- the same pattern the get path had, but it hurts a put far
more, and an end-to-end profile put a number on it: allocation was 68% of all
time spent in BatchPutRanges.

The reason is batch size.  A put's ranges must tile whole objects, so one page
uses up a large share of the per-call range budget and an offload call carries
single-digit keys where a load call carries hundreds -- measured end to end,
about 7 against 512.  A per-call cost is then amortized over almost nothing,
and there are two orders of magnitude more calls to pay it.  Every one of those
calls took the backend mutex once per key, the same mutex the load path's
resolve takes, on a client every rank on the node shares.

Requests are now grouped by the backend that will hold them, so each backend
sees one BatchAllocate for the call, one BatchAbort for everything that has to
be released, and one BatchCommit.  Ordering is unchanged: commit still happens
strictly after the transfer, and a slot that fails to build, transfer or commit
is still aborted -- only the number of calls changes.

The byte counters are aggregated the same way the get path's now are: one
update per call instead of two per key.

`items` is also reserved from the range count, which for a put is exact -- a
put's ranges tile the object, so no range can straddle two pages.

Local ranged put on the bench: allocation fell from about 39 us per key to
about 2.  The bench cannot show the end-to-end effect, because its own offload
workload fills the medium and then fails every allocation with no-space (see
the note in the PR); the shape of the fix follows from the profile, not from
the bench.

Ranged, batch-put, cross-node, SSD-ranged, medium-selection, metrics, shm-IPC,
range-map, component-metrics and DRAM-tier suites pass; bytes verified on six
shapes including one with offload running.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Self-review found this: moving the bench onto a RangedClient interface
collapsed a per-key metric into an all-or-nothing one.

Before the RangedClient abstraction, local_write_mib was computed by resolving
a pool's keys directly against the reading node's backend and crediting
object_bytes() for every key that resolved -- partial credit, since an
asynchronous install landing SOME of a pool's objects before others is exactly
the case this metric exists to show. Folding that resolve behind the new
AllResident() (a single bool) turned it into all-or-nothing: unless every key in
the pool happened to be resident at the same instant, the whole pool reported
zero, understating -- often to zero -- an install that was genuinely in
progress. --verify does not catch this because it checks bytes, not this
column.

AllResident's other call site (classifying a whole load call as local for
local_hit_pct) genuinely wants all-or-nothing, so the fix is not to make
ResidentCount answer both questions with one bool -- it's to have both callers
derive their own answer from one per-key count: local_hit_pct compares the
count to the chunk size, local_write_mib multiplies it directly. One
primitive, two derivations, instead of two notions of "resident" that could
drift apart.

No production code touched -- this is bench-only, and the fix is scoped to the
regression it introduces.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rollup was only emitted when a call happened to land more than one
cadence window after the previous emit. A measured phase shorter than
the cadence therefore printed no line at all -- not a small number, no
output -- and so did the final partial window of a long run. That is
indistinguishable from the instrumentation being switched off, and it
silently costs whoever is measuring a whole run.

RangedStatsInstance is a deliberately leaked singleton, so there is no
destructor to hang the flush on; an atexit hook gives it back without
reintroducing static-destruction ordering against the loggers it uses.
It runs after main returns, so the hot path is unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nd too

A standalone server only declared the GPU mapping it imports over IPC to
its inner backend when that backend was Local. Skipping it under
Distributed is correct -- an unregistered device pointer is classified
rather than assumed host (ClassifiedUserBytes), so HbmCopyEngine still
claims the pair and the bytes land where they should -- but it is not
free, and the cost is not where you would look for it.

A range that misses the region table is described by its OWN address
instead of its region's base. Engine planning folds items into one plan
per distinct (src base, dst base) pair, so sharing a base collapses a
whole batch into one plan while distinct addresses give every range a
plan of its own, each constructing two TransferRefs' worth of
MemoryDescs. It also costs a hipPointerGetAttributes per range.

Measured on DSv4-Pro / TP8, single node, 256K prompt at 100% hit, with
only this declaration flipped:

  plan            23.2%  ->   1.4%  of a ranged call
  classify         5.0%  ->   2.3%
  mean_call       4,911  -> 3,465 us
  load residual   179.5  -> 118.8 ms   (-33.8%)
  TTFT            367.4  -> 304.1 ms   (-17.2%)

submit -- the phase that actually moves bytes -- is unchanged at ~1,550
us, so the whole saving is bookkeeping. Against the same-machine,
same-build Local inner backend (84.3 ms residual) the gap goes from
2.13x to 1.41x.

This also explains why two earlier attempts at this path did nothing:
both cut fragments per range, and plan count follows RANGES, not
fragments, so neither could move it.

Rather than add a second entry point, RegisterMemory takes a
MemoryRegistration mode. kPinned is the default and behaves exactly as
before; kLocalCopyOnly records the region for engine selection without
handing it to the IO engine, because an RDMA MR over an IPC-imported
ROCm mapping buys nothing and its dmabuf fallback is unsafe. Every
engine's Deregister already no-ops on a descriptor-less ref.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@isytwu
isytwu force-pushed the perf/umbp-ranged-single-node branch from 482367d to d3be4e4 Compare August 25, 2026 07:07
@maning00

Copy link
Copy Markdown
Contributor

Independently verified this series from the sglang side (we are the consumer of the direct linker this targets). It does what it says — but it does not build the Python bindings, which is how sglang consumes mori, so that needs one fix before we can adopt it.

Blocker: BUILD_UMBP=ON pip wheel fails on this branch

FAILED: src/pybind/CMakeFiles/mori_pybinds.dir/pybind_umbp.cpp.o
pybind11/pybind11.h:535:13: error: static assertion failed due to requirement
  'expected_num_args<...>(sizeof...(Args), ...)':
  The number of argument annotations does not match the number of function arguments

Commit 4 gives IUMBPClient::RegisterMemory a 5th parameter (MemoryRegistration mode, umbp_client.h:162), but git diff --stat <base>..HEAD -- src/pybind/ is empty: the .def still declares four py::args, and MemoryRegistration is not bound as a Python enum at all.

It slipped through because the Test Plan builds only bench_umbp_pool_client_ranges. cmake --build build --target mori_pybinds (or a plain pip wheel) reproduces it immediately.

Two additions fix it:

+  py::enum_<MemoryRegistration>(m, "UMBPMemoryRegistration")
+      .value("Pinned", MemoryRegistration::kPinned)
+      .value("LocalCopyOnly", MemoryRegistration::kLocalCopyOnly)
+      .export_values();
+
   py::class_<UMBPHbmConfig>(m, "UMBPHbmConfig")
       .def("register_memory", &IUMBPClient::RegisterMemory, py::arg("ptr"), py::arg("size"),
            py::arg("loc") = mori::io::MemoryLocationType::CPU, py::arg("device") = -1,
+           py::arg("mode") = MemoryRegistration::kPinned,
            py::call_guard<py::gil_scoped_release>())

With those, the wheel builds clean and every existing 2- and 4-argument register_memory call still works. All numbers below were taken on that build.

Verification, on a different instrument and geometry

One MI355X node (n09-29), image lmsysorg/sglang-rocm:v0.5.17-rocm720-mi35x-20260820, baseline bb259b50 vs this branch (d3be4e4c), both through StandaloneProcessClient (so a gRPC hop your C++ bench does not have).

Model-free probe — replays exactly what the sglang linker puts on the wire, sweeping fragment count at fixed fragment size (8192 B) so the marginal per-fragment cost is separated by construction rather than fitted:

bb259b50 this branch
local, µs/fragment 0.466 0.459
distributed, µs/fragment 1.227 0.480
distributed − local +0.761 +0.021
distributed / local 2.63× 1.05×

97% of the distributed-specific per-fragment penalty is gone. Two things corroborate the mechanism you describe rather than something else moving: the local arm does not budge (0.466 → 0.459), matching "local mode did not move"; and the marginal bandwidth is unchanged on both arms (local 9.76 → 9.96, distributed 8.85 → 9.15 GB/s), so the win is bookkeeping removal, not a faster copy.

Real connector, topology 3 — DeepSeek-V4-Pro real weights, TP8, 64 GiB hugepage tier, paired local/distributed arms back-to-back in one session:

tokens delta (dist − local), bb259b50 delta, this branch
16,384 +18.3 ms +10.3 ms (−44%)
65,536 +45.7 ms +13.6 ms (−70%)

device (arm-independent) drifts −6.1% / −8.3% between arms; charging all of that against distributed as a conservative bound puts the two deployments at parity (0.9× at both lengths, from 7.5× and 3.8× before). Correctness also passes: 8/8 linkers report ranged_io=yes, cache_test_len 3/3, and a per-token restore_equivalence probe returns umbp == device at both 16,384 and 32,768.

Your finer buckets confirm the shape you were aiming for — classify 0.6%, plan 1.0%, submit 85.5%. The copy is the dominant item now.

One data point for the follow-up put PR

You note a local ranged put spends 18% in BatchRoutePut, alongside the caveat that the bench's put half is unreadable once the medium fills. On the real connector with this branch applied we measure:

SUMMARY PUT calls=1005 bytes=41.28GiB items_per_call=1793
  | resolve=28.2% classify=0.1% build=1.8% route=50.6% xfer=19.0%(plan=0.4% submit=18.3%)

route is 50.6%, and we independently measured 60.4% on an earlier run with a different tier. So the unconditional BatchRoutePut looks considerably more expensive on real traffic than the bench suggests — worth sizing against the per-key BatchAllocate work before choosing an order for that PR.

Happy to re-run any of the above, and to test a revision on the sglang path.

read_lease_until_ was an unordered_map keyed by the same string as
owned_, so resolving a key hashed it twice: once to find the slot, once
to renew its lease. On DSv4-Pro the keys are ~128 bytes, both maps hold
several hundred thousand entries, and the whole thing runs under the one
mutex every rank on the node shares.

Moving the deadline into OwnedSlot means the renewal goes through the
iterator the lookup already produced. Evict and ClearLocal likewise
already hold the slot. The reaper sweep that dropped expired leases goes
away with the map it existed to bound -- an expired deadline sitting in
a slot costs nothing and is overwritten by the next resolve.

BatchResolve also reads the clock once per batch instead of once per
key. A batch carries a thousand keys and completes in well under a
millisecond against a TTL measured in seconds.

Measured on DSv4-Pro / TP8, single node, 256K prompt at 100% hit:

  resolve         39.8%  -> 23.7%  of a ranged call (1,378 -> 664 us)
  mean_call       3,463  -> 2,800 us
  load residual   118.3  ->  96.5 ms   (-18.4%)
  TTFT            307.8  -> 281.7 ms

submit is unchanged, so this is bookkeeping again. Semantics are
unchanged: a lease still only exists for an owned key, still fences
eviction, and still expires by wall time rather than by being erased.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
isytwu added a commit that referenced this pull request Aug 25, 2026
Giving IUMBPClient::RegisterMemory a fifth parameter left the pybind
.def declaring four py::args, which is a compile-time failure in
pybind11's arity static_assert -- so BUILD_UMBP=ON pip wheel did not
build, and sglang consumes mori as a wheel.

It survived review because the bindings are not in the build that was
tested: the tree used for the benches and ctest is configured with
-DBUILD_PYBINDS=OFF, so a clean `ninja` there says nothing about them.

Binds MemoryRegistration as an enum, gives `mode` its annotation and
default, and re-exports the enum from mori.umbp alongside the other
UMBP types. Existing two- and four-argument register_memory calls are
unaffected -- the default is kPinned, which is the old behaviour.

Reported by maning00 on #596.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Giving IUMBPClient::RegisterMemory a fifth parameter left the pybind
.def declaring four py::args, which is a compile-time failure in
pybind11's arity static_assert -- so BUILD_UMBP=ON pip wheel did not
build, and sglang consumes mori as a wheel.

It survived review because the bindings are not in the build that was
tested: the tree used for the benches and ctest is configured with
-DBUILD_PYBINDS=OFF, so a clean `ninja` there says nothing about them.

Binds the pinned form explicitly rather than widening the Python
signature. MemoryRegistration picks whether a region is pinned for RDMA
or only recorded for local copies, and that is the deployment's call --
the standalone server derives it from its inner backend mode. Callers
that reach this binding are registering their own buffers and always
want them pinned: sglang's linker passes (ptr, size) and lets the
process client classify the rest. Offering a knob there that no caller
should set, and that the server would override anyway, is worse than
not offering it.

So the Python surface is byte-identical to before: register_memory(ptr,
size, loc=CPU, device=-1) -> bool, and no enum is exported. The C++ side
keeps its single entry point.

Reported by maning00 on #596.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@isytwu
isytwu force-pushed the perf/umbp-ranged-single-node branch from ebecdd0 to 5e7e654 Compare August 25, 2026 14:24
setuptools 84 requires build_ext.run() to have initialized the compiler
before build_extension() is called, and CMakeBuild calls
build_extension() directly. With no upper bound, build isolation pulls
the newest setuptools and the wheel dies in

  AssertionError: run() must precede build_extension()

which is what a clean checkout of this branch does today: BUILD_UMBP=ON
pip wheel fails before it reaches any of our code. Carried over from the
earlier work on this branch, where it was fixed locally but never
committed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@isytwu
isytwu merged commit d828579 into refactor/umbp-backend-agnostic Aug 25, 2026
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.

2 participants