Skip to content

perf(umbp): partial-range remote fetch + asynchronous locality install - #589

Merged
isytwu merged 11 commits into
refactor/umbp-backend-agnosticfrom
perf/umbp-ranged-partial-fetch
Aug 24, 2026
Merged

isytwu merged 11 commits into
refactor/umbp-backend-agnosticfrom
perf/umbp-ranged-partial-fetch

Conversation

@isytwu

@isytwu isytwu commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

What

A remote ranged get pulled the whole object into the scratch arena and then copied the caller's ranges out of it, installing the object into the local medium synchronously, inside the arena lock.

sglang's direct linker loads KV by layer group: the same page keys, ceil(num_layers / UMBP_LAYER_GROUP) calls, each taking one slice of every object. So reading 8 layers cost 61 layers' worth of wire — and all of it landed on the first group, the one call a layer-wise reader has no compute to overlap with, so it went straight into TTFT.

This PR fetches only the requested spans, and rebuilds locality without paying for it twice.

group g:
  remote RDMA moves only this group's ranges  ->  scratch arena (packed)
                                              ->  CopyContiguousToRanges -> caller
  then exactly one of:
    arena slice already IS the whole object  ->  install from it, zero extra wire
    otherwise                                ->  queue a background whole-object pull
later groups:
  local medium hit -> phase 1, no arena, no lock
  not yet landed   -> another range-granular remote read

No protocol change, no engine change. The peer's BatchResolveKeys reply already carries the full page list and its memory descriptors, and TransferPlan is already a scatter-gather of (src_offset, dst_offset, size).

Performance

DeepSeek-V3.2 shape: 61 layers × 73,728 B → 4,392 KiB objects, 8-layer group slice 576 KiB, 32 pages/request, 256 MiB arena, medium page = object. Median of 3 runs — this machine's single-run variance reaches ±20%.

ranks restore p50 (ms) TTFL p50 (μs)
1 38.48 → 23.60 1.63× 18,620 → 3,910 4.8×
2 42.00 → 27.70 1.52× 17,884 → 3,800 4.7×
4 76.39 → 31.02 2.46× 48,439 → 5,303 9.1×
8 139.73 → 40.53 3.45× 112,015 → 7,858 14.3×

ttfl is the stall before the first layer group is readable — the part of a layer-wise load that nothing can hide. With concurrent offload, 8 ranks: restore 154.69 → 45.03 ms, TTFL 127,747 → 8,312 μs, get 4.46 → 11.70 GiB/s, put 5.34 → 8.77 GiB/s.

Bytes on the wire (rank 1, 3 requests, 137 MiB working set each): 1,372 MiB → ~491 MiB total (0.36×). The ranged path alone is 6.4× lighter; the locality pull accounts for the rest and can be switched off, which takes it to 432 MiB with zero local writes.

Other shapes — no regression outside noise except one, noted below:

shape restore TTFL
128 pages, rank 4 2.60× 11.9×
503 KiB objects, 128 pages, rank 8 1.86× 7.0×
30 KiB objects, rank 8 1.01× 2.2×
1 page/request (no batching), rank 8 1.05× 2.2×
layer_group=2, rank 8 4.57× 59.5×
whole-object read (layer_group=61) neutral neutral
strided ranges (nothing coalesces), rank 8 3.54× 15.6×

The strided row is deliberately adversarial: layers are dealt round-robin to groups, so byte count, call count and range count are unchanged and only adjacency is gone.

One shape is genuinely slower

503 KiB object + layer_group=1 + 128 pages + 8 ranks: restore 11% slower (113.87 → 126.15 ms median of 7), TTFL 3.1× faster.

Not a bug — the cost of making the install asynchronous, at the point where it is largest. The synchronous install won by construction; with 61 groups there are 61 chances to lose the race, remote calls go 16 → 35, and at 1 MiB per call the fixed cost (peer resolve RPC + lock) dominates so the 61× byte saving cannot pay for it. In this shape the new path moves more total bytes than the old one; what it buys is that the 1 GiB is off the critical path.

Needs all three of: tiny group, tiny object, high concurrency. sglang's default is UMBP_LAYER_GROUP=8, where the same object is 1.86× faster.

