Skip to content

Feat/dstorage transport - #825

Open
khalilswdp wants to merge 5 commits into
JustVugg:devfrom
khalilswdp:feat/dstorage-transport
Open

Feat/dstorage transport#825
khalilswdp wants to merge 5 commits into
JustVugg:devfrom
khalilswdp:feat/dstorage-transport

Conversation

@khalilswdp

Copy link
Copy Markdown
Contributor

Summary

On Windows, big expert-miss reads go through pread and are CPU/copy-bound
well below what the SSD can deliver. This PR adds a DirectStorage transport:
COLI_DSTORAGE=1 serves the coalesced expert-slab disk reads via DirectStorage
instead of pread (primary replica only; the dual-SSD mirror keeps its stock
path), and the VRAM-side arena API ships alongside for the VRAM-depot PR's
startup fill.

Opt-in twice over, so the smallest change that engages it is zero: the DLL only
gains the code when built with DSTORAGE_HOME (the entry points are optional
exports; dstorage.dll is loaded dynamically at runtime), and the engine only
uses it when COLI_DSTORAGE=1. Any failure falls back cleanly to pread.

Hard-won notes live in the backend_dstorage.cpp header: never take the
default D3D12 adapter (it's the display adapter — on hybrid boxes an iGPU
whose resources CUDA cannot import; enumerate DXGI for the NVIDIA one); the
arena splits into ≤2 GiB chunks (destination offsets past 4 GiB wrap on this
stack); every chunk gets D3D12_RESIDENCY_PRIORITY_MAXIMUM (a demoted page
means a torn DMA write); reads chunk at 16 MB against a 128 MB staging buffer.

Stacked on the Windows-suite PR (its commits ride along until it merges;
feat(win): DirectStorage expert-load transport is the only new content).

Possible next step on Linux: the engine already batches expert reads via
io_uring (uring.h); routing this same coalesced slab read through io_uring
(O_DIRECT + registered buffers) would be the natural counterpart there.

Validation

  • make -C c check — full pass at this tip (Windows, mingw-w64 gcc 16.1);
    warning set identical to dev's own baseline on the same toolchain.
    Token-exact oracle: 32/32 TF, 20/20 greedy with the flag unset, and a
    pread-vs-DS A/B on a tiny int4 container produced byte-identical output
    (the transport delivers the same raw file bytes into the same slab).
  • make -C c cuda-test — n/a for the transport (backend_dstorage.cpp is
    _WIN32-only and compiles only with DSTORAGE_HOME); backend_cuda.cu
    kernel code is untouched, and the loader-side symbols are optional.
  • Performance (i5-13400 · 32 GB DDR4 · RTX 5080 16 GB · NVMe; GLM-5.2
    358 GB int4):
    - Transport microbench (dsbench harness, 64 GB cold file, random
    4K-aligned offsets, interleaved arms): 19 MB slabs 2.25 GB/s pread →
    3.7–4.2 GB/s DS (+65–85%); 38 MB slabs 2.24 → 5.05 GB/s (+125%). On an
    HDD both arms are seek-bound at parity (~220 MB/s) — the win is
    NVMe-class only.
    - Engine A/B in a deliberately miss-heavy config (RAM_GB=13 CTX=2048,
    ~2% hit rate, 3 interleaved rounds, frozen .coli_usage, 48-token
    generations): decode 0.13/0.13/0.13 → 0.18/0.18/0.18 tok/s (+38%),
    prefill −23%, ~630 GB/run served via DS at ~1.9 GB/s sustained. In
    tuned configs (20%+ hit rate) the gap compresses — this is a
    disk-bound-regime win, which is exactly when the flag matters.

Compatibility

  • The default CPU build remains dependency-free — nothing new compiles
    without DSTORAGE_HOME, and no default dependency is added anywhere.
  • No model files, generated binaries, or benchmark artifacts are included
    (the DirectStorage redistributable DLLs are gitignored, not vendored).

Engine defaults changed: none (COLI_DSTORAGE unset = stock pread path).

khalilswdp and others added 5 commits August 4, 2026 17:04
COLI_DSTORAGE=1 serves the coalesced expert-slab disk read via
DirectStorage instead of pread (primary replica only; mirror keeps its
stock path); unavailable/failed DS falls back to pread. The VRAM arena
API ships alongside for the depot's startup fill. Windows-only, doubly
opt-in: compiled only with DSTORAGE_HOME, engaged only by the env var.

Possible next step on Linux: the engine already batches expert reads
via io_uring (uring.h); routing this same coalesced slab read through
io_uring (O_DIRECT + registered buffers) would be the counterpart.
One conflict, in c/coli's cmd_run. dev added the DeepSeek V4 branch (JustVugg#165)
directly after a plain banner("run"); this branch moved that banner below
env_for so it can report the CUDA backend -- banner("run", COLI_CUDA=="1").

Taking dev's hunk verbatim would have reintroduced the plain banner and
printed two. Taking this branch's side would have dropped the V4 path.
Resolved by keeping the V4 branch and giving it its own banner("run"),
since that path sys.exit()s before reaching the CUDA-aware call. Both
intents preserved; neither side loses a line.

