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; } 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/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) { 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")