Also fixed

  • BuildRemoteGetTransfers described its destination with the object size. A ranged entry's arena slice is the span total, so this makes FindRegisteredMemory's containment check reject a slice near the end of the arena and silently downgrade a zero-copy read to a staged one.
  • Remote GET byte counters reported the object size — in ranged mode that overstates remote bandwidth by the whole read-amplification factor.
  • The ranged path's synchronous install never consulted cache_remote_fetches or the admission gate, so a deployment that had explicitly disabled re-caching still had every remotely fetched object written into its local medium. sglang's direct connector is exactly such a deployment.
  • BatchRouteGet does not touch the arena, so it no longer holds every other arena user behind it.

Config

One new knob, ranged_locality_prefetch (default on). Separate from cache_remote_fetches on purpose: that one gates a re-cache which copies out of the caller's buffer, which a GPU-destination deployment cannot do and therefore has to turn off. This one never reads the caller's buffer — it pulls peer pages straight into a freshly allocated local slot — so the same deployment can keep locality.

Turning it off costs cross-request locality and, at 8 ranks, 3.8× on restore: without any local hits every group goes through the arena and its lock. Worth it only on a single-rank deployment or a bandwidth-starved link.

Commits

  1. refactor — extract the object-range → page-range walk into a shared header, unit-test it. No behaviour change.
  2. perf — fetch only the requested spans.
  3. feat — rebuild locality: install from the arena when it holds the object, otherwise pull in the background.
  4. test — 12 new cases.
  5. docs — full before/after numbers, method, and what was not measured.

Testing

test_umbp_range_map 13/13 (new) · test_umbp_pool_client_ranges 19/19 (7 pre-existing + 12) · test_umbp_cross_node_smoke 19/19 · test_umbp_pool_client_batch_put 9/9 · test_umbp_medium_selection 9/9.

Image lmsysorg/sglang-rocm:v0.5.16-rocm720-mi30x-20260730, ROCm 7.2, mlx5 RoCE. This container's default 8192 send-WR trips ibv_create_qp EINVAL, so runs need MORI_IO_QP_MAX_SEND_WR=256 MORI_IO_QP_MAX_RECV_WR=256 MORI_IO_QP_MAX_CQE=1024.

Not in this PR

Zero-copy straight into the caller's registered destination, which would remove the arena hop and the arena lock entirely. It is blocked on standalone_server.cpp:990, which registers an IPC-imported GPU mapping for RDMA only in Local mode. That restriction is an assertion in a comment — nothing in the tree records it being tried and failing — and this host has the prerequisite (amdkfd peer-memory client, 1.48M MRs to date). Worth a probe before committing to it.

Caveats

  • The bench uses fresh keys per request, so it cannot see cross-request prefix reuse — the locality prefetch's numbers here are a lower bound.
  • Measured 20 configurations; the 7 adversarial ones were designed by reasoning about where this would break, not by enumeration.

🤖 Generated with Claude Code

@isytwu
isytwu force-pushed the perf/umbp-ranged-partial-fetch branch from 3071e14 to bf3825a Compare August 21, 2026 04:10
The mapping from a caller's byte range onto the pages backing an object was
inlined in BuildLocalRangeTransfers, reachable only through the local medium
path. The remote path is about to need the identical arithmetic against a
peer's pages, and an error in it does not fail: it reads or writes the wrong
bytes of a KV block while reporting success.

Extract it as ForEachRangePageFragment in a new internal header, rewrite the
local builder on top of it, and unit-test it directly. Behaviour is unchanged.

The direct test covers the shapes that are awkward to reach through the live
two-node fixture and were therefore untested: a short last page, a range
ending exactly on the object's last byte, one byte past the end, a slot that
does not start at page 0, and a page count that cannot hold the object.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@isytwu
isytwu force-pushed the perf/umbp-ranged-partial-fetch branch from 91270f2 to 47ccac8 Compare August 21, 2026 10:37
isytwu and others added 5 commits August 21, 2026 10:47
A remote ranged get pulled the WHOLE object into the scratch arena and then
copied the caller's ranges out of it. A layer-wise KV reader asks for one
layer group of many over the same keys, so this moved an order of magnitude
more bytes than it used, and it moved all of them on the FIRST group -- the
one call a layer-wise reader has no compute to overlap with, so it lands
directly in TTFT.

