ReviewOS

zig-utils/zig-js

RegExp: bound compiled-program and matcher-scratch retention

#990
Closed glennmichael123 opened this 8 days ago ·
8 days ago

Problem

After #989 bounded the immutable RegExp lookup table, the programs and matcher scratch behind evicted entries are still arena-owned. Dropping an index entry therefore does not reclaim either allocation domain. Interpreter.regex_matchers also grows once per distinct compiled program, and each Regex.Matcher lazily retains DFA/VM scratch through the compiled regex's arena allocator.

This is a focused child of #480 and #473.

Executed evidence

Measured from exact main commit e3e2ca49119937119d8dbe32bc731f0743a2a5bc with a freshly built test262 runner. Each row is a fresh process; the source uses unique constant-shape patterns token_<i>_(?:alpha|beta)+$. strings builds the source only, construct eagerly validates/compiles with new RegExp, and match additionally calls .test() once. All rows returned the expected checksum.

patternsstrings RSSconstruct RSSmatch RSS
51219,988,48024,756,22428,622,848
2,04821,495,80838,436,86453,395,456
8,19227,361,28093,257,728152,748,032
32,76851,003,392316,293,120557,334,528

At 32,768 patterns, construction retains about 265 MB beyond the string control; matching retains another 241 MB beyond construction. The guarded matrix peaked at 497 MB of physical footprint and completed with the host at 44% low-water, so this is measured growth rather than a guard artifact.

Root cause

  • src/regexp_program_cache.zig bounds only 64 lookup entries. Its contract explicitly says eviction does not free programs or keys because they are context-arena-owned.
  • compileRegex allocates every regex.Regex and all compiled state from Interpreter.arena, then stores the pointer on the RegExp object.
  • regexMatcher stores one matcher per compiled pointer in an unbounded identity map.
  • zig-regex's matcher allocates its lazy DFA and VM scratch through re.allocator, which is the same context arena; even Matcher.deinit() cannot reclaim arena pages.

Acceptance

  • Give compiled programs and matcher scratch explicit interpreter-local, freeable ownership.
  • Bound the number of retained programs and matchers without lowering JavaScript limits or rejecting valid patterns.
  • Preserve immutable-program reuse while keeping mutable backtracking state isolated per RegExp/interpreter.
  • Make eviction and interpreter teardown reclaim owned allocations with no dangling object pointers or reentrant-use hazard.
  • Add focused ownership/eviction tests and tree/VM semantic witnesses.
  • Verify affected test262 RegExp subtrees, no-GIL, and TSan with no regressions.
  • Publish raw exact-parent before/after memory evidence and update #480 and #473.
8 days ago

Implementation landed on main in fc7ac53. The interpreter now owns compiled RegExp programs and matcher scratch in one 64-entry lease cache: unpinned eviction reclaims both, all-pinned saturation uses a transient owned lease, and mutable backtracking programs are keyed by stable RegExp identity. Interpreter teardown is wired through Context unregistration and the standalone Worker delivery path. Focused evidence under the 3072 MB guard: 4/4 cache ownership tests, 2/2 interpreter integration tests (plus the linked JIT test), and a successful test262-bin build; peak observed build/test footprint was 1969 MB. Remaining #990 work: exact-parent memory comparison, tree/VM test262, no-GIL/TSan verification, checked-in raw evidence/docs, then issue closure.

8 days ago

Implementation and local acceptance are complete on main:

  • fc7ac534 — explicit 64-entry interpreter-local lease ownership for compiled programs and matcher scratch.
  • fe0d3260 — recyclable compile/matcher/result arenas with stable pinned slots and transient all-pinned fallback.
  • 8de5d5f9 — exact-parent raw evidence, dated report, README/benchmarks links, and checked BunPress manifest.

The final exact-parent Debug matrix compares e3e2ca49 with fe0d3260. At 32,768 distinct patterns, construction peak RSS is 301.64 → 73.34 MiB (-75.7%) and construction plus first match is 531.52 → 108.28 MiB (-79.6%). The same-size source-string control is 48.64 → 48.67 MiB and every checksum matches. This is a one-sample retained-memory diagnostic, not a throughput claim.

Acceptance receipts, all run sequentially under the 3072 MiB guard:

  • cache ownership tests: 6/6;
  • interpreter integration: 2/2 plus linked JIT test;
  • unchanged upstream high-retention witness: 0:p, DONE;
  • explicit tiers: tree OK 228266, VM OK 228266;
  • affected test262: 2,456/2,456 pass, 0 fail (1,879 RegExp, 238 regexp literals, 339 String integrations);
  • real-collector no-GIL TSan: regexp-lastindex-shared.js and regexp-churn-threads.js both pass, 0 sanitizer reports;
  • docs: 342 links resolve; checked 51-page BunPress build passes.

Evidence: report · raw matrix and receipts · CI run.

The worktree is clean and HEAD == origin/main == 8de5d5f9. I am leaving the issue open only until the latest CI run is terminal.

8 days ago

Final CI follow-up after the original acceptance pass:

  • fdd9c2e in zig-regex separates persistent lazy-DFA/VM scratch from recyclable per-match result allocation. Its upstream CI run 36419826925 is green.
  • e07db373 in zig-js consumes that API, adds the exact lazy-DFA-after-VM regression, and pins all CI/performance checkouts plus the dependency inventory to the new revision.
  • Focused local evidence: zig-regex allocator regression 1/1; regexp program-cache module 7/7.
  • Post-push CI 36420281383: dependency audit passed, docs passed, test262-parallel passed, and the exact formerly crashing threads-gil (shard 3/4) passed. That shard previously aborted in semantics/regexp-lastindex-shared.js through dfa.zig:649 after result-arena reset invalidated lazily created matcher scratch.
  • Current no-GIL red shards report only the same pre-existing CVE baseline cases seen before this fix (mc-aint-poll-resume-stale-elided.js, mc-life-wasm-grow-relocate.js, and mc-grow-s4-detach-nullvec-repro.js); their logs contain no RegExp crash, dangling DFA, or new #990 regression.

Root cause: a shared global RegExp could first enter at nonzero lastIndex, priming VM scratch, then later enter at zero and lazily create DFA scratch while Regex.allocator pointed at the resettable result arena. OwnedMatch.deinit() reset that arena and left the cached DFA transition table dangling. Matcher scratch now has its own explicit program-owned allocator; result cleanup reclaims only per-call match data.

The implementation, measured memory evidence, documentation, dependency pin, focused ownership regressions, tier/test262 evidence, and authoritative shared-RegExp CI witness are complete. Closing #990.

Sign in to comment on this issue.