Repository navigation
Conversation
Supplemental v0.2.5 rebase measurements — partial configuration comparisonI reviewed the per-request records from a stock v0.2.5 run and a separately built v0.2.5 candidate carrying this patch. The cached-input improvement is reproducible in these records, but this is not an isolated OFF/ON experiment for #4002, and it does not establish that long-context admission/queueing is solved. Build and scope
All three recorded rounds (seconds unless indicated; stock → candidate)
*The runner's p95 over 16 requests uses index †The candidate's third “256K first” request was not cold: 223,232 cached + 38,517 freshly processed tokens, versus stock 0 cached + 261,749 fresh tokens. Its following repeat is also marked as a sequence caveat. Both rows are retained above rather than silently discarded; no clean three-round 256K-cold median claim is made. The first two cold pairs were approximately unchanged. The 128K cold requests each processed 130,675 fresh tokens with zero cache. Evidence limits and outstanding work
Conclusion: this v0.2.5 candidate retains the observed cache-repeat TTFT benefit, with little change in cold-input latency and no demonstrated native decode gain. Attribution to only #4002, and resolution of the main long-context scheduling bottleneck, still require controlled follow-up evidence. |
Merge main at 4b9e05a into the existing cache-copy PR without rewriting the historical tested commit. Keep the upstream memory-guard rewrite and relocate the opt-in stage-copy description beside the current cache docs. The six other patch files retain exactly the original added/removed lines. Constraint: No production configuration changes, restart, or load testing Confidence: high Scope-risk: narrow Not-tested: GPU backend builds and runtime validation on this integration
Add CPU regressions for zero stages, empty stage bodies, idle teardown, recovery after a worker exception, and two callers sharing the pool. No engine runtime code or defaults change. Constraint: Local CPU verification only; no service or GPU operations Confidence: high Scope-risk: narrow Not-tested: GPU failures, partial thread-construction failure, backend execution
…wait forever stop_and_join set stop_ and notified without holding the slot mutexes, so a worker that had checked its predicate and was about to sleep could miss the wakeup and sleep forever, hanging the join. A worker exiting on stop_ also left slot.done unset, so a run() in its completion wait never returned. stop_and_join now takes run_mu_ (destruction waits out an in-flight run), sets stop_ under the slot mutexes, and a worker exiting on stop_ marks its slot done with an error; run() also wakes on stop_ and reports it. Regression: 300 hammer rounds and 100 destructions with a run in flight.
…er dereference UB TSan flagged the test itself: the caller thread dereferences the pool unique_ptr while the main thread resets it. No class can make that safe; the stop/wakeup races the fix targets stay covered by the hammer block.
Preserve both the opt-in parallel-copy boundary and upstream arena admission guidance. Constraint: Keep all copy switches opt-in; no production activation. Confidence: high Scope-risk: moderate
Add a standalone CPU ownership probe and sanitized passive eviction samples. Preserve allocator attribution and current-head runtime validation limits. Constraint: No production restart, workload or probe attachment. Not-tested: Current-head GPU/model runtime, HIP/SYCL builds and Metal runtime.
Rerun the probe against LF-normalized Git blobs and record their hashes instead of Windows working-tree CRLF hashes. Observed results are unchanged.
|
@Niko1221 This PR is now ready for review at Allocator retention remains an inference, not a confirmed RSS root cause. The committed CPU ownership probe and passive eviction/refill data do not exclude other live allocations; natural-traffic measurements came from an earlier deployed revision. The latest head has not completed GPU/layer-split model parity/performance or sustained runtime stability; HIP/SYCL builds and Metal runtime validation also remain missing. These gaps and all incomplete/failed attempts are retained in the PR body and committed validation record. Ready for review does not mean merge approval or production acceptance. No merge or production restart is requested. |
…read-local park trace, stage-parallel snapshot test The parts of upstream PR Niko1221#4002 (Niko1221/Strata, perf/cache-parallel-v024-20261024, head 29a84e8) that live in its merge commits and so cannot be cherry-picked on their own: - 71ec493 ("Merge v0.2.7 and guard parallel captures against shared async staging"): STRATA_STAGE_COPY_PARALLEL stays off, with one line, when STRATA_PARK_ASYNC=1 or STRATA_CKPT_ASYNC=1 (both share one staging state across stages); conversation_park_async_begin returns before touching the process-wide ParkAsync when STRATA_PARK_ASYNC is off (concurrent synchronous stage captures no longer write it); kv_save_trace is thread_local (each stage's sums belong to its worker); conversation_snapshot_test's STRATA_TEST_STAGE_PARALLEL regression (concurrent independent captures and restores against serial byte-exact references, GPU) and its two CTest entries. - cfff8ec (merge of main 870aedd): the BATCHING.md row's final wording. The evidence-only files of 3704e9c / 29a84e8 (docs/validation/pr4002, the RSS release probe) are not taken. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…opy streams made at start, checkpoint mounts Port follow-up to upstream PR Niko1221#4002 (cherry-picked above), for this fork's engine and our 4-stage layer split: - The stage copy workers go where StageThreads' (STRATA_CKPT_THREADS) go: wide, then the aux CPUs. The host thread pins itself to one CPU once the CPU pool starts and every thread it creates inherits that CPU; the copies of a streamed state (pinned host K/V <-> pageable images, pinned <-> pinned for a slot) are host memory traffic, which four workers on one CPU would only take in turns. StagePool takes an optional start hook for it (tested in stage_pool_test: once per worker, on the worker's thread, before any body; a throwing hook leaves the worker running). - Niko1221#2409 kept: a restore must not create a copy stream on a card the expert cache has filled (an out-of-memory there ends the process), and the check asked for the CALLING thread's stream - with the pool the restore runs on worker k. Worker k always copies on the same card (bodies are in stage order), so each makes its stream right after the pool starts (before the expert cache takes the cards' memory; conversation_copy_stream_prepare), and the restore check asks for the worker's stream when the pool is on. - A checkpoint mount (a request resuming from a checkpoint of the main session's chain or of a slot) restores every stage's running state at once through the pool too (the same checkpoint_restore calls; a mismatched stage_parts count still ends the request as before). - The borrowed-prefix restore (ConversationCache::borrows) runs only without a layer split (generate.cpp: `borrow = stages.empty() && ...`), so it has no stages to run at once; unchanged. Off by default: without STRATA_STAGE_COPY_PARALLEL=1 no pool exists and every path is the serial one. The copies are the same calls on the same bytes (kv-lean's cut at the lazy host K/V's registration pieces is inside each copy, and the piece table is written only at start-up); only the stages run at once, each on its own card and sessions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem and result
On multi-GPU layer splits, conversation capture, parked restore and batch-slot copies serialize across stages. This PR adds opt-in persistent stage workers and optional reusable host K/V shuttles. All copy switches remain off by default; cold prefill and prompt admission are unchanged.
The engine-code validation anchor is
cfff8ec3ec1db4f1226d451cd2f8e73b47aa317f. The evidence-only follow-up adds the standalone ownership probe and sanitized records linked below; engine implementation and defaults are unchanged. It preserves the original PR history, incorporates the StagePool shutdown fix, and merges upstream main870aedd9e1ebe1c91f94da93b59af17729f6c5c0. The only manual conflict was documentation; both the upstream arena-memory guidance and the copy-switch limits were retained.Related discussion: #3321, including the maintainer's request for a PR: #3321 (comment) . Earlier approaches #3742 and #3711 were considered.
Approach and boundaries
STRATA_STAGE_COPY_PARALLEL=1: persistent stage workers for capture, parked restore, direct and host slot copies. Publish a snapshot only after all stages succeed; propagate worker errors and exceptions.STRATA_SLOT_SHUTTLE_REUSE=1: reuse storage but copy the full image. Equal token prefixes do not establish equal K/V bytes. This switch remains off by default; the measured reuse arm below regressed short concurrent throughput.STRATA_STAGE_COPY_TRACE=1: optional per-stage durations; off by default.STRATA_PARK_ASYNC=1orSTRATA_CKPT_ASYNC=1takes precedence and disables the pool with a visible conflict message. Their shared staging state keeps serial stage copies. Preserve upstream serial checkpoint pool/move/top-up behavior.Engine-code validation anchor cfff8ec — 2026-10-30
47e5ebc9cab42cc2253994bd9fdb6220cc746bae098602ad9522b7ca9cb9e848.STRATA_ENABLE_CUDA=OFF, HIP/SYCL off): configure, stage_pool_test build and CTest passed.personality(ADDR_NO_RANDOMIZE). This was not counted as a passing round or silently discarded; host execution of that binary produced the 10/10 result above.Reproduction selections (from the recorded CUDA configuration):
cmake --build /work/cmake-build --parallel 2 --target strata stage_pool_test conversation_cache_test conversation_snapshot_test conversation_split_failure_test conversation_memory_test session_save_reclaim_test conv_cache_test ctest --test-dir /work/cmake-build --output-on-failure --timeout 120 -R '^(stage_pool_test|conversation_cache_test|conversation_split_failure_test|conversation_memory_test|session_save_reclaim_test|conv_cache_test)$'No model requests, GPU kernels, production restarts or performance workload were run for the cfff8ec update or the evidence-only follow-up. GPU/model tests below belong to earlier revisions; they are not current-head runtime results.
Earlier controlled integration evidence — 2026-10-29/30
These results belong to local validation merge
30d397635a934b9f38a1b0cda34e2489190f413e(PR71ec493plus main46c152a1), engine SHA-256a6f1ea0a5354da2d7001e598b129726e59279c0125f2f341eca546eb4894b3fc. They precede the shutdown fix and the latest main merge.Memory investigation — unresolved stability limit
The parallel 16-client soak stopped at the 32 GiB available-memory guard after 494 iterations / 7904 requests. A separate OFF control completed 533 iterations / 8528 requests over one hour. A second candidate soak also hit the guard. The planned two-hour workload and final-reference parity were not completed; do not describe this as a passed long-duration soak.
Follow-up 400-request sequential ramps used the same engine with parallel ON/OFF. Both grew: ON RSS 130.809→194.838GiB, OFF 130.808→200.400GiB. Their first 300-request slopes were 227.849 / 227.846 MiB per request. This contradicts the earlier claim that only parallel copies grow. It does not establish that allocator retention is the root cause, that all memory was freed, or that this PR has no effect under sustained concurrency. The earlier 'not by this PR / identically' attribution is superseded by this bounded result.
Archived engine logs now identify the initial growth: each parked SavedConversation snapshot accounts for approximately 227.8 MiB. Before eviction, cache-accounting increments averaged 227.810 MiB per park in both arms, matching the RSS slopes above. The runs used a 96 GiB cache budget and a 32 GiB free-memory floor. This initial growth was real cached running/checkpoint/K/V state, not evidence by itself of a leak.
After eviction, the cache accounting fell from 69.411 to 19.134 GiB (parallel) and 70.078 to 15.352 GiB (OFF), while RSS ended at the values above. The cache no longer accounts for that historical peak, but these logs lack live allocation/free counters or allocator statistics. Source inspection shows value-owned vectors destroyed on eviction; worker callables are destroyed before run returns. That does not prove where all remaining resident pages belong. Allocator retention and other unaccounted live allocations still cannot be distinguished conclusively from the archived RSS/cache counters. No malloc_trim, debugger attachment or new workload was used to force a diagnosis. The residual post-eviction gap and the incomplete concurrent soak remain open stability limits.
Cache release and passive RSS evidence — 2026-10-30
Evidence follow-up head:
29a84e8108fd3b50627ddef485aec13544326427. The probe was rerun successfully against exact Git source blobs; recorded fingerprints match those blobs.The committed validation record includes the revision-bound test inventory, successful and failed/blocked attempts, incomplete soak, reproduction commands, CPU probe source, observed output, header fingerprints and sanitized numeric samples.
take()instead transfers ownership: cache accounting reaches zero while the external owner still holds 5 MiB, which is freed when that owner is destroyed. Three cases passed. This is a bounded, single-threaded ownership check, not a GPU, concurrency or allocator-to-OS reclamation test.71ec4933, not the latest PR engine build. UTC 03:14:02→03:15:02: eviction count 0→41, cache 58.615→2.534 GiB, anonymous RSS 108.518→107.566 GiB; in-flight requests 0→1. At 03:23:02 cache had refilled to 46.470 GiB while anonymous RSS was 107.669 GiB and the service was idle.Remaining limits
This PR is now submitted for maintainer review, with the following validation gaps explicitly open. Current-head GPU/layer-split execution, model parity/performance and sustained stability are not rerun under the no-production-restart/no-load constraint. HIP/SYCL builds and Metal runtime execution remain untested here. CI checks must be tied to the displayed revision: the checks linked above belong to cfff8ec; follow-up check results are recorded separately. CONTRIBUTING.md backend build requirements remain incomplete; targeted maintainer help with HIP/SYCL and device runtime validation is requested before promotion. Preserve all failed and environment-blocked attempts; no performance or memory-root-cause claim is inferred from passing CPU tests.
Maintainer review request — 2026-10-30
Please review the ownership, synchronization and failure paths and identify the minimum backend/runtime checks needed before merging. Ready for review is not a claim of merge approval or production acceptance. Allocator retention remains an inference: passive eviction/refill observations and the CPU ownership probe do not uniquely attribute residual RSS or exclude all live allocations. The latest head has not been exercised with GPU/layer-split model parity, performance or sustained stability; HIP/SYCL builds and Metal runtime remain missing. The committed validation record reflects Draft status when recorded; this review request supersedes that status recommendation, not its evidence limits. No merge, production restart or workload is requested.