Fetch only the caller's spans, packed back to back into the arena. The peer's
BatchResolveKeys reply already carries the full page list and its memory
descriptors, and TransferPlan is already a scatter-gather of
(src_offset, dst_offset, size), so no protocol or engine change is involved.

Adjacent spans are merged before items are emitted rather than left to the
engine's planner. The planner would coalesce them too, but only after sorting
and bucketing every one: a 61-range whole-object read would build 61
TransferItems per key just to merge them back into one, and that CPU is
invisible next to a multi-MiB object and very visible next to a small one.

The packing unit is now (key, span prefix) instead of key, so an object larger
than the arena is no longer unservable -- only a single span larger than the
arena is. A key's units are built aside and spliced in only once the whole key
has been laid out: a later range that cannot be served has to discard the
earlier ones too, or the key would be fetched in part and still reported
whole, which the caller has no way to detect.

Also here, because the same lines are involved:

  * BuildRemoteGetTransfers described its destination with the OBJECT size.
    A ranged entry's arena slice is the span total, and passing the object
    size makes FindRegisteredMemory's containment check reject a slice near
    the end of the arena, silently downgrading a zero-copy read to a staged
    one instead of failing.
  * Remote GET byte counters reported the object size, which in ranged mode
    overstates remote bandwidth by the whole read-amplification factor.
  * BatchRouteGet never touches the arena, so it now runs before the arena
    mutex is taken instead of holding every other arena user behind it.
  * ExecuteRemoteBatchGetPlan splits the submit-all/wait-all half out of
    ExecuteBatchGetPlan; the ranged plan has no local half by construction,
    so it no longer threads three vectors nothing reads.

Restoring locality after a ranged read is a separate concern and follows in
the next change; this one leaves the remote path without a local install.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous change left the remote ranged path without a local install,
because a ranged read holds only the caller's spans and a medium slot is
whole-object: committing one would publish a key whose remaining bytes are
undefined. Without an install every layer group of a cold key goes back to the
peer, and on a shared PoolClient each of those takes the arena mutex.

Restore it two ways, exactly one of which applies per fetched key:

  * The arena slice already IS the object -- the caller's ranges tiled it in
    ascending order and fit in one unit, which is what a whole-object reader
    produces. Install straight from the arena, before the next sub-batch
    reuses that slice. Zero extra bytes on the wire.
  * Otherwise queue a background pull of the whole object from the peer into
    a freshly allocated local slot.

The ascending requirement is not pedantry: the arena is packed in caller
order, so ranges that cover every byte out of order (a packed multi-component
KV pool emits K and V interleaved) leave an arena layout that is not the
object. Those fall through to the pull.

The pull is peer pages -> medium slot pages as plain RDMA: no arena, no bounce
buffer, no memcpy, and the caller's destination is never read. That last point
is why this gets its own switch instead of reusing cache_remote_fetches: the
existing re-cache copies out of the caller's buffer, which a GPU-destination
deployment cannot do and therefore has to turn off.

It is BATCHED, and it has to be. Per key the fixed cost is a peer resolve RPC
plus an RDMA submit/wait, and one worker thread pays it. Done one key at a
time that cost dominates the bytes for small objects so thoroughly that the
worker cannot keep up with the reader, the prefetch loses more races than it
wins, and the feature turns negative -- measured 6% slower at 4 ranks and 16%
at 8 on a 503 KiB object. Draining the queue in one pass per peer makes the
same shape 1.41x and 1.96x faster instead.

Deduplicated twice over: at enqueue against keys already in flight, and in the
worker against keys that became local while the job waited. The second check
is what stops a whole object of wire being spent on a key that is already
here.

The destination is named by the ref the backend publishes rather than a raw
pointer. UserBufferRef only knows regions registered through RegisterMemory,
so a medium pool -- RDMA-reachable but never handed to RegisterMemory -- would
not be found, and the transfer would fall through to a bounce the staging pool
may be too small for.