`python -m ast` parses the result.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
JustVugg added a commit to khalilswdp/colibri that referenced this pull request Aug 4, 2026
Same conflict as JustVugg#825 and the same resolution, in c/coli's cmd_run.
dev added the DeepSeek V4 branch (JustVugg#165) right after a plain banner("run");
this branch moved that banner below env_for so it can report the backend --
banner("run", COLI_CUDA=="1").

Kept the V4 branch and gave it its own banner("run"), since that path
sys.exit()s before reaching the CUDA-aware call. Taking either side whole
would have printed two banners or dropped the V4 path.

`python -m ast` parses the result.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ZacharyZcR pushed a commit to ZacharyZcR/colibri that referenced this pull request Aug 5, 2026
`coli chat`, `run`, `info`, `plan` and `tune` all printed

    GLM-5.2 · 744B MoE · int4 · streaming CPU

regardless of what was loaded. Load Inkling, Kimi K3, OLMoE or the new
DeepSeek V4 engine and the third banner line still named GLM.

## Why it was wrong, and where the wrongness lives

Not in the banner. In what the banner would have had to ask:

    def model_arch(model):
        ...
        return "glm"          # <- every unrecognised model_type

That default is correct for its job -- colibri.c is the general engine, so
an unknown checkpoint should be offered to it. It is the wrong answer to a
different question, "what did the user load", and the banner needs that
one. So `model_banner_line()` reads the raw `model_type` instead, and the
dispatch in `model_arch()` is untouched.

## What it prints

    GLM-5.2 · 744B MoE · 372 GB on disk
    DeepSeek V4 Flash · 284B MoE · 167 GB on disk
    OLMoE · 7B MoE · 4.2 GB on disk
    qwen3_moe · 48L x 128E MoE · 61 GB on disk      <- not in the roster

Parameter counts come from the README roster, so the two cannot drift apart
without someone noticing. A model_type that is not in the table is NOT
forced into a name: it prints its own type and the geometry measured from
its config, which is honest and still useful. No config.json at all, or an
unreadable path, keeps the original tagline -- `coli info` banners before it
validates the model directory, so this has to be safe on a bad path.

Size is measured by stat-ing the shards; no safetensors headers are parsed.
A banner runs before every command and may not cost a scan of a 400 GB
checkpoint.

## Note for the open PRs that also touch banner()

JustVugg#825 and JustVugg#670 both change `banner("run")` to pass a CUDA flag positionally.
`model=` is therefore **keyword-only**, so a second positional argument
raises TypeError instead of being silently read as a path. There is a test
for exactly that.

Eight tests in tests/test_cli_output.py, including the regression itself: a
`deepseek_v4` config must not produce a line containing "GLM" or "744B".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JustVugg JustVugg added enhancement New feature or request performance Velocità / tok-s / ottimizzazioni labels Aug 7, 2026
ErikTromp pushed a commit to SensAI-PT/aviary-hy3 that referenced this pull request Aug 9, 2026
`coli chat`, `run`, `info`, `plan` and `tune` all printed

    GLM-5.2 · 744B MoE · int4 · streaming CPU

regardless of what was loaded. Load Inkling, Kimi K3, OLMoE or the new
DeepSeek V4 engine and the third banner line still named GLM.

## Why it was wrong, and where the wrongness lives

Not in the banner. In what the banner would have had to ask:

    def model_arch(model):
        ...
        return "glm"          # <- every unrecognised model_type

That default is correct for its job -- colibri.c is the general engine, so
an unknown checkpoint should be offered to it. It is the wrong answer to a
different question, "what did the user load", and the banner needs that
one. So `model_banner_line()` reads the raw `model_type` instead, and the
dispatch in `model_arch()` is untouched.

## What it prints

    GLM-5.2 · 744B MoE · 372 GB on disk
    DeepSeek V4 Flash · 284B MoE · 167 GB on disk
    OLMoE · 7B MoE · 4.2 GB on disk
    qwen3_moe · 48L x 128E MoE · 61 GB on disk      <- not in the roster

Parameter counts come from the README roster, so the two cannot drift apart
without someone noticing. A model_type that is not in the table is NOT
forced into a name: it prints its own type and the geometry measured from
its config, which is honest and still useful. No config.json at all, or an
unreadable path, keeps the original tagline -- `coli info` banners before it
validates the model directory, so this has to be safe on a bad path.

Size is measured by stat-ing the shards; no safetensors headers are parsed.
A banner runs before every command and may not cost a scan of a 400 GB
checkpoint.

## Note for the open PRs that also touch banner()

JustVugg#825 and JustVugg#670 both change `banner("run")` to pass a CUDA flag positionally.
`model=` is therefore **keyword-only**, so a second positional argument
raises TypeError instead of being silently read as a path. There is a test
for exactly that.

Eight tests in tests/test_cli_output.py, including the regression itself: a
`deepseek_v4` config must not produce a line containing "GLM" or "744B".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request performance Velocità / tok-s / ottimizzazioni

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants