Orchestrata Ultimate › Plans › spec-110-codemod-fast-c-rewrite

SPEC-110 _codemod_fast — C extension rewrite spec

Source: plans/spec-110-codemod-fast-c-rewrite.md at 8a2cb42ad · last changed 2026-07-27 · first committed 2026-07-27 · 204 lines.

Prose pending.

Plan text

SPEC-110 _codemod_fast — C extension rewrite spec

Target: scripts/_codemod_fast/ on feat/codemod-engine (wt-QWEN) Status: spec — not yet built. Do not edit wt-QWEN while opencode is building. Author: Mohamed Date: 2026-07-27 Motivation: measured review of the existing 473 LOC found 1 of 4 functions delivering its claim, and 1 actively breaking a safety guard.

0. Findings this spec responds to

Read against the current implementation, not the design doc.

Two structural defects across all four:

1. No Py_BEGIN_ALLOW_THREADS anywhere. Every compute region runs with the thread state attached.
2. Every function touches the Python object layer at its boundaries (PyList_GetItem, PyLong_FromLong, PySet_Add, refcounts) — which is exactly where the open free-threading races live (CPython #149816, #153852, #150695 et al). This C code participates in the crash class instead of sidestepping it.

1. The governing pattern

Every function in this extension MUST follow this shape. No exceptions without a written reason.

PyObject* fn(PyObject* self, PyObject* args) {
    Py_buffer in;
    if (!PyArg_ParseTuple(args, "y*", &in)) return NULL;   /* buffers in  */

    /* allocate raw output with malloc/calloc — no PyObject */
    int32_t* out = malloc(n * sizeof(int32_t));
    if (!out) { PyBuffer_Release(&in); return PyErr_NoMemory(); }

    Py_BEGIN_ALLOW_THREADS                                  /* detach      */
    compute(in.buf, in.len, out, n);                        /* PURE C      */
    Py_END_ALLOW_THREADS                                    /* re-attach   */

    PyObject* res = PyBytes_FromStringAndSize((char*)out, n * sizeof(int32_t));
    free(out);
    PyBuffer_Release(&in);
    return res;                                             /* buffer out  */
}

Invariant: between Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS there is not one Python C-API call. No Py* symbol, no refcount, no allocation through the Python allocator. That is what makes the region genuinely parallel and immune to the object-layer races.

This is the whole point of the rewrite. A C extension that marshals Python objects element by element buys nothing over the Python it replaces and inherits every race.

2. Per-function specification

2.1 content_hash — DELETE (blocking)

Do not fix this in C. Remove it.

The declared dependency xxhash>=3.0.0 is itself a C extension wrapping upstream libxxhash with SIMD XXH3. Reimplementing it in our own C cannot be faster and can only diverge. Delete content_hash.c, drop xxhash3_64 from _native, and make _fast_fallback.xxhash3_64 the single implementation — it already calls xxhash.xxh3_64(data).hexdigest() (_fast_fallback.py:115).

Why this is blocking, not cosmetic. REQ-110-015's concurrent-edit guard hashes every file at load() and re-checks at commit(). With two implementations under one name, a file hashed with the extension present and re-checked with it absent reads as modified — the guard either refuses a legitimate commit or, on the inverse path, fails to catch a real concurrent edit. The extension is currently not installed, so installing it later silently changes commit behaviour.

Also remove the SHA-256 truncation fallback in _fast_fallback.py:117 for the same reason: three implementations behind one name is worse than two. xxhash is a hard dependency — if it is missing, fail loudly.

Acceptance: one hash implementation reachable in the whole codebase; a test asserts xxhash3_64(b"...") equals xxhash.xxh3_64(b"...").hexdigest() exactly.

2.2 mmap_loader — DELETE unless a benchmark defends it

Current code cannot be zero-copy: it copies into PyBytes before unmapping. And the consumer defeats the premise anyway — LibCST parses str, so the bytes get decoded, which is another full copy regardless of how they were read.

For ~2,500 sequentially-read source files, Path.read_bytes() is the baseline to beat. mmap wins for large files, random access, or repeated access — none of which is the workspace-load pattern.

Default action: delete, use Path.read_bytes(). Keep it only if §4's benchmark shows a material win on the real tree, in which case it must be rewritten as a genuine zero-copy: a custom type exposing bf_getbuffer/bf_releasebuffer that defers munmap to buffer release. Do not keep the current mmap-then-copy form under any circumstance — it is slower than the alternative it was written to replace.

2.3 csr_build — REWRITE to the pattern, or drop

Signature change. Stop accepting list[tuple[int, int]]. Accept two array.array('i') (or one flat interleaved buffer) so input arrives as Py_buffer with zero per-edge object access:

csr_build(node_count: int, src: array('i'), dst: array('i')) -> tuple[bytes, bytes]

Output change. Return raw bytes for offsets and neighbors. The caller wraps them once:

offsets = array.array("i"); offsets.frombytes(off_bytes)

That is two bulk copies instead of ~17,500 PyLong allocations.

Threading. Both counting passes and the prefix sum go inside ALLOW_THREADS.

Honest scope note. CSR build is O(E) over ~15,000 edges and runs once at load, against a parse step budgeted at 120 seconds. Python does this in milliseconds. If §4's benchmark shows the C version saving less than a few tens of milliseconds on the real graph, drop it and keep the Python fallback as the only implementation. Do not carry C for a path that is not hot.

2.4 csr_bfs — KEEP, two fixes

The traversal (csr_index.c:183-204) is already correct and genuinely fast. Two changes:

1. Wrap lines 174–204 in Py_BEGIN_ALLOW_THREADS / Py_END_ALLOW_THREADS. The region touches only int*, char* and locals — it qualifies as-is.
2. Return bytes (the visited node ids as packed int32) instead of building a PySet with a PyLong per node. The caller makes a set once if it needs one.

This is the function that justifies the extension existing. It is also the one that, after fix 1, runs truly in parallel across the 12 cores.

3. Parity tests (REQ-110-129) — currently unprovable

The extension is not installed (ModuleNotFoundError: codemod_fast), so every C/Python parity test either skips or silently exercises the fallback twice. Wave 5's parity claim cannot have been met.

Required:

1. A gate test that fails when the extension is absent while declared. codemod-fast is a declared dependency in pyproject.toml; if the import fails, that is a broken environment, not a reason to skip. It must be an error. (RUL-002: no skips.)
2. A test that asserts which path is active, so a green parity run cannot mean "fallback vs fallback".
3. Per-function parity on real inputs from the live tree: identical CSR arrays, identical BFS result sets, identical hash digests.
4. Free-threaded parity under load — run the C path from 12 concurrent threads over the real CIG and assert identical results plus zero crashes. This is the test that would have caught the missing ALLOW_THREADS and any object-layer touching.

4. Benchmark gate — measure before keeping any of it

No C function survives without a number. For each of csr_build, csr_bfs, and (if retained) the loader, benchmark C vs the Python fallback on the real orchestrata-ultimate tree — 2,500 files, ~15,000 edges — and record it in the commit message.

Keep threshold: ≥2× on a path that is actually hot. Anything below that is C-maintenance debt for no gain, and should be deleted in favour of the Python fallback.

This gate exists because every claim in the current implementation's design doc — 10× CSR, 5 GB/s hash, zero-copy mmap — turned out to be unsupported by the code that shipped under it. Numbers, not adjectives.

5. Order of work

1. content_hash deletion + single-implementation hash test — blocking, do first, it is a correctness bug in a safety guard
2. csr_bfs ALLOW_THREADS + buffer return — the real win
3. Benchmark harness (§4) against the Python fallback
4. csr_build rewrite only if step 3 justifies it
5. mmap_loader deletion (or zero-copy rewrite only if step 3 justifies it)
6. Parity + FT-under-load tests (§3), including the absent-extension gate
7. Install the extension in the dev environment so parity actually runs

6. Relationship to the free-threading crash class

The open CPython free-threading races (#149816, #153852, #150695, #126365 and others — see memory python314t-heap-corruption-investigation) all live in the object layer: refcounts, dealloc, dict/list/type mutation. Code that spends its time in ALLOW_THREADS regions over raw buffers is structurally outside that class.

That makes this rewrite more than a performance exercise: it is the one place where moving work into C measurably reduces exposure to crashes we cannot fix. It does not remove the exposure — the boundaries still marshal objects — but it shrinks the window in proportion to how much of the runtime sits inside the detached regions.

Related

• Plans
• Orchestrata Ultimate — project index
• Workflow
• Tasks