Also fixes an inconsistency the ranged path had from the start: its
synchronous install never consulted cache_remote_fetches or the admission
gate, so a deployment that had explicitly turned re-caching off still had
every remotely-fetched object written into its local medium.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fixture's scratch arena is a plain vector the test owns, which makes it
the oracle: fill it with a known byte and whatever the wire did not bring back
still holds the poison. RemoteRangedFetchMovesOnlyRequestedBytes uses that to
assert both halves of the claim in one case -- the requested spans arrived,
packed in caller order, and nothing else in the arena was touched. A
whole-object fetch would have overwritten all of it.

The rest:

  * RemoteRangedFetchCrossesPageBoundary -- an object that is not a page
    multiple, one range straddling a page and one ending on the object's last
    byte. This is the short-last-page clamp on the REMOTE builder, which
    nothing reached before.
  * MultiLayerGroupsOverSameKeys -- the sglang shape: the same key set, once
    per layer group, each call taking a different slice. Byte-exact at the end.
  * OversizedRangeFailsItsKeyWholeAndAlone -- an object larger than the arena
    whose last range is larger still. The earlier ranges have already been
    split into their own fetch unit by then, so this pins the rule that a key
    is delivered whole or not at all: the caller cannot detect a half-filled
    buffer behind a true result. Fails without the guard in the previous
    commit.
  * ObjectLargerThanArenaIsServedAcrossSubBatches -- the same split path when
    every range is servable.
  * CompleteArenaObjectIsInstalledWithoutRefetching and
    OutOfOrderFullCoverageDoesNotInstallFromArena -- the two sides of the
    install-vs-pull decision, including the case where full coverage arrives
    out of order and must NOT be installed from the arena.
  * LocalityPrefetch{InstallsWholeObjectAfterRangedRead,OffLeavesNothingLocal,
    DedupsAcrossLayerGroups} -- the pull lands the whole object and not just
    the slice that was read, the switch really disables it, and eight layer
    groups over one cold key schedule one pull rather than eight.
  * MixedLocalAndRemoteRangedKeys, PartialFailureAttributionAcrossKeys.

RemoteRoundTripSubBatchesAndInstallsLocally now asserts the payload rather
than the route. Its old comment claimed the first get "synchronously installs"
-- true then, and it is what the second get relied on; now the install is
asynchronous so either route is correct and only the bytes are.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A ranged read whose ranges tile the object in order already has the object's
exact layout, so staging it in the arena buys nothing: allocate the medium slot
first, let the peer write straight into it, serve the caller from there, and
commit. Committing IS the local install, so locality costs nothing extra, and
these keys never touch the arena or its mutex -- which on a PoolClient shared
by every rank on the node is what they were really queueing for.

Measured on the whole-object shape, medians of 3, against 767056f:

  61 layers in one call, 4 ranks:   142.50 -> 38.05 ms   (3.7x)
                                    TTFL 133,277 -> 28,848 us
  503 KiB objects, 8 ranks:         173.38 -> 50.14 ms   (3.5x)
                                    TTFL 164,177 -> 40,928 us

with the same bytes installed locally as before. The layer-wise shape is
untouched: its ranges are one group of many, never the whole object, so it
does not take this path and its numbers do not move.

Falls back to the arena when the medium has no slot to give or hands back one
that is not a single contiguous run. That decision is made on the allocation,
which is local and cheap; never after a failed transfer, because a transfer
does not fail on account of where it was pointed and retrying it elsewhere
would only double the latency of a failure the caller treats as fatal.

The copy-out happens before the commit on purpose: a pending slot cannot be
reclaimed, a committed one is an ordinary evictable object and reading it
afterwards races the allocator. The commit happens even if the copy-out fails,
because the slot holds the object either way and the next reader still wants
it; only the caller's result goes false.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cases added so far read one or two keys from one thread, and the paths
this series added only diverge under load: whether a key is already local when
a call starts, whether a slot can be had, whether the background pull lands
before the next layer group asks. All of that is decided by a race, and none
of it was being checked against the bytes.

ConcurrentMixedShapesAllReturnCorrectBytes drives layer-wise readers and
whole-object readers at the same keys from four threads over three rounds, and
compares every restored page afterwards. The caller's medium is deliberately
too small to hold the key set, so slot allocation fails part of the time and
the arena fallback runs too.

