Skip to content

fix(core): preserve workspace layout state after allocation overflow - #297

Open
DuncanBetts wants to merge 1 commit into
Neroued:masterfrom
DuncanBetts:refactor/workspace-layout-builder
Open

DuncanBetts wants to merge 1 commit into
Neroued:masterfrom
DuncanBetts:refactor/workspace-layout-builder

Conversation

@DuncanBetts

@DuncanBetts DuncanBetts commented Sep 20, 2026 •

Copy link
Copy Markdown

fix(core): preserve workspace layout state after allocation overflow

Problem and scope

Related Issue: none; this draft is submitted for scope review before runtime verification.

WorkspaceLayoutBuilder::alloc and alloc_bytes currently commit the aligned cursor before
checking whether the requested allocation end is representable:

cursor_ = align_up(cursor_, alignment, "workspace layout");
cursor_ = checked_add(cursor_, bytes, "workspace layout");

If checked_add throws, the builder is left partially modified. Starting from cursor 3, a failed
SIZE_MAX-byte allocation aligned to 8 leaves the cursor at 8. If the exception is caught, the
next one-byte allocation ends at 9 rather than 4.

This change gives workspace allocation the same commit-after-validation behavior already used by
LayoutBuilder::add. Its scope is Core's dry-run workspace bookkeeping. Successful workspace
estimates, real arena allocation, CUDA work, model behavior, and the public Engine API are
unchanged.

Implementation

WorkspaceLayoutBuilder now contains a LayoutBuilder and delegates nonempty allocation
accounting, temporary scopes, and final peak alignment to it. LayoutBuilder::add computes the
aligned offset locally and updates its cursor only after the checked end calculation succeeds.
A failed allocation therefore leaves both cursor and peak unchanged.

The workspace-specific interface remains intact:

  • alloc returns a null-backed Tensor with the requested dtype and shape;
  • alloc_bytes returns a null-backed span;
  • zero-byte scratch remains a no-op, including when its unused alignment is invalid;
  • successful allocation offsets, peaks, alignment, and nested-scope lifetimes are unchanged.

WorkspaceLayoutBuilder::Scope becomes an alias for LayoutBuilder::Scope, removing the second
movable scope-guard implementation. This is composition inside Core; it adds no inheritance,
virtual dispatch, callback, heap allocation, or new allocator abstraction.

The production diff removes 30 net lines (37 deletions, 7 additions). The focused test adds
96 lines and its registration adds three. Total repository LOC grows because the failure and
scope contracts receive direct behavioral coverage; the production rule itself has one owner.

For valid allocation sequences, the computed peak is intended to remain byte-for-byte identical.
Expected VRAM saving is 0 bytes, expected RAM saving is 0 bytes, and no GPU work changes. Host-side
delegation may compile away or may have a very small cost; it has not been benchmarked, so this PR
makes no CPU, binary-size, memory, or latency claim.

Verification

Performed locally with GCC 15.2.0:

c++ -std=c++20 -Wall -Wextra -Werror -Wno-missing-field-initializers -Isrc \
  -fsyntax-only src/core/layout.cpp tests/test_layout.cpp
clang-format --dry-run --Werror \
  src/core/layout.cpp src/core/layout.h tests/test_layout.cpp
git diff --check

All three checks passed.

The new ninfer_layout_test covers the overflow reproduction and verifies that the next
allocation ends at byte 4. It also covers empty and aligned peaks, Tensor shape and null backing,
nested and moved scope restoration, exception unwinding, zero-byte scratch, invalid alignment,
maximum representable size, and allocation-end/cursor/final-alignment overflow.

I also verified the change in a container based on
nvidia/cuda:13.1.2-devel-ubuntu24.04. The container configured a Release sm_120a build with
tests enabled, applications and benchmarks disabled, and then built:

cmake --build /build -j --target \
  ninfer_layout_test \
  ninfer_qwen3_5_runtime_mechanisms_test

