Repository navigation
Feat/umbp tree connector port - #569
Merged
TianDi101 merged 19 commits intoAug 17, 2026
Merged
Conversation
Records why this is not a merge, which upstream files can be taken verbatim
and which must be hand-merged, and what the distributed side has to
reimplement rather than port.
The load-bearing decisions, so they survive the session:
- pool_client.cpp is reimplemented. Upstream's five edit sites are
functions Phase 6 deleted, and FindRegisteredMemory changed type.
- Three files auto-merge cleanly but wrongly. Worst is our
StandaloneProcessClient::RegisterMemory throw, which silently kills the
GPU-IPC path upstream adds.
- Three upstream pieces are already ours: GPU pair dispatch
(HbmCopyEngine::CanHandle), run coalescing (both local engines merge at
plan time), and loc/device in the RegisterMemory API.
- Distributed ranged I/O stays a warn-once stub, as upstream ships it.
- Whole-object I/O is not collapsed into the degenerate ranged case;
a port that also redesigns is not reviewable against its source.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
These 24 files are ones the backend-agnostic refactor never touched, so
upstream's final state is exactly what a merge would produce. Taking them
whole avoids replaying nine sequential cherry-picks through dram_tier.cpp,
which every one of those commits edits.
What arrives here:
- Ranged multi-buffer I/O in the local tier stack: TierBackend and
LocalStorageManager gain range-aware read/write, DRAMTier implements
them, and range_utils.h holds the shape and tiling validators that all
three deployment modes share.
- GPU buffers in standalone-process mode: the server maps worker regions
by GPU IPC handle, and umbp_standalone.proto carries the handle,
device id and range descriptors.
- The DRAM tier's GPU-side performance work: whole-tier host
registration on a background thread, a gather kernel for scattered
copies, one stream per copy thread, and UMBP_DRAM_PROFILE counters.
The distributed data plane is untouched by this commit, so it cannot
regress here. Build wiring for the new sources lands next.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…registration Reconciles the shared files between the two branches. IUMBPClient gains BatchGetRanges/BatchPutRanges, and all three implementations satisfy them: StandaloneClient and StandaloneProcessClient for real, DistributedClient as a warn-once stub exactly as upstream ships it (see the port plan, §5 — the transfer layer can already express a range, the missing piece is the object-range to page-range mapping). Two reconciliations worth calling out, because git merges both of them cleanly and wrongly: RegisterMemory keeps our four-argument form rather than reverting to upstream's two. In StandaloneProcessClient the throw on loc != CPU is gone, replaced by upstream's GPU-IPC path, and classification of the pointer is authoritative with loc/device only filling in a device ordinal that hipPointerGetAttributes could not supply. Trusting the pointer over the argument is deliberate: a connector written against the two-argument API leaves loc at its CPU default, and honouring that would register device memory as host memory — surfacing later as a memcpy from device memory, far from the call that caused it. StandaloneServer::RegisterBackendMemory now takes loc/device and the GPU-IPC path passes them. The server's mapping of a worker's device allocation is device memory in the server process too; registering it as host memory would send it down the inner client's host paths, which is a memcpy from device memory on a Local backend and a host staging bounce on a Distributed one. This half of the fix is invisible in the upstream client diff. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The three gaps that upstream's 200-line pool_client.cpp diff exists to close. They close smaller here because HbmCopyEngine already claims any pair with a GPU endpoint and derives the hipMemcpyKind from it — no call site needs its own hipMemcpy. 1. Classify the caller's buffer instead of assuming host memory. TransferRef::HostBytes stamps loc=CPU, and CanHandle dispatches on loc, so an unregistered device pointer was claimed by LocalCopyEngine and memcpy'd. ClassifiedUserBytes fixes both sites: UserBufferRef's unregistered fallback (the remote paths) and BuildLocalPageTransfers (the self-target path, which built the user ref directly and so was not covered by UserBufferRef at all). One classification per batch, not per page. 2. Refuse to stage GPU memory through the host bounce pool. MoriIoEngine decided bounce eligibility on HasHostPtr() alone and then memcpy'd in and out of a host region. IsBounceable additionally requires loc==CPU, so an unregistered GPU endpoint is rejected at Plan and reported per key through ApplyRejectedTags. Upstream spreads this over five guards at PoolClient call sites; one predicate in the transfer layer covers all of them, and it is the only layer that knows whether staging is involved. 3. Harden RegisterMemory: reject a re-registration that grows the region (the old one would silently under-cover the tail), validate that the ref the engine returned actually matches the requested base, size, loc and device, and turn a throwing engine into a false return rather than letting it escape through pybind. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ient
Upstream's two multi-page byte-exact cases port unchanged — they drive the
public BatchPut/BatchGet and so exercise our LocalCopyEngine coalescing the
same way they exercised upstream's BuildTierPageRuns. Every other case in this
file uses exactly one page per key, so the merge path had no coverage, and a
wrong run boundary still returns true.
Two cases neither branch had, for the port's actual subject:
- A device source and destination on the self-target path, multi-page with a
short tail. This is what BuildLocalPageTransfers used to mislabel as host
bytes, sending the pair to LocalCopyEngine.
- An unregistered device source on a batch that can route remotely. Routing
has no local affinity, so this cannot assert a per-key disposition; it
asserts the invariant instead — the vector length survives, and any key
reported as succeeded reads back byte-exact. A remote-bound key comes back
false through ApplyRejectedTags rather than being memcpy'd out of device
memory, which is a fault and not a corruption, so the pre-port behaviour
would have taken the suite down here.
Both GPU cases skip when no HIP device is present. A skip is not a pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t does
The first version of this case allowed either disposition per key, on the
assumption that route-put has no local affinity. In this fixture it does not
need one: caller_ is pinned to a single page precisely to force remote
routing, which is what StagingFallbackSucceedsWithoutWarn already relies on.
Measured on a live 2-node fixture, all four keys are rejected.
So the case now asserts the contrast directly — the same batch shape that
succeeds for every key with a host buffer must fail for every key with a
device buffer — plus two things the earlier version left open:
- the "transfer unplannable" WARN is emitted, which pins the failure to
MoriIoEngine refusing the GPU endpoint rather than to anything else that
can also fail a put. UmbpLogCapture forces the module to WARN, which is
why the message is observable in the test and not in a default-level run.
- no key is published, so a later Get cannot hand back whatever the staging
buffer happened to hold.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replaces the planned verification section with measured results, and the
planned stage list with the commits as landed.
Three findings worth keeping:
- There is no cmake target for src/pybind/pybind_umbp.cpp; it is compiled by
pip install, so a plain cmake --build never touches it and a bad binding
surfaces only at install time. The doc records the hipcc -fsyntax-only
invocation that checks it, including the two include paths that are easy
to miss (-I . and build_check/src/umbp/proto_gen).
- This host has /dev/kfd and /dev/dri but no /dev/infiniband, and the live
PoolClient tests pass anyway — so the known infiniband requirement applies
to the multi-node suites, not to these.
- The image sets ENTRYPOINT ["bash"], so --entrypoint bash is required or the
command is handed to bash as a filename.
Also states what was NOT verified: the multi-node suites, GPU-IPC registration
against a live standalone server, and any throughput measurement.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ffer hole Answers "which modes does this actually support" per feature, because the two features do not line up and the answer inverts between them. GPU buffers work in all three deployment modes, and in distributed mode across every medium. Ranged I/O only works where the ultimate backend is Local, and only on the DRAM tier — TierBackend's default ranged methods return all-false and only DRAMTier overrides them, so this is narrower than "Local mode" even before the DistributedClient stub is considered. The new follow-up is a gap inherited from upstream, not introduced here: SSDTier::ReadIntoPtr preads directly at the caller's address and neither ssd_tier.cpp nor local_storage_manager.cpp has any device awareness, so in Local mode a device destination works for a DRAM hit and not an SSD hit. Distributed mode has no equivalent hole, which is the refactor paying off rather than luck: every byte there moves through the transfer layer, so device-awareness is one property of TransferRef instead of a per-tier concern. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Port of upstream 93f2998. Moves HostTierRegistration and LaunchDeviceGather from DRAM-tier-private headers into src/umbp/include/umbp/common/, leaving forwarding shims at the old paths so the tier and its tests keep compiling unchanged. The distributed ranged-I/O work needs both helpers outside the local tier, which is what this move is for. Deviation from upstream: upstream's move rewrote the two headers' comments down to a few lines. This keeps the original rationale — the measured per-fragment costs that justify the kernel over hipMemcpy/hipMemcpy2DAsync, the ~128 KiB crossover, and why registration is single-shot and all-or-nothing — because those numbers are why the code is shaped this way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Port of upstream 1d3c859. Large offload bursts sent every unacknowledged event bundle in each heartbeat RPC. Once a request exceeded the 3-second deadline inherited from RpcShutdownTimeoutMs(), no bundle was acknowledged, so the next heartbeat resent the old backlog together with newly queued events — a self-amplifying loop; the triggering 8-rank run observed 921 DEADLINE_EXCEEDED failures. Split drained delta events into bounded bundles, cap the bundles per RPC, and schedule another heartbeat immediately when the ack advances. Heartbeat RPCs get a deadline independent of the shutdown timeout. Configurable through UMBP_HEARTBEAT_RPC_TIMEOUT_MS (3000), UMBP_HEARTBEAT_EVENTS_PER_BUNDLE (1024) and UMBP_HEARTBEAT_MAX_BUNDLES_PER_RPC (16). master_client.cpp took upstream's change as-is; the only reconciliation is that the drain reads DrainAllBackends(Backends()) rather than the deleted DrainAllSources(owned_sources_). The test needed rebuilding. Upstream's fake implements OwnedLocationSource and attaches via AddOwnedLocationSource, both of which Phase 3 deleted — the cherry-pick auto-merges clean and does not compile. The backlog is now built by committing keys into a MockBackend (a full MediumBackend already in the tree) and attaching it through a BackendRegistry, which drives the same DrainAllBackends path production uses. The registry is attached after the commits, since SetBackendRegistry installs the auto-flush hook that would otherwise drain the backlog before it accumulates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Port of upstream 875d4ac, and the end of the port doc's §5 non-goal: ranged multi-buffer I/O now works against a Distributed backend instead of returning the warn-once stub. §5 predicted this would be much smaller here than upstream's shape suggests, and it is. MediumBackend has no data-movement virtual, and every byte already moves as a TransferItem carrying src_offset/dst_offset/size — a TransferItem *is* a range. So no backend, engine or wire-format change was needed, and the whole mapping is one function, BuildLocalRangeTransfers, which intersects each caller range with the pages backing the object. Whole-object I/O is the same mapping with the range pinned to [0, size); it keeps its own builder, since §5 also ruled out collapsing the two paths inside a port. What upstream needed and this does not: - No RangeCopyRun planner, no AppendRangeCopyRun merging, no per-endpoint hipMemcpyKind, no ScopedHipDevice at the call site. The engines already coalesce adjacent segments while planning and derive kind and device from the endpoint pair (port doc §2). - No assembly buffer for a locally-routed put. Upstream assembles scattered ranges into the arena and writes the object; here the ranges are written straight into the slot's pages, because the transfer layer takes offsets. Deliberate differences from upstream, both visible in behaviour: - A remote object is served to the caller from the scratch arena, and the local install is a caching side effect whose failure no longer fails the read. Upstream installs first and serves through the tier, so an install failure loses a fetch it already paid for. The install-failure metric is kept and now counts exactly that: a missed caching opportunity. - No DRAM/HBM tier filter on remote ranged reads. Upstream needs one because its remote path is a hand-written DRAM RDMA read; ours fetches the whole object through ExecuteBatchGetPlan, which reaches any medium the owning peer publishes, SSD included. - Per-key results instead of all-or-nothing. Items are tagged with the caller's key index, so the engine's failed tags map back per key. The gather kernel lands in HbmCopyEngine rather than at the call sites, which is the §4.2 argument again: the engine is the only layer that sees the segment shape a copy decomposes into, so it is the only one that can tell when a kernel beats hipMemcpy. Bucketing is per device across the whole batch, not per plan — a caller reading three ranges into three separate GPU allocations produces three single-segment plans, which is nothing to gather within any of them and exactly the scattered small-fragment batch the kernel wins on together. This is also what makes the local ranged put batch across keys: one kernel for the batch, not one per key. PoolClient declares the medium's host buffers and the scratch arena as gather regions; every other GPU transfer in the tree gets the same fast path for free. Standalone-process mode advertises the inner backend's mode and ranged support through Ping, so a connector can ask rather than assume. GPU-IPC registration now accepts a Distributed backend, and — following upstream — does not register the imported mapping with it: an RDMA MR buys nothing for a local HIP copy endpoint and the dmabuf fallback is unsafe for IPC-imported ROCm mappings. That is safe here only because an unregistered device pointer is classified rather than assumed host (§4.1), so HbmCopyEngine still claims the pair. SupportsRangedIO is keyed on the selected medium (not SSD) rather than upstream's !ssd.enabled, which predates the single-medium selector. Tests: test_pool_client_ranges.cpp is upstream's, rebuilt against this architecture — the fixture drives BackendRegistry/MediumBackend instead of the deleted PeerDramAllocator, and the backend self-allocates its pool. All four cases pass, including the two that assert the gather kernel actually ran. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Upstream was rebased and grew three commits since the first round. Records the
new provenance (and the range-diff showing the rebase left the original nine
byte-identical), retires the §5 non-goal now that distributed ranged I/O is
implemented, updates the support matrix, and adds §10/§11 for what the three
commits needed here.
The two things most worth having written down:
- The heartbeat fix's test cherry-picks clean and does not compile — it uses
OwnedLocationSource, which Phase 3 deleted. That is §1's "auto-merges
cleanly but wrongly" hazard showing up again in round two.
- The gather kernel must bucket per device across the whole batch, not per
plan. Per-plan bucketing is correct byte-for-byte and silently never
launches a kernel, so only the launch-count assertions catch it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Upstream c0044f8. The arena defaulted to 256 MiB, so every distributed client allocated and RDMA-registered it whether or not it ever issued a ranged operation. Zero is now the default and means "this deployment does not do ranged I/O": no allocation, no registration, and SupportsRangedIO() reports false so a caller can ask instead of discovering it as a failed batch. Validate() no longer rejects zero, and neither does the standalone server's UMBP_DISTRIBUTED_RANGED_SCRATCH_BYTES parsing. Two deviations from upstream, both because our side already differs here: - SupportsRangedIO() gains the arena test rather than being replaced by it. It stays keyed on the selected medium as well (§9): the arena is what the remote direction needs, and the medium is what decides whether object ranges can map onto published in-process endpoints at all. Both conditions are necessary and neither implies the other. - StandaloneServer's shared_reads_ keeps our predicate. Upstream now gates concurrent standalone reads on SupportsRangedIO(), which on this branch would serialize every read of a distributed SSD-medium deployment — the pure-SSD mode c12c68e just landed — for a reason that does not apply: the RPCs under that lock are whole-object reads, and the only two paths that touch the arena guard it with ranged_scratch_mutex_ and bail out cleanly when it is absent. The upstream cleanup paths that null the DRAM pool alongside the arena have no counterpart here; the DRAM backend has owned its own pool since Phase 2b, and HostMemAllocator::Free is already a no-op on an unallocated handle. The test is upstream's, adapted to drive a real DistributedClient through CreateUMBPClient — the opt-in lives in the constructor and in SupportsRangedIO(), neither of which the bare PoolClient fixture sees. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds §12 and §13, updates the provenance header and the §9 support matrix.
Three things worth having written down:
- The rebase over the pure-SSD/multi-drive work cost one conflicted line
(an include). §1 predicted this shape from the other direction, so the
number is recorded rather than treated as luck — and the two things it
could plausibly have broken (ShardedSSDTier's inherited ranged stubs,
the SSD test targets under the port's rewritten CMakeLists) were checked
instead of assumed.
- Ranged I/O is now opt-in, so a connector must set ranged_scratch_size
and read supports_ranged_io() back. That is a behaviour change for
anything written against §10.3.
- §11's account of the umbp_local_client failure was wrong, and §13 says
so. It is not order-dependent and it does not pass standalone: it aborts
on test_gpu_put_get in 4 of 12 runs from a neutral working directory,
with the gather kernel off as well as on. The pre-rebase head gives the
same 4 of 12 on the same assertion, which is what makes it inherited
rather than a regression — that measurement is the reason to trust the
claim, so it is recorded with the caveat that its throwaway build also
fails an unrelated follower test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The fixture guessed a port in [55500, 57000) from a time-seeded rand and hoped it was free. On a loaded node it is not: these tests run inside a --network host container sharing the host's port space with every other container on the machine, and the collision surfaced as `WakesHeartbeatThreadImmediately` failing its `ASSERT_NE(server_, nullptr)` with the real cause — EADDRINUSE — only visible in gRPC's log noise above it. Binding "127.0.0.1:0" and reading the assigned port back through AddListeningPort's out-parameter removes the guess. This is the same fix, and the same idiom, as FreePort() in test_pool_client_ranges.cpp. Measured on smci355-ccs-aus-n06-21, a node with a dozen other containers running: 1 failure in 20 runs before, 0 in 40 after. Four sibling tests keep the same guessed-port helper (test_batch_route_get_columnar, test_master_client_lifecycle, test_umbp_client_metrics, test_umbp_tags). None of them failed in this run; they are the same latent flake and worth the same change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follows the flush-heartbeat fix. The four siblings were not all the same
problem, so they do not all get the same change.
test_batch_route_get_columnar and test_master_client_lifecycle guessed from
a time-seeded range the way flush-heartbeat did (52000+ and 54000+). Both
build their own grpc::ServerBuilder, so both now bind ":0" and read the
assigned port back out of AddListeningPort.
test_umbp_tags and test_umbp_client_metrics already asked the kernel, via an
AllocPort() that binds :0, reads the port, and closes. That is much better
than a guess but still leaves a window in which the port can be taken before
the real server binds it. Closed wherever the bound port can be read back
afterwards instead: the tags E2E gRPC server through AddListeningPort, and
both fixtures' MasterServer through listen_address ":0" + GetBoundPort().
test_umbp_client_metrics' io_engine.port is now 0 for the same reason, which
also retires its retry-until-unique helper.
One window cannot be closed that way. MasterServerConfig::metrics_port is a
number, 0 already means "no metrics server", and nothing reports what it
bound, so it must still be chosen before MasterServer::Run() binds it.
Losing that race is not a test failure but a std::terminate — MetricsServer's
constructor throws on the Run() thread — which is exactly how it showed up:
terminate called after throwing an instance of 'std::runtime_error'
what(): [metrics] bind() on port 46077 failed: Address already in use
Both fixtures now run Run() under a try/catch and retry the whole server on
a fresh metrics port.
Also: test_umbp_tags' AllocPort returned a hardcoded 50400 on failure, which
is the collision it exists to prevent, given two callers. It throws now.
Measured on smci355-ccs-aus-n06-21 alongside a dozen other containers:
30 standalone runs each of the four fast targets, 10 of client_metrics, and
10 full-suite runs — all clean. Before, the full suite failed roughly 1 run
in 6 on umbp_tags.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Upstream ec72ec1 and 0a17427, taken together — the second is a follow-up that replaces the first's temporary CI step, so landing them separately would add and remove the same block. The launch-counter assertions need the DRAM tier's host memory registered for GPU access. That registration is best-effort by design: where hipHostRegister does not take, Covers() stays false, the planner leaves the batch on the copy engine, and the counter never moves while every byte expectation still holds. Asserting it is asserting a machine capability, which is how three DeviceGatherTest cases and PoolClientRangesTest.GpuRangesUseGatherKernel came to fail on `0 vs 0` upstream with nothing actually wrong. Both files probe the registration once and gate only the counter checks on it. test_dram_tier_gather.cpp gains its own main() so the skip exits 77 and lands in the ctest summary rather than reading as a pass; the CI net effect collapses to raising the log level, since the availability step 0a17427 removes is one ec72ec1 added. Applied without conflict, including into this branch's rebuilt test_pool_client_ranges.cpp — the probe sits beside FreePort() and the two guards land on the assertions our version kept from upstream's. Verified on smci355-ccs-aus-n06-21 that this did not just turn the coverage off: all five DeviceGatherTest/HostTierRegistrationTest cases and all five PoolClientRangesTest cases report [ OK ], not SKIPPED, on both the kernel and kernel_off variants. The registration takes on this host, so the launch counters are still being enforced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
§14 covers the fourth round. Both branches moved within hours of round three: backend-agnostic gained one Python-only commit, upstream two test commits. The rebase was byte-identical across all sixteen and the pick was clean, which is worth recording precisely because rounds one to three were not — that is the port converging. The part worth reading is the check, not the change. A commit whose content is "skip this assertion sometimes" is §1's auto-merges-cleanly-but-wrongly hazard in its friendliest form: had the probe returned false here, the build would pass, the suite would be green, and the gather coverage would be gone. §14.2 records that ctest -V shows [ OK ] and not SKIPPED on all ten cases. §15 documents the test-port flakes found running §13's suite on a loaded node. Kept as its own section rather than folded into verification because the five tests did not share one problem — three guessed a port, two raced for one, and one window cannot be closed without a production API change this branch deliberately does not make. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three failure modes hit while benchmarking DRAM/HBM/SSD on n06-21. All three produce numbers that look healthy, which is what makes them worth fixing in the harness rather than documenting. Undersized pools evict mid-run. The master's eviction manager walks every (node, tier) every 5s and, once used/total crosses EvictionConfig:: high_watermark (0.9), evicts down to low_watermark (0.7). Both are deliberately not env-overridable, so the only lever is staying under the watermark. A 32 GiB dataset in a 34 GiB pool reports a clean WRITE_DONE ok=16384 fail=0 and then misses 25.6% of reads with no eviction line in any client log: 94% trips 0.9, and 0.7*34816MiB/2MiB = 12185 survivors, exactly the hits measured. Size capacity from the dataset and the watermark instead, for DRAM, HBM and SSD alike -- HBM's flat 4 GiB default would have kept ~1400 of 16384 keys, and SSD is watermarked twice (master plus PeerSsdManager) and charges in 4 KiB-padded record bytes. An explicit env value that invites eviction now warns and predicts the survivor count. This is a race, not a hard cap, so a short run can beat the check -- small probes are misleading and the sizing must not depend on one. The SSD staging arena died registering for RDMA. It is auto-sized from the sweep (routinely 8 GiB+) and pinned twice, hipHostRegister then ibv_reg_mr; at 4 KiB pages the second fails at ~2M pages with errno 12, which is fatal -- the writer dies and the reader only sees ReadMessageHeader(type) failed ... EOF, with nothing pointing at page size. Default the arena to hugepages; HostMemAllocator still falls back to anonymous pages with a warning where no pool exists. A hugepage shortage died as a bare SIGBUS. The DRAM pool, staging arena and read buffer are all hugepage-backed and bound to --numa-node, and hugepages are reserved per node, so a healthy /proc/meminfo total means nothing to a pinned process. mmap(MAP_HUGETLB) succeeds against the reservation, MADV_POPULATE_WRITE then returns EFAULT, and the process dies faulting pages in. Raising the pool needs root and silently dropping to 4 KiB pages would invalidate the comparison, so this one preflights and exits with the shortfall, the per-node nr_hugepages command, and the opt-out. Verified on n06-21 with no env overrides: DRAM 97,639 / HBM 40,675 / SSD 3-drive 21,461 MiB/s, zero misses, byte-exact correctness on all three media, umbp ctest 25/25. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TianDi101
added a commit
that referenced
this pull request
Sep 3, 2026
- Port tree connector changes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Technical Details
Test Plan
Test Result
Submission Checklist