A read is allowed to fail here -- the medium really is too small -- but it must
never return the wrong bytes, so the assertion separates the two.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@isytwu
isytwu force-pushed the perf/umbp-ranged-partial-fetch branch from 47ccac8 to 7eeb883 Compare August 21, 2026 10:49
@isytwu isytwu self-assigned this Aug 24, 2026
isytwu and others added 5 commits August 24, 2026 04:47
Every performance number claimed by the five commits before this one came from
this file, and it was not in the tree, so none of them could be reproduced.

It drives BatchGetRanges the shape the sglang tree connector does, which is the
shape the optimisation targets and the one no existing bench covers: N ranks
walking the same keys in layer groups, a loader thread fetching group g+1 while
a forward thread "computes" group g.  Both endpoints are PoolClients in one
process with different node ids, so a read is genuinely routed remote --
BatchRouteGet, the RDMA path, the scratch arena and its mutex are all the real
ones -- while the bytes cross a local NIC.  Absolute bandwidth is therefore
optimistic; the structural results are not.

Latency alone cannot tell whether this series works, so it also reports:

  ttfl_us         time the forward thread waits for the first group.  The one
                  span of a layer-wise load that no compute overlaps, so it is
                  where moving only the requested ranges shows up.
  wire_whole_mib  what a whole-object fetch would have moved
  wire_range_mib  what a range-granular fetch actually moves
  repeat_remote   keys one request had to fetch remotely more than once, which
                  is what falls when locality is rebuilt between groups
  local_write_mib what the asynchronous install landed here; must be zero
                  under --no-prefetch, or the switch is not doing anything

--verify seeds each object with SeedByte(key_index, offset) rather than one
repeated byte and recomputes it after every request.  A constant fill cannot
catch a misplaced range, which is the failure mode a range-granular fetch has
and a whole-object fetch does not.  The check runs after the timing is recorded
so it cannot inflate per-request latency.

Not a CTest target: it takes minutes and its output is numbers to read.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bench took one --layers and one --layer-bytes, so every range in a run was
the same size.  No model is shaped that way: DeepSeek-V4 spreads its layers over
six pools whose per-layer sizes span more than an order of magnitude -- 37,440 B
for c4, 8,448 for its indexer, 1,728 for c128 -- and this path is fragment-size
bound.  Measured on that geometry, 1,728 B ranges reach 5.4 GB/s against
37.5 GB/s at 37,440 B, so a uniform run samples one point on a 7x curve.

Which point it sampled mattered.  The default, 61 x 73,728 B, sits past the fast
end of that curve, where per-descriptor cost counts for least -- precisely the
cost that moving only the requested ranges, and gathering on device, exist to
remove.  At equal bytes per page the uniform shape reports 7.93 GiB/s against
the real three-pool shape's 3.89, and a TTFL of 1,667 us against 6,143.

--pools takes name:layers:layer_bytes[:first/stride | :l1,l2,...] entries.  The
layer placement is not decoration: pools interleave, so it decides which groups
reach which pool, the loop _plans_covering runs in the connector.  It also has
to accept an explicit list, because c128 holds layers 0,1,3,5,...,59 -- the
first two adjacent and the rest every other one, which no stride expresses.
That geometry is not guesswork; it was dumped from a 262,144-token restore on
DeepSeek-V4-Pro, and its (30+30+31)x1023 growth ranges plus 121 from the three
pools that do not grow with length account for the 93,214 that load issued.

Omitting --pools synthesises a single pool from --layers/--layer-bytes, so
command lines written before this option describe the workload they used to.

Output gains a per-pool split, because the totals cannot show the thing several
pools exist to expose: c4_indexer and c128 are two thirds of the descriptors and
a fifth of the bytes, and only the split says which pool a change helped.
ServeWholeObjectUnitsFromMedium served the caller by handing the slot's raw
`host_ptr` to CopyContiguousToRanges, which wraps whatever it is given in
TransferRef::HostBytes -- i.e. unconditionally loc=CPU, device=-1.