It ran the focused tests with:

ctest --test-dir /build --output-on-failure \
  -R '^(ninfer_layout_test|ninfer_qwen3_5_runtime_mechanisms_test)$'

Both selected tests passed (2/2). They are host-side tests, and the container ran without GPU
passthrough. ninfer_layout_test covers the overflow recovery and layout/scope boundaries;
ninfer_qwen3_5_runtime_mechanisms_test checks the existing model-side workspace-planning behavior.

Notes for reviewers

  • The behavior change is only after a caught failure. For valid nonempty allocations, both
    implementations perform the same align_up(cursor, alignment), checked addition, peak update,
    and final peak alignment.
  • No current production caller was found recovering this builder. Repository search found no
    path that catches this workspace overflow and continues with the same estimator. In ordinary
    execution the exception therefore aborts planning and the partially advanced cursor dies with
    the builder. The fix establishes a strong failure-state guarantee for future recovery/reuse; it
    is not presented as a currently observed inference failure.
  • The overflow does not itself allocate or touch GPU memory. It throws during the dry-run size
    calculation, before a workspace allocation or device access, so it does not directly corrupt
    memory.
  • Caught reuse can only inflate the estimate. The failed request leaves the cursor at its
    aligned offset while the peak remains unchanged. A later successful allocation can incorporate
    that phantom padding into the peak. It cannot move the cursor backward or produce an undersized
    workspace estimate through this failure mode.
  • The retained amount is bounded by alignment padding. It is at most alignment - 1 bytes for
    the failed request when alignment itself succeeds: normally at most 255 bytes for the default
    256-byte workspace alignment, though callers can request another power-of-two alignment.
  • A polluted estimate can waste capacity or fail conservatively. If a recovery path reused the
    builder, the inflated peak could allocate a little more VRAM than necessary, trigger a false
    capacity/OOM decision at a tight boundary, or contribute to a later arithmetic overflow. These
    are plausible consequences of future caught reuse, not failures observed in current inference.
  • The smallest alternative is local commit-after-validation. Keeping local temporaries in both
    workspace allocation methods would fix this reproduction, but it would retain the duplicate
    scope, cursor, peak, alignment, and overflow implementation that allowed the rules to diverge.
    Composition reuses the already-correct Core implementation without broadening ownership.
  • Zero-byte behavior is intentionally outside LayoutBuilder::add. That method rejects empty
    regions; workspace scratch treats zero bytes as a no-op and must continue ignoring its otherwise
    unused alignment.
  • One error-precedence detail can change. alloc constructs its dry-run Tensor and computes
    its byte size before calling LayoutBuilder::add. If both Tensor sizing and alignment are
    invalid, the Tensor-size error may be observed first. This does not affect valid planning.
  • The Scope name changes type identity internally. It is now an alias to
    LayoutBuilder::Scope. Repository search found no consumer spelling
    WorkspaceLayoutBuilder::Scope outside its own method definition; callers use auto.
    NInfer has no installed/exported C++ SDK.
  • Object storage remains equivalent. The old workspace builder stored two size_t counters;
    its LayoutBuilder member stores the same two counters. No heap or device allocation is added.
  • The returned region record is discarded. LayoutBuilder::add returns offset/size/alignment
    metadata that this adapter does not need. Optimized-code effects were not inspected or measured.
  • Real allocation is untouched. WorkspaceArena and DeviceArena retain their current
    capacity, pointer, and CUDA lifetime behavior.
  • Review order: compare the old two-step mutation with LayoutBuilder::add; check the
    zero-byte early return; then read test_workspace_boundaries before the broader scope tests.

Not verified / limitations

  • Optimized assembly and host planning latency were not compared.
  • origin/master and origin/dev were fetched immediately before submission. master matched the
    candidate baseline, and commits ahead on dev did not touch the affected files.

@DuncanBetts
DuncanBetts marked this pull request as ready for review September 20, 2026 13:07
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