From 5feaff3ce33c439e7808943abac5dad208970c04 Mon Sep 17 00:00:00 2001 From: Marco Bambini Date: Sun, 23 Aug 2026 11:52:12 +0200 Subject: [PATCH 1/3] VirtualLock is bounded by the minimum working set, so the raise missed it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cf3ecc6 raised the process maximum and deliberately preserved the minimum. The minimum is the bound: "the maximum number of pages that a process can lock is equal to the number of pages in its minimum working set minus a small overhead". So the fix wired exactly as much as the bug did — nothing, 5928 of 5928 slots refused, the same count at the same budget as the original report. Measured on the reporter's Windows host rather than reasoned about (#36): a standalone probe raised the maximum from 1.3 MB to 2049 MB and the same VirtualLock still failed with 1453; raising both bounds made it succeed. Two budgets, two passes each, all four identical afterwards — 2706 and 5928 cache slots wired, 0.96 GB of trunk wired at both, which is what it should be since the trunk does not scale with the cache. The same probe also settles the privilege question the old comment anticipated: the raise succeeded with SE_INC_WORKING_SET_NAME disabled, so ERROR_WORKING_SET_QUOTA was never about the privilege and the AdjustTokenPrivileges path does not have to be written. The latch stays. It is still reachable — a job object with a working-set cap refuses the raise — and it is what keeps a refused raise from costing 5928 doomed syscalls, ~4% in the original report. One comment beside `grow` is now false and is corrected rather than left: overshooting was free when this raised only the maximum, because a maximum is a ceiling. A minimum is a reservation the host honours, so `grow` is memory taken from the rest of the machine. Reported and fixed by GTSUltear in #36; unverified here, as before, but it cross-compiles -Werror clean under mingw. --- src/ecache.c | 70 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/src/ecache.c b/src/ecache.c index dd0cb5a..06dbdc1 100644 --- a/src/ecache.c +++ b/src/ecache.c @@ -101,29 +101,44 @@ static void ec_volatile(waste_ecache *c, int si) { (void)c; (void)si; } #include int waste_wire(void *p, size_t n) { return mlock(p, n) == 0; } #else -/* VirtualLock is bounded by the process *maximum working set*, not by - * available memory, and the default maximum is far below any cache worth - * wiring. So this wired nothing at all on Windows: 5928 of 5928 slots - * refused with ERROR_WORKING_SET_QUOTA on a machine with 54 GB free and - * nothing paging (#36, gap 5). mlock has no equivalent requirement, which - * is why the port worked everywhere else and this went unnoticed. +/* VirtualLock is bounded by the process working set, not by available + * memory, and the default is far below any cache worth wiring. So this + * wired nothing at all on Windows: 5928 of 5928 slots refused with + * ERROR_WORKING_SET_QUOTA on a machine with 54 GB free and nothing paging + * (#36, gap 5). mlock has no equivalent requirement, which is why the port + * worked everywhere else and this went unnoticed. * - * Raise the ceiling on demand rather than up front: waste_wire is handed one - * slot at a time and never learns the total, so the first refusal is the - * only place the size actually needed is known. SetProcessWorkingSetSize - * takes both bounds, so the current minimum is read back and preserved — - * passing a minimum above the maximum fails the call outright. + * The bound is the *minimum* working set, not the maximum — "the maximum + * number of pages that a process can lock is equal to the number of pages + * in its minimum working set minus a small overhead". The first fix for + * this raised the maximum and deliberately preserved the minimum, and so + * wired exactly as much as before: nothing. Measured on the reporter's host + * rather than reasoned about — the maximum went from 1.3 MB to 2049 MB and + * the lock still failed with 1453; raising both bounds made the same lock + * succeed. So `lo` has to move, and `hi` is carried up with it because a + * minimum above the maximum fails the call outright. * - * If the raise is refused — it can need SE_INC_WORKING_SET_NAME, which a - * service account may not hold — latch and stop asking. The caller's - * accounting is unchanged, since a latched call still reports failure; what - * goes away is thousands of syscalls that cannot succeed. Reporting the - * failure honestly is the existing behaviour and stays: an engine that - * refused to open because it could not wire its cache would be worse than - * one running with a pageable cache. + * That makes this a stronger request than it looks. Raising the minimum + * tells Windows to keep that many pages resident for this process, which is + * the point when wiring an expert cache — but the host gives up that memory, + * and `grow` is how much it gives up. * - * Unverified on a real Windows host: no machine here has one. It builds - * under both toolchains and the failure path is the previous behaviour. */ + * Raise on demand rather than up front: waste_wire is handed one slot at a + * time and never learns the total, so the first refusal is the only place + * the size actually needed is known. + * + * If the raise is refused — a job object with a working-set cap will refuse + * it — latch and stop asking. The caller's accounting is unchanged, since a + * latched call still reports failure; what goes away is thousands of + * syscalls that cannot succeed, which cost ~4% in the original report. + * Reporting the failure honestly is the existing behaviour and stays: an + * engine that refused to open because it could not wire its cache would be + * worse than one running with a pageable cache. + * + * SE_INC_WORKING_SET_NAME is *not* required for this: the reporter's probe + * raised both bounds successfully with the privilege disabled, so the + * AdjustTokenPrivileges path an earlier version of this comment anticipated + * does not have to be written. */ static int wire_quota_exhausted; /* raise refused; stop trying */ int waste_wire(void *p, size_t n) @@ -138,11 +153,18 @@ int waste_wire(void *p, size_t n) return 0; } /* Room for this slot and the ones behind it, so the raise is not paid - * once per slot. Overshooting costs nothing: the maximum is a ceiling - * the process may reach, not a reservation. */ + * once per slot. Overshooting is not free here, unlike when this raised + * only the maximum: the minimum is a reservation the host honours, so + * `grow` is memory taken from the rest of the machine. One slot plus + * as much again is the smallest step that still amortises. */ const SIZE_T grow = n + (n < (64u << 20) ? (64u << 20) : n); - if (hi > (SIZE_T)-1 - grow || !SetProcessWorkingSetSize(GetCurrentProcess(), - lo, hi + grow)) { + if (lo > (SIZE_T)-1 - grow) { + wire_quota_exhausted = 1; + return 0; + } + const SIZE_T need = lo + grow; + if (!SetProcessWorkingSetSize(GetCurrentProcess(), need, + need > hi ? need : hi)) { wire_quota_exhausted = 1; return 0; } From acba190360cce1318464d6fbdf103b19b74b0de3 Mon Sep 17 00:00:00 2001 From: Marco Bambini Date: Sun, 23 Aug 2026 11:52:28 +0200 Subject: [PATCH 2/3] diskbench narrowed its offset to off_t, so 8 GB of file was 2 GiB of reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `waste_pread` takes int64_t and splits it into ReadFile's Offset/OffsetHigh pair precisely so a read past 2 GiB works. `rand_reader` handed it an off_t, which under LLP64 — MSYS2 UCRT64, the documented Windows build path — is a signed 32-bit long. The default file size is 8 GB, so this was the default path and not a corner. Two failure modes, and the quiet one is worse. Offsets that wrapped negative were refused by waste_pread's `off < 0` guard, which returns -1 without touching errno: "short read -1: No error", exactly one per reader thread. The thread then breaks out while the bytes it never read still divide into its elapsed time, which is why the random rows came out ~40% low and *inverted* with thread count. Offsets that wrapped positive did not fail at all — they read the wrong place successfully, keeping the entire working set inside the first 2 GiB, which sits in a consumer SSD's SLC cache and DRAM. That is the flattery this tool exists to avoid. Reported independently and with the same diagnosis by GTSUltear (#36 gap 4) and mfethe1 (#43); the measurement below is mfethe1's, on a Zen 2 / PCIe 4.0 box, three passes per cell: diskbench PATH 8 12 4 before after short reads 7 x "-1: No error" none rand 1 thr 0.36 GB/s 1.48 GB/s rand 2 thr 0.11 GB/s 2.23 GB/s rand 4 thr 0.06 GB/s 2.25 GB/s seq write / seq read 2.25 / 2.17 2.21 / 2.13 The sequential rows barely move, which is what makes the pair comparable. A 1.5 GB control — every offset under 2^31 — agrees between the two builds. docs/GATES.md Gate H and the README's 12.78 vs 0.94 GB/s are unaffected: they were measured on macOS, where off_t is 64-bit. Confirmed here, where the patched tool still reports 9.6-12.5 GB/s on the internal SSD. Also `~4095UL`, which mfethe1 flagged and left out of #43 as a defect he had not measured. It is the same class one line further on: under LLP64 that is a 32-bit mask which zero-extends and clears the high half of a size_t. It only bites a record size >= 4 GiB, so nothing observable changes — but the file already carries one truncation that looked like a measurement, and `~(size_t)4095` closes the other by construction. --- tools/diskbench.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tools/diskbench.c b/tools/diskbench.c index 3b80b53..bc0009e 100644 --- a/tools/diskbench.c +++ b/tools/diskbench.c @@ -247,7 +247,15 @@ static void *rand_reader(void *p) { double got = 0; for (int i = 0; i < g_reps; i++) { seed = seed * 1103515245u + 12345u; - off_t off = (off_t)(seed % nrec) * g_rec; + /* int64_t, not off_t, which is a signed 32-bit long under LLP64 — + * MSYS2 UCRT64, the documented Windows path — and truncated every + * offset at the default 8 GB file. The loud half wrapped negative + * and waste_pread refused it, which cost ~40% off the random rows + * and inverted them with thread count. The quiet half wrapped + * positive and read the wrong place *successfully*, keeping the + * working set inside the first 2 GiB — an SSD's SLC cache, which + * is the flattery this whole tool exists to avoid. #36 gap 4. */ + int64_t off = (int64_t)(seed % nrec) * (int64_t)g_rec; int64_t r = waste_pread(fd, buf, g_rec, off); if (r != (int64_t)g_rec) { fprintf(stderr, "short read %lld: %s\n", (long long)r, strerror(errno)); break; } got += r; @@ -274,7 +282,7 @@ int main(int argc, char **argv) { * cannot be derived from what was measured does not get printed. */ double gb_tok = argc > 5 ? atof(argv[5]) : 0.0; g_file = (size_t)(file_gb * (1u << 30)); - g_rec = (size_t)(rec_mb * (1u << 20)) & ~4095UL; + g_rec = (size_t)(rec_mb * (1u << 20)) & ~(size_t)4095; /* A record under one page rounds to zero and divides by it two screens * further down, as a crash rather than as a usage error. */ if (!g_rec || g_file < g_rec) { From 9aa071bbab367e8a457581f412dacc361a37be81 Mon Sep 17 00:00:00 2001 From: Marco Bambini Date: Sun, 23 Aug 2026 11:52:44 +0200 Subject: [PATCH 3/3] A container's JSON went through Python text mode, so Windows built other bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GTSUltear reported in #36 that the rotary fixture declares itself stale on Windows, and the reply here assumed it was the same staleness already skipping on macOS. It is not. The fixture is fine: $ python3 tools/make_test_container.py --rope --seed 0 rope.waste got 7ae7f6c06a6c8956b668d334c2b14ae9a7d94e9adc1bbbc89db3c30563b6510b want 7ae7f6c06a6c8956b668d334c2b14ae9a7d94e9adc1bbbc89db3c30563b6510b `manifest.json` and `specials.json` were opened in text mode, so on Windows Python writes CRLF, the container hashes differently, and the provenance gate cf3ecc6 added reports "regenerate me" against a fixture that is perfectly good. That is #36 gap 2 — CRLF out of a Python stdout — one layer in, and it lands on the gate written to catch a different problem. The gate is right and stays. What was wrong is that the artefact it hashes was not reproducible across platforms in the first place. convert.py and requant_vision.py get the same treatment, because they write the JSON of containers people actually ship and the defect is identical: a K3 container converted on Windows would not be byte-comparable with one converted anywhere else. Nothing fails loudly there — the hand-written parser in the engine takes CRLF as ordinary JSON whitespace — which is exactly why it would have gone unnoticed until some future gate hashed a manifest and blamed the weights. Verified here: the macOS hash is unchanged, so the shipped fixture stays valid and Windows now agrees with it rather than needing a new one. 53 passed, 0 failed, 6 skipped against Kimi-Linear and K3, unchanged. --- tools/convert.py | 8 ++++++-- tools/make_test_container.py | 14 ++++++++++++-- tools/requant_vision.py | 4 +++- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/tools/convert.py b/tools/convert.py index 4b45035..8df74de 100644 --- a/tools/convert.py +++ b/tools/convert.py @@ -70,8 +70,12 @@ def atomic_copyfile(src, dst): def atomic_json(path, value): + # newline="\n" so a container converted on Windows is byte-identical to + # one converted anywhere else. Python's text mode translates, the engine + # parses either, and nothing fails loudly — what breaks is every gate + # that hashes a container to prove its provenance. #36 gap 2. tmp = path + ".tmp" - with io.open(tmp, "w", encoding="utf-8") as out: + with io.open(tmp, "w", encoding="utf-8", newline="\n") as out: json.dump(value, out, indent=1) out.flush() os.fsync(out.fileno()) @@ -1400,7 +1404,7 @@ def reclaim_layer(L, size): f"manifest listed: {', '.join(dropped)}", file=sys.stderr) manifest_tmp = manifest_path + ".tmp" - with open(manifest_tmp, "w") as f: + with open(manifest_tmp, "w", newline="\n") as f: # see atomic_json json.dump(manifest, f, indent=1) f.flush() os.fsync(f.fileno()) diff --git a/tools/make_test_container.py b/tools/make_test_container.py index 3572aa6..8c376ed 100644 --- a/tools/make_test_container.py +++ b/tools/make_test_container.py @@ -260,7 +260,13 @@ def write_tokenizer(outdir): base = len(tokens) specials = [{"id": base + i, "text": s} for i, s in enumerate(SPECIALS)] - with open(os.path.join(outdir, "specials.json"), "w") as f: + # newline="\n" on every JSON the container carries. Python's text mode + # translates on Windows, so the same seed builds a byte-different + # container there. The engine parses it either way and nothing fails + # loudly — what breaks is any gate that hashes a container to prove + # provenance. #36 gap 2, one layer in. + with open(os.path.join(outdir, "specials.json"), "w", + newline="\n") as f: json.dump(specials, f, indent=1) return base + len(SPECIALS) @@ -443,7 +449,11 @@ def main(): "layers": layers, "trunk": t.index, } - with open(os.path.join(args.out, "manifest.json"), "w") as f: + # See the note beside specials.json above: this container is hashed by + # tests/run.sh to date the rotary fixture, so a CRLF manifest reads as + # "regenerate me" on Windows against a fixture that is perfectly good. + with open(os.path.join(args.out, "manifest.json"), "w", + newline="\n") as f: json.dump(manifest, f, indent=1) total = sum(os.path.getsize(os.path.join(args.out, f)) diff --git a/tools/requant_vision.py b/tools/requant_vision.py index d7a6466..72f240a 100644 --- a/tools/requant_vision.py +++ b/tools/requant_vision.py @@ -121,7 +121,9 @@ def main(): os.fsync(tf.fileno()) tmp = man_path + ".tmp" - with io.open(tmp, "w", encoding="utf-8") as f: + # newline="\n": a manifest rewritten on Windows must stay byte-comparable + # with the container convert.py built. #36 gap 2. + with io.open(tmp, "w", encoding="utf-8", newline="\n") as f: json.dump(man, f, indent=1) os.replace(tmp, man_path) # the container flips in one step print(f"appended {added / 2**20:.0f} MB, manifest updated")