That is only true for the scratch arena, which was the function's sole caller
before. A medium slot is whatever the medium is: with `medium = HBM`,
PageBackend::BufferRef() hands back a hipMalloc'd device pointer and a ref
whose loc is GPU. The guard checked HasHostPtr(), which is just a null test and
passes for device memory, and never looked at loc.

So on an HBM-medium node every whole-object ranged read was broken, in one of
two ways depending on where the caller's buffer lives:

  * host destination -- both endpoints claim CPU, LocalCopyEngine takes the
    pair, and it memcpy's from a device pointer;
  * device destination -- HbmCopyEngine takes it, but KindFor() reads the
    direction off the source classification and issues HostToDevice with a
    device src, which errors out and fails the key.

Pass the ref the backend already published, plus the slot's byte offset, and
keep its loc/device. The existing pointer overload becomes a one-line wrapper
that says the arena is host memory, which is the one place that claim belongs.

Also: report the bytes this path pulled. remote_bytes was only accumulated by
the arena loop, and a batch whose units are all slot-served returns before
reaching it -- so ScopedBatchBandwidth saw zero and ObserveBatchBandwidth
dropped the sample. The ranged-GET remote bandwidth metric went blank for
exactly the shape this path exists to make fast. The per-key wire counters in
FinalizeRemoteGetEntries were unaffected and stay as they are.

