Skip to content

engine: RAMMAP, NUMA, and telemetry (PR1 of #377 split) - #820

Open
BColsey wants to merge 14 commits into
JustVugg:devfrom
BColsey:successor/pr1-engine
Open

engine: RAMMAP, NUMA, and telemetry (PR1 of #377 split)#820
BColsey wants to merge 14 commits into
JustVugg:devfrom
BColsey:successor/pr1-engine

Conversation

@BColsey

@BColsey BColsey commented Aug 4, 2026

Copy link
Copy Markdown

Draft successor — supersedes #377. One of three dependent PRs reconstructed file-by-file from the frozen green donor 5f6f31a (onto base 72ddb673). Do not merge #377; it is closed as superseded once all three are open + linked.

Scope

Engine C RAMMAP/NUMA + CUDA accounting, strict telemetry parsing, and their tests/docs. Excludes planning, mounts, lifecycle, benchmark, UI.

  • Engine: c/{colibri.c, backend_cuda.cu, compat.h, st.h, telemetry.h}, c/Makefile, c/resource_plan.py
  • Serve/telemetry: c/openai_server.py (PROF parsing)
  • Tests: test_rammap.c, test_resource_masks.c, test_uring.c, test_serve_sentinel.c, test_backend_cuda.cu, test_rammap_e2e.py (parse logic), test_openai_server.py, test_openai_tools_e2e.py, test_fp8_e2e_repack_load.py, test_resource_plan.py
  • Docs: docs/serve_protocol.md, docs/cuda.md

Reconstruction work

  • STRICTEN PROF telemetry parsing (design L78-80): reject field counts other than 10/17/18; read physical_ssd_valid as == "1" (donor uses bool(int(...))); reject non-finite numerics; a malformed known-kind record is a protocol error.
  • Make docs/serve_protocol.md normative at the PROF contract.
  • Hunk-split c/Makefile (test targets here; install rules → PR2) and test_rammap_e2e.py (parse logic here; live tmpfs cells → PR3).

Dependency

None — PR1 is the root. openai_server.py is stdlib-only; C tests compile against PR1 headers only.

Build verification (subagent worktree)

  • make -C c check GREEN end-to-end: C build + 41 test binaries, C suite ALL PASS (0 failures), Python discovery 302 passed / 0 failed / 18 expected skips. Standalone — no PR2/PR3 dependency.

Reconstruction update

PR1 reconstruction landed (commit f56604f): PROF telemetry strictened to exactly 10/17/18 fields + physical_ssd_valid == "1" + finiteness checks; docs/serve_protocol.md made normative; +4 tests. Green: 282 passed / 0 failed. Still TODO: hunk-split Makefile/test_rammap_e2e (organizational).


Update — 2026-08-07: forward-merged upstream/dev (forward-only merge commit; reconstruct history preserved). Conflicts resolved against the green donor 3658a5d; branch rebuilt and tested linux-green (make -C c check exit 0, 403 tests OK; conflicts in c/Makefile, c/openai_server.py, c/st.h, c/telemetry.h). Merged dev @ aa46e7b; GitHub reports this branch MERGEABLE into dev (dev has since advanced to 8d06bd1, but this branch's engine-only changes don't overlap the newer commits, so it stays conflict-free).


Update — 2026-08-17: forward-merged upstream/dev @ 1cc0be2 (forward-only merge commit c9767cc; reconstruct history preserved). 123 dev commits integrated. One textual conflict (c/openai_server.py Engine.__init__) resolved as a union: dev's family keeps the 7th positional slot (dev's serve() calls it positionally), the branch's command_prefix/stderr trail as keyword-only additions. Two semantic conflicts fixed past the textual merge: the branch's spawn-hook test expectation now includes dev's new V4_MTP_GPU child-env default (b7117bf), and test_cpu_vs_cpu_determinism's tok/s tolerance is a known local-load flake (identical hit rates, green 3x isolated, file untouched since the merge base) — CI is the gate there. make -C c check: C suite green; 568 python tests, 1 local timing flake, 29 expected skips. GitHub reports this branch MERGEABLE (CLEAN) into dev.

Update — 2026-08-17 (later): dev advanced again (1cc0be2c945cd5, cluster expert workers + expert-store registry + qrow hoist); forward-merged as 3ca0be8 (forward-only). One textual conflict (c/colibri.c main): union of dev's CLUSTER_WORKERS init block with the branch's weights_dir model_init signature + rammap_build() call. Validation: make -C c check fully green (574 tests, 0 failures — including the earlier local-only tok/s flake), and the greedy token-exactness A/B re-run on this merge (merged binary vs dev c945cd5, shared seed-1234 fixture, gates unset): byte-identical token streams, 20/20 both sides. MERGEABLE (CI pending approval as usual for fork pushes).

@JustVugg JustVugg added discussion Proposta / discussione aperta, non un task enhancement New feature or request performance Velocità / tok-s / ottimizzazioni labels Aug 7, 2026
@BColsey
BColsey force-pushed the successor/pr1-engine branch 2 times, most recently from 4dbacf5 to ea46a57 Compare August 9, 2026 12:49
@BColsey

BColsey commented Aug 9, 2026

Copy link
Copy Markdown
Author

Merge order: 1 of 3

Merge this PR first, then merge #821, followed by #822.

Dependency chain: #820#821#822

@BColsey

BColsey commented Aug 9, 2026

Copy link
Copy Markdown
Author

@JustVugg, this PR is ready for review.

The current CI workflow and check workflow are both action_required with no jobs started. Because this is a fork PR, they require approval from a base-repository maintainer before CI can run.

Merge position: 1 of 3#820#821#822.

@BColsey
BColsey marked this pull request as ready for review August 9, 2026 13:10
@BColsey
BColsey force-pushed the successor/pr1-engine branch from ea46a57 to 6dde621 Compare August 14, 2026 01:14
@BColsey

BColsey commented Aug 15, 2026

Copy link
Copy Markdown
Author

@JustVugg can you approve the workflow?

@JustVugg

Copy link
Copy Markdown
Owner

Done 👍🏻👍🏻

@JustVugg

Copy link
Copy Markdown
Owner

@BColsey — this has been open a while and you've done a lot of careful work on it, so rather than leaving it in the queue I went and did the verification myself. Two of the things that were making me cautious turned out to be non-issues, and I'd rather tell you that than have you guess at what was blocking it.

What I checked

1. The strict PROF parsing — safe. My concern was that rejecting anything other than 10/17/18 fields would break engines, because we have direct evidence this week that the five engines implement the shared telemetry surface inconsistently (#890 was V4 emitting incomplete phase telemetry; a review of MiniMax found it emitting a 5-field STAT where its siblings emit 7 — passing the >= 5 check with zero margin).

So I counted what each engine actually emits on dev:

colibri.c      PROF %.3f %d %d %.3f %.3f %.3f %.3f %.3f %llu   -> 10 fields
inkling.c      PROF %.3f %d %d %.3f %.3f %.3f %.3f %.3f %d     -> 10 fields
kimi_k3.c      PROF %.3f %d %d %.3f %.3f %.3f %.3f %.3f %d     -> 10 fields
olmoe.c        PROF %.3f %d %d 0.0 0.0 0.0 0.0 0.0 %d          -> 10 fields
deepseek_v4.c  PROF %.3f %d %d %.3f 0.000 %.3f 0.000 0.000 0   -> 10 fields

All five are exactly 10, and the current parser accepts len(fields) >= 10 (openai_server.py:1804). Your strict parser rejects none of our engines. Concern withdrawn — and making docs/serve_protocol.md normative at that contract is the right move, especially given the drift we keep finding.

2. It's opt-in — confirmed. g_rammap = 0, g_ram_prefault = 0, gated on COLI_RAMMAP / COLI_RAM_PREFAULT, with NUMA behind COLI_NUMA / COLI_NUMA_NODES. That matters a great deal here: +917/-156 in colibri.c reads alarming in the diffstat, but with the variables unset the new paths are inert. The blast radius is far smaller than the line count suggests.

Why it sat

Honestly: not because anything was wrong with it. c/colibri.c is the flagship engine and the single most shared file in the tree, and this is the largest change to it in the open queue. Everything I merge there gets weighed against the fact that a regression lands on every GLM user immediately. Combined with it being the root of a three-PR chain whose later members are much more invasive, it needed a maintainer to actually sit with it rather than glance at a green tick. That's on me, not on you.

The work itself is visibly disciplined: reconstructed file-by-file from a frozen green donor, make -C c check green end-to-end at 403 tests, four new test files (test_rammap.c, test_resource_masks.c, test_rammap_e2e.py, plus test_openai_server.py grown by 500 lines). The sched_setaffinity readback — applying a mask and then verifying it rather than trusting the syscall — is the kind of detail that tells me you've been bitten by this before.

The one thing I'd like before merging

A token-exactness check with the feature off, on a tiny model: same prompt, greedy, dev vs this branch with COLI_RAMMAP/COLI_NUMA unset, confirming byte-identical output.

make check being green proves the tests pass; it doesn't specifically prove that the default path produces the same tokens. Given that the precedence chain in the expert lookup changed shape (RAM map -> pin -> LRU), that's the property I want stated explicitly. If it's already implied by one of your existing tests, just point me at it and that's enough.

That is the whole ask. Not another split, not a rewrite.

On the chain

Your own PR body says PR1 is the root with no dependency, and I'll treat it that way: merging this does not commit the project to #821 or #822. They get judged on their own terms, and they're much bigger, so they'll take longer. I don't want that to hold this one hostage.

One question so I can sequence properly rather than leave you guessing: is the RAM-map tier usable today without #821, if a user mounts a tmpfs and populates it themselves? If yes, this lands as a complete, self-contained capability and #821 becomes the convenience layer — which is a much easier thing for me to merge and for users to understand.

Thanks for the persistence on this one, and for splitting #377 when I asked. Give me the exactness check and I'll take it.

@BColsey

BColsey commented Aug 17, 2026

Copy link
Copy Markdown
Author

@JustVugg thanks for verifying the PROF question yourself instead of just asking me, and for saying outright what had been holding it up. Here's the exactness check, run the way you described it.

Token exactness with the feature off

I built dev (12368b7, this morning's tip) and this branch (6dde621) side by side on the same machine with the same compiler and flags, generated the tiny GLM oracle fixture once (c/tools/make_glm_oracle.py, seed 1234), and ran greedy decode on both with every gate scrubbed:

env -u COLI_RAMMAP -u COLI_NUMA -u COLI_RAM_PREFAULT -u COLI_MMAP -u COLI_WEIGHTS_DIR \
  SNAP=./glm_tiny COLI_TEMP=0 ./colibri 64 16 16

Both binaries emit:

Reference (oracle): 207 187 119 103 103 103 103 103 119 34 103 103 103 103 103 136 112 7 119 34
GLM C engine      : 207 187 119 103 103 103 103 103 119 34 103 103 103 103 103 136 112 7 119 34
Matching tokens: 20/20

I ran each side twice; all four runs came out identical. The only lines in the full stdout that differ between the two binaries are timing measurements (RSS, tok/s, PROFILE timings), which vary from run to run on the same binary anyway. Worth knowing in case you try it yourself: in serve mode the output does change with the flags unset — PROF grows a few fields, there are new GPUS/GPUDETAIL lines, TTFT appears in the run summary. That's the telemetry this PR adds; the token-bearing output is what's byte-identical.

It's already implied by CI, since you offered that as an out: the "Efficiency suite (tiny oracle)" job runs this PR's binary with none of the gates set, against the same seed-1234 fixture. Both dev @ 0578bba1 and this head logged PREFILL (teacher-forcing) C vs oracle: 32/32 positions — same torch and transformers versions on both sides, so same reference, both perfect. The Inkling and V4 tiny oracles also compile the st.h this PR changes and are green on both sides.

One thing I'd rather point out than have you find: the colibri oracle score is print-only. The binary returns 0 regardless of the match count, identically on dev — the 32/32 lives in the logs, not the exit code. Inkling gates it properly (return (match == ngen) ? 0 : 1;); I can mirror that for colibri as a one-line follow-up if you'd like the check enforced rather than just visible.

Your tmpfs question: yes, it works without #821

Everything the tier needs is in this PR. By hand:

  1. Copy the converted shards onto tmpfs (cp <model>/*.safetensors /dev/shm/<model>/, or a dedicated mount -t tmpfs) and symlink any shard you didn't stage back to the canonical dir. Hybrid layouts work — eligibility follows the opened file descriptor (fstatfs), not the path.
  2. Run with SNAP=<canonical> COLI_WEIGHTS_DIR=/dev/shm/<model> COLI_RAMMAP=1 ./colibri …. Optional: COLI_RAM_PREFAULT=1, COLI_NUMA=1 + COLI_NUMA_NODES=…. Combining with COLI_MMAP=1 is rejected. (COLI_WEIGHTS_DIR is new in this PR.)
  3. Check stderr at startup for [RAMMAP] N direct tmpfs experts, X GB mapped. If staging went wrong you'll see 0 direct tmpfs experts and it just runs the ordinary path. PROF=1 adds [PROF] physical SSD reads: 0.000 GB.

So this PR is usable on its own, and #821 is the convenience layer. #821 changes nothing in the engine's C sources or the serve protocol — it does touch c/Makefile, c/resource_plan.py, and one test file. It's the mount/staging/lifecycle automation and the coli ramdisk command.

The real gap is docs: that recipe currently exists only in the c/tests/test_rammap_e2e.py docstring, and docs/cuda.md mentions coli ramdisk, which doesn't exist until #821. I can put a short docs-only commit here (recipe into docs/ENVIRONMENT.md, fix the forward reference) or leave the docs to #821, where the full environment documentation lands anyway. Either works for me — your call.

Separately, dev has moved about 110 commits since my last forward-merge. The merge-tree dry-run against today's tip comes out clean, so once you've had a look at this I'll forward-merge and push. That way the head you merge includes current dev. That push will need the usual workflow approval click.

Forward-merge of dev (1658b1f..1cc0be2, 123 commits) ahead of review;
reconstruct history preserved.

Textual conflict (c/openai_server.py Engine.__init__): union of the
branch's spawn-refactor params with dev's family-registry param —
family stays the 7th positional (dev's serve() calls it positionally),
command_prefix/stderr trail as keyword-only additions.

Semantic conflicts beyond the textual merge:
- Engine(env) child env gains dev's V4_MTP_GPU default (b7117bf);
  branch's spawn-hook test expectation updated to match.
- test_cpu_vs_cpu_determinism tok/s tolerance flakes under local suite
  load (46% drift, identical hit rates, green 3x in isolation,
  untouched by both sides since the merge base) — left as-is for CI.
Forward-merge of dev (1cc0be2..c945cd5, 24 commits: cluster expert
workers, expert-store registry, qrow hoist, family-dispatch finish)
ahead of review; reconstruct history preserved.

# Conflicts:
#	c/colibri.c
Forward-merge of dev (c945cd5..256bbe9, 35 commits) ahead of review;
reconstruct history preserved.

# Conflicts:
#	c/colibri.c
@BColsey

BColsey commented Aug 18, 2026

Copy link
Copy Markdown
Author

@JustVugg — dev moved again overnight (~35 commits through 256bbe9), so I forward-merged once more; the branch is current and conflict-free at 61cd171. One textual conflict this time (expert_load_impl's unquantized fallback: dev's new qt_planarize kept alongside the tmpfs-filtered byte accounting and slot-backing tag). make -C c check is green (581 tests — the one tok/s timing wobble is local machine load, passes isolated), and I re-ran the greedy A/B against dev's new tip: byte-identical token streams, 20/20 both sides, gates unset.

The new head needs the usual workflow approval click whenever you have a moment. Once CI is green this is ready from my side.

@JustVugg

Copy link
Copy Markdown
Owner

Workflow approved — both runs are live on 61cd171. Noted and appreciated: the forward-merge kept qt_planarize alongside the tmpfs-filtered byte accounting in expert_load_impl's fallback (that's the K1 planar layout that landed overnight, #1079/#1086 — the flag must be reset and re-applied on every slab refill, which your resolution preserves). With your 20/20 byte-identical A/B against the new tip and green CI here, this moves to the front of the review queue.

Resolve overlapping CUDA accounting, engine telemetry, echo protocol, and dispatcher test changes for PR JustVugg#820.
@BColsey

BColsey commented Aug 20, 2026

Copy link
Copy Markdown
Author

@JustVugg — the merge conflicts are resolved and current dev is forward-merged at 6c99dcc. Both new workflow runs are action_required with no jobs, so they need the usual approval click: CI and check. Local validation completed: full C tests passed and all 172 OpenAI server tests passed; the broader Python run had only the previously documented scheduler-sensitive tok/s timing wobble. Once approved, CI can run.

Forward-merge upstream/dev at 6896e5f into PR JustVugg#820.\n\nResolve c/colibri.c by preserving PR JustVugg#820's weights_dir/RAMMAP path around upstream's KV_TQ safety guards. Resolve c/openai_server.py by retaining the shared termination helper and applying upstream's best-effort second reap wait there.
Resolve PR JustVugg#820's runtime and launcher conflicts while preserving RAMMAP-first residency, indexed pin/LRU publication, and measured-cap startup semantics.

Keep PILOT reservations mutex-published, io_uring telemetry descriptor-aware, and prefetch/victim bookkeeping race-free. Add deterministic regression coverage for RAMMAP, cache indexes, and PILOT handoff.
Resolve the follow-up GLM segment-loader and OpenAI server test conflicts introduced after upstream dev advanced.

Preserve the separate staged weights namespace across the range-based loader while retaining upstream Segment adapter registration and tool-call parsing coverage.
@BColsey

BColsey commented Aug 26, 2026

Copy link
Copy Markdown
Author

@JustVugg can you please approve the workflow

Resolve the follow-up Windows launcher conflicts after upstream dev advanced.

Preserve stream-specific Windows binary mode and staged weights/state initialization while retaining the shared console guidance and release launcher.
Resolve the v1.9.0 telemetry conflict while preserving RAMMAP, CUDA, and Vulkan residency accounting.

Account for Vulkan overlap by expert identity so host and RAMMAP tiers remain exclusive.
Bring PR 820 through upstream dev commit 2a12e44, including the GLM-5.3 release-build contract.
@BColsey

BColsey commented Aug 28, 2026

Copy link
Copy Markdown
Author

@JustVugg — could you please approve the two workflow runs for the new conflict-free head 94a38b0?

Both are currently action_required with no jobs started. GitHub reports the PR mergeable against current dev.

Forward-merge upstream/dev at b690bfb into PR JustVugg#820.

Preserve PR JustVugg#820 staged weights namespace and protected engine startup path while retaining upstream model lifetime, planned-cap cleanup, Qwen 3.8, and GLM-5.3 no-think fixes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discussion Proposta / discussione aperta, non un task 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