Not covered by a test: the fixture's medium is DRAM, so reaching the broken
path needs an HBM-backed PoolClient in the two-node fixture.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ranged_locality_prefetch` gated all three ways a remote ranged read can leave
the object on this node, but they do not cost the same thing:

  * the arena slice already holds the whole object -> install from it;
  * the ranges tile the object -> let the peer write into a medium slot and
    commit it;
  * anything else -> fetch the object a SECOND time in the background.

Only the third costs anything on the wire -- up to one extra copy of the
object. The first two are landing bytes that crossed the wire regardless, and
the pre-ranged code did exactly that on every remote ranged read,
unconditionally. Gating them behind a switch whose documented job is "the
background pull" made turning that pull off silently give up the free install
too, which is a pure loss: no traffic saved, locality gone, and nothing in the
switch's name or docs to warn you.

Split the predicate. CanInstallLocally() asks the only question the free paths
need -- is there a medium to install into -- and LocalityPrefetchAdmits() keeps
the switch and the re-cache admission cap for the pull that pays for them.

This also removes an unintended size regression: routing the free paths through
ShouldAdmitReCache meant objects over admission_max_block_bytes (16 MiB by
default) stopped being installed at all, where the pre-ranged code installed
them regardless.

The switch was also unreachable from any deployment. It defaults to on, and
neither pybind nor the standalone server's environment exposed it, unlike
cache_remote_fetches which has both. So a GPU-destination deployment that had
deliberately set cache_remote_fetches=false could not decline the background
pull, the medium capacity it consumes, or the re-cache worker thread that
Init() now starts for it. Add the binding and
UMBP_DISTRIBUTED_RANGED_LOCALITY_PREFETCH.

pure-ssd-mode.md said cache_remote_fetches=false "keeps remote-fetch cost
honest by not re-caching". That has not been true for ranged reads since they
grew their own switch; say which flag covers which path.

To be explicit about something an earlier commit message got wrong: the ranged
path still does not consult cache_remote_fetches. LocalityCachingAdmits passed
a hardcoded true and its successors keep doing so. That is deliberate -- the
two flags gate paths with different failure modes, and the sglang connector
turns cache_remote_fetches off because the non-ranged re-cache copies out of
the caller's GPU buffer, which this path never touches -- but the commit that
introduced it claimed to have fixed an inconsistency it did not fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All three passed with the behaviour they were named for removed, so each was a
false signal about coverage that existed.

RangedFetchSplitsWhenSpansExceedArena never split. kObjectSize == kScratchSize
== 8192 and third == 2730, so the three spans totalled exactly 8192; the split
test is `> scratch_size`, which 8192 is not. Worse, the spans then tiled the
object in order, so the unit took the slot path and never touched the arena at
all -- it was a third copy of WholeObjectReadBypassesTheArena. A single object
the size of the arena can never split, because its ranges cannot total more
than it. Rewritten with an object larger than the arena and gaps between the
spans, so the total exceeds the arena AND the read is not a tiling one, which
is the case ObjectLargerThanArenaIsServedAcrossSubBatches does not cover: a
split on the arena path rather than on the slot path. Renamed to say so. It now
fails if the split is disabled.

LocalityPrefetchDedupsAcrossLayerGroups asserted nothing about dedup. Deleting
prefetch_inflight_ leaves every one of its assertions passing -- a duplicate
pull is absorbed by the worker's BatchResolve check and by
kSuccessAlreadyExists. The eight groups also run sequentially, so by the second
one the key is usually already local and no second prefetch is ever scheduled;
the racy state the set exists for is not even reachable. Renamed to what it
does test (the sglang shape against a cold key) and the comment now says
plainly that dedup has no external observable and is not pinned here.

ConcurrentMixedShapesAllReturnCorrectBytes asserted the opposite of its own
contract. Its comment, and the commit that added it, say a read may
legitimately fail because the medium is deliberately too small -- then it
asserted failures == 0. It also byte-checked keys whose read had failed, whose
buffers are untouched, so any capacity refusal would have been counted as
corruption as well. Now only keys the client said it served are compared,
failures are logged rather than asserted, and the mismatch count keeps its
teeth.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@isytwu
isytwu merged commit c82a8ba into refactor/umbp-backend-agnostic Aug 24, 2026
isytwu added a commit that referenced this pull request Aug 24, 2026
…stall counter

Two follow-ups to #589, both code-health rather than behaviour.

ExecuteRemoteBatchGetPlan was introduced as "the submit-all/wait-all half split
out of ExecuteBatchGetPlan", but nothing was actually taken out of the latter:
the two carried the same reserve, the same submit loop and the same wait loop,
differing only in whether run_local() sat between them. Give the shared one an
optional in-flight window and have ExecuteBatchGetPlan pass run_local into it.
The overlap it exists for is preserved -- local reads still run after every
peer is posted and before any is waited on -- and the ranged callers, which
have no local half by construction, pass nothing.

MORI_UMBP_METRIC_RANGED_REMOTE_INSTALL_FAILURES_TOTAL had narrowed to one
branch. It used to fire on every failed install on the ranged path; after the
locality rework it fired only from MaybeInstallCompleteArenaObject, which is
now the least-taken of the three routes. A slot the medium refused, a slot that
is not one addressable run, a refused commit, and every failure of the
background pull were all silent apart from a debug log -- that is, the counter
named for "the object did not become local" stopped covering most of the ways
that happens, while the feature it measures grew two new ones.

Route every such case through one helper. The background pass reports its total
in one go rather than per key, since it is there to say how much locality is
being lost, not to trace individual keys.

Also drops commit_index, written and never read since the pull was batched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TianDi101 pushed a commit that referenced this pull request Sep 3, 2026
…stall counter

Two follow-ups to #589, both code-health rather than behaviour.

ExecuteRemoteBatchGetPlan was introduced as "the submit-all/wait-all half split
out of ExecuteBatchGetPlan", but nothing was actually taken out of the latter:
the two carried the same reserve, the same submit loop and the same wait loop,
differing only in whether run_local() sat between them. Give the shared one an
optional in-flight window and have ExecuteBatchGetPlan pass run_local into it.
The overlap it exists for is preserved -- local reads still run after every
peer is posted and before any is waited on -- and the ranged callers, which
have no local half by construction, pass nothing.

MORI_UMBP_METRIC_RANGED_REMOTE_INSTALL_FAILURES_TOTAL had narrowed to one
branch. It used to fire on every failed install on the ranged path; after the
locality rework it fired only from MaybeInstallCompleteArenaObject, which is
now the least-taken of the three routes. A slot the medium refused, a slot that
is not one addressable run, a refused commit, and every failure of the
background pull were all silent apart from a debug log -- that is, the counter
named for "the object did not become local" stopped covering most of the ways
that happens, while the feature it measures grew two new ones.

Route every such case through one helper. The background pass reports its total
in one go rather than per key, since it is there to say how much locality is
being lost, not to trace individual keys.

Also drops commit_index, written and never read since the pull was batched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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