perf: guard the big-int rescue scan on a necessary condition #71

Merged
yuris merged 4 commits from perf/69-quote-guard into main 2026-08-20 17:29:33 +00:00
Owner

Closes #69.

quote_big_ints ran its pattern gsub unguarded at both callsites — once per frame in decode_server, once per row in decode_row. On a table with no 128-bit column every one of those scans rewrote nothing and still cost about what the parse did. Its sibling quote_wide_ints has had a necessary-condition guard for a while; this gives the big-int scan the same shape.

The condition: every key the pass can rescue contains the byte run "__. A plain find is ~free, and absent that run the pattern cannot match. A "__ inside a row's double-encoded string is a false positive — it runs the scan for nothing, which is exactly the old behaviour. There is no false negative, which is the only direction that could corrupt a u128.

Measurement

Both from dev/tools/conformance.sh, 10000 rows, STDB_BULK_RUNS=3, hand-written schema, same machine. The harness runs two engine processes per invocation; the second is quoted here.

Before, on main @ a606406:

BULK SUMMARY rows=10000 runs=3 frame min=29.2 med=29.3 max=29.4ms spread=0% med pre=0.5 decode=13.2 quote=11.7 build=3.6 post=0.1ms share pre=2% dec=45% quo=40% bld=12% post=0% us_per_row=2.93

After, on this branch:

BULK SUMMARY rows=10000 runs=3 frame min=20.0 med=20.2 max=20.6ms spread=3% med pre=0.5 decode=12.4 quote=0.0 build=6.9 post=0.2ms share pre=3% dec=62% quo=0% bld=34% post=1% us_per_row=2.02

quo 40% → 0%, frame median 29.3ms → 20.2ms. PHASE5 (identity-keyed table) passes, which is the live exercise of the rescue path — the guard does not defeat it. HTML5 conformance also passes, with quo=0% there too.

The harness needed correcting, not the guard

PHASE6 asserted quote_calls + wide_calls == decode_calls — "exactly one quoting scan precedes every decode". The guard makes that false by design: the gsub is now reached only when the guard hits. So the invariant is re-derived rather than relaxed, and it is now strictly stronger:

  • guard_calls + wide_calls == decode_calls — every decode is still preceded by exactly one quoting attempt;
  • quote_calls == guard_hits — the rescue gsub runs on exactly the attempts that hit, never on a miss. A mismatch catches both failure directions: a miss leaking through to the scan (the cost this PR removes, back again), or a hit being swallowed (a rescue silently skipped, which would corrupt u128 columns).

The guard is counted, not timed, for the same reason the wide-int guard is: timing it was tried first and billed ~1.8ms of pure instrumentation at 10k rows — 10% of the frame, all apparatus, since two clock reads cost more than the find they measure. One clock read per apply still marks the apply's start. That is why quote reads 0.0ms rather than a small number.

A note on the unit test

The issue's sketch asked for assert(rawequal(out, input)) to pin that the guard skips the copy. That assert cannot discriminate here and was dropped: this is Lua 5.1, which interns every string, so a gsub that matched nothing hands back an object already rawequal to its input. Verified directly — the assert passes with the guard removed entirely, and it passed with a 2400-byte frame too, so it is not a short-string artefact. Publishing it would have been a test that cannot fail.

What the tests assert instead is the part that would actually break if the condition were too strong: a frame with no "__ reaches json.decode unrewritten, a frame carrying a rescuable key is still rewritten and still decodes to exact digits, and the escaped \"__connection_id__\" form inside row data still round-trips. The performance claim is carried by the quo share above, which is where it belongs.

README

The performance section described the scan as unguarded and bounded a codec swap at 6–8x on that basis. Re-measured and rewritten rather than patched: every number in it moved, and the two claims that went stale in the unfavourable direction are called out rather than dropped.

  • Native @10k: json.decode 11.9ms, quoting 0.0ms, construction 5.6ms, frame 18.0ms.
  • Wasm @10k: 28ms / 0ms / 17.5ms, frame 92.5ms, with ~51% of it socket delivery and rAF that no decoder touches.
  • Codec-swap bound is now ~3x native (was quoted 6–8x) and ~1.4x wasm. The guard already collected the part a BSATN decoder would have deleted.
  • Crossings, swept across rather than extrapolated: 16.7ms at ~8600 rows native (was 6100), 100ms at ~45000 (was 31000), 100ms at ~10700 rows wasm (was 9800).
  • The generated-schema penalty was documented as "about a tenth". It is now ~60% (29.3ms vs 18.0ms at 10k, 57–66% across a matched sweep) — decode_typed never paid the per-row big-int scan, so it got nothing from this guard while the hand-written path got everything. Nearly half a typed frame is construction.

Sweep data behind those crossings: native 7000→13.2ms, 8000→14.7, 8500→16.3, 9000→18.2, 10000→18.0, 25000→53.6, 40000→90.5, 45000→98.7, 50000→112.9 (pooled medians, two processes each). Wasm 1000→21.5, 5000→46.5, 10000→92.5, 11000→102.5, 12000→106.

stylua --check clean, 7 passed, 0 failed, both conformance harnesses pass.

Closes #69. `quote_big_ints` ran its pattern `gsub` unguarded at both callsites — once per frame in `decode_server`, once per *row* in `decode_row`. On a table with no 128-bit column every one of those scans rewrote nothing and still cost about what the parse did. Its sibling `quote_wide_ints` has had a necessary-condition guard for a while; this gives the big-int scan the same shape. The condition: every key the pass can rescue contains the byte run `"__`. A plain `find` is ~free, and absent that run the pattern cannot match. A `"__` inside a row's double-encoded string is a false positive — it runs the scan for nothing, which is exactly the old behaviour. There is no false negative, which is the only direction that could corrupt a u128. ## Measurement Both from `dev/tools/conformance.sh`, 10000 rows, `STDB_BULK_RUNS=3`, hand-written schema, same machine. The harness runs two engine processes per invocation; the second is quoted here. Before, on `main` @ a606406: ``` BULK SUMMARY rows=10000 runs=3 frame min=29.2 med=29.3 max=29.4ms spread=0% med pre=0.5 decode=13.2 quote=11.7 build=3.6 post=0.1ms share pre=2% dec=45% quo=40% bld=12% post=0% us_per_row=2.93 ``` After, on this branch: ``` BULK SUMMARY rows=10000 runs=3 frame min=20.0 med=20.2 max=20.6ms spread=3% med pre=0.5 decode=12.4 quote=0.0 build=6.9 post=0.2ms share pre=3% dec=62% quo=0% bld=34% post=1% us_per_row=2.02 ``` `quo` 40% → 0%, frame median 29.3ms → 20.2ms. PHASE5 (identity-keyed table) passes, which is the live exercise of the rescue path — the guard does not defeat it. HTML5 conformance also passes, with `quo=0%` there too. ## The harness needed correcting, not the guard PHASE6 asserted `quote_calls + wide_calls == decode_calls` — "exactly one quoting scan precedes every decode". The guard makes that false by design: the `gsub` is now reached only when the guard *hits*. So the invariant is re-derived rather than relaxed, and it is now strictly stronger: - `guard_calls + wide_calls == decode_calls` — every decode is still preceded by exactly one quoting *attempt*; - `quote_calls == guard_hits` — the rescue `gsub` runs on exactly the attempts that hit, never on a miss. A mismatch catches both failure directions: a miss leaking through to the scan (the cost this PR removes, back again), or a hit being swallowed (a rescue silently skipped, which would corrupt u128 columns). The guard is **counted, not timed**, for the same reason the wide-int guard is: timing it was tried first and billed ~1.8ms of pure instrumentation at 10k rows — 10% of the frame, all apparatus, since two clock reads cost more than the `find` they measure. One clock read per apply still marks the apply's start. That is why `quote` reads `0.0ms` rather than a small number. ## A note on the unit test The issue's sketch asked for `assert(rawequal(out, input))` to pin that the guard skips the copy. That assert cannot discriminate here and was dropped: this is **Lua 5.1, which interns every string**, so a `gsub` that matched nothing hands back an object already `rawequal` to its input. Verified directly — the assert passes with the guard removed entirely, and it passed with a 2400-byte frame too, so it is not a short-string artefact. Publishing it would have been a test that cannot fail. What the tests assert instead is the part that would actually break if the condition were too strong: a frame with no `"__` reaches `json.decode` unrewritten, a frame carrying a rescuable key is still rewritten and still decodes to exact digits, and the escaped `\"__connection_id__\"` form inside row data still round-trips. The performance claim is carried by the `quo` share above, which is where it belongs. ## README The performance section described the scan as unguarded and bounded a codec swap at 6–8x on that basis. Re-measured and rewritten rather than patched: every number in it moved, and the two claims that went stale in the *unfavourable* direction are called out rather than dropped. - Native @10k: `json.decode` 11.9ms, quoting 0.0ms, construction 5.6ms, frame 18.0ms. - Wasm @10k: 28ms / 0ms / 17.5ms, frame 92.5ms, with ~51% of it socket delivery and rAF that no decoder touches. - Codec-swap bound is now ~**3x** native (was quoted 6–8x) and ~1.4x wasm. The guard already collected the part a BSATN decoder would have deleted. - Crossings, swept across rather than extrapolated: 16.7ms at ~8600 rows native (was 6100), 100ms at ~45000 (was 31000), 100ms at ~10700 rows wasm (was 9800). - The generated-schema penalty was documented as "about a tenth". It is now ~**60%** (29.3ms vs 18.0ms at 10k, 57–66% across a matched sweep) — `decode_typed` never paid the per-row big-int scan, so it got nothing from this guard while the hand-written path got everything. Nearly half a typed frame is construction. Sweep data behind those crossings: native 7000→13.2ms, 8000→14.7, 8500→16.3, 9000→18.2, 10000→18.0, 25000→53.6, 40000→90.5, 45000→98.7, 50000→112.9 (pooled medians, two processes each). Wasm 1000→21.5, 5000→46.5, 10000→92.5, 11000→102.5, 12000→106. `stylua --check` clean, `7 passed, 0 failed`, both conformance harnesses pass.
Author
Owner

All eight findings actioned. Three of them changed the shipped code, not just the prose — thank you, particularly for mutation-testing the guard and for catching that apparatus was reporting exactly double.

The guard now keys on the initials, not the bare "__ run

Finding 3 was the one with real reach. Keying on "__ left the frame-level scan in place for every InitialSubscription (unescaped "__time_duration_micros__") — i.e. for every caller of the documented legacy db:subscribe, which is the ~5.6ms you measured. Taking the suggestion:

if not (text:find('"__c', 1, true) or text:find('"__u', 1, true)) then
	return text
end

Still a necessary condition — both BIG_INT_FIELDS keys spell out to "__connection_id__ or "__uuid__ — and "__t is not rescuable, so the micros envelope short-circuits too. A TransactionUpdate still hits, correctly: its __connection_id__ is a value the scan exists to rescue.

Because that condition is now keyed on the initial, a future BIG_INT_FIELDS key starting with another letter would be silently short-circuited. Noted at the table itself, since that is where someone would add one.

timed_calls was reporting double

Finding 1, taken verbatim: local timed_calls = (rows + 1) + quote_calls. Confirmed on the hand-written path — apparatus=1.49ms → 0.71ms, matching the 0.74ms you measured on the typed path for the same 10001 timed calls. The comment claiming "one timed quoting scan per row" and "the exact size of the bias against the hand-written schema" is rewritten, since post-guard that bias is zero.

guard_hits ~= 0 added

Finding 5 is right that quote_calls == guard_hits is 0 == 0 and cannot see a guard broadened until it hits everywhere. Added alongside, mirroring the wide-int guard four lines below. Both assertions kept: the equality catches a swallowed hit (a rescue silently skipped), the zero-check catches a restored scan. Passes on both tiers.

__uuid__ was genuinely uncovered

Finding 4, taken verbatim. Verified it discriminates — mutating the guard to find('"__connection_id__'):

FAIL dev/test/test_protocol.lua: FAIL the guard does not defeat the uuid rescue either: expected ffffffff-ffff-ffff-ffff-ffffffffffff, got nil
6 passed, 1 failed

Finding 6 accepted as stated: the PLAIN assertion documents the contract rather than testing it, and the comment now says so instead of claiming it covers over-tightening. I also dropped two assertions I had duplicated from the pre-existing block 30 lines above (micros-stay-numbers, escaped-key).

README: the construction row was the guard's own cost

Finding 2 was correct and the fix needed a measurement, not a rewording. Paired main-vs-branch runs at STDB_BULK_RUNS=5, back to back in one session:

main   frame med=30.1ms  decode=13.8  quote=12.0  build=3.9  apparatus=1.49
main   frame med=30.0ms  decode=14.4  quote=12.1  build=3.8  apparatus=1.48
branch frame med=20.2ms  decode=12.4  quote=0.0   build=7.1  apparatus=0.71
branch frame med=21.5ms  decode=12.0  quote=0.0   build=8.9  apparatus=0.72

build 3.85 → 8.0ms with no construction code touched, exactly as you said. Microbenchmarked the two contributors at 10000 rows on a row of this table's shape:

raw find, the guard as shipped                 2.18 ms
same, through the harness find wrapper         3.23 ms
the old unguarded gsub, now skipped            6.19 ms

So construction is published as main's 3.9ms, and the table gained an explicit unbilled: the guard, and the harness's own wrapper | ~3.3ms row rather than hiding ~3.3ms inside a bucket labelled construction. The codec bound moves to ~2.4x (from the ~3x I had, which was computed off the inflated residual).

Finding 7: arithmetic fixed and re-measured rather than just recomputed — wasm 1000 rows is 22.0ms pooled over six samples, so 17ms is 77% of it. The wasm column is also corrected against a fresh STDB_BULK_RUNS=3 run (decode 28 → 23.5ms, build 17.5 → 19.5ms, bound ~1.4x → ~1.3x).

Native crossings re-swept with the final code, pooled: 8000 → 15.8ms, 8500 → 16.9, 9000 → 17.7, 45000 → 107.7. So 16.7ms at ~8500 rows and 100ms at ~42000, both stated to one significant figure now, with the observed 15-25% spread quoted beside them. The typed penalty lands at ~50% against the paired 20.9ms baseline.

Finding 8: taken — full lines below, apparatus included, since that field is what exposed finding 1.

main   BULK SUMMARY rows=10000 runs=5 frame min=29.6 med=30.1 max=33.8ms spread=14% med pre=0.5 decode=13.8 quote=12.0 build=3.9 post=0.1ms share pre=2% dec=46% quo=40% bld=13% post=0% us_per_row=3.01 quant=+/-0.11ms apparatus=1.49ms
branch BULK SUMMARY rows=10000 runs=5 frame min=18.6 med=20.2 max=21.9ms spread=16% med pre=0.5 decode=12.4 quote=0.0 build=7.1 post=0.1ms share pre=2% dec=61% quo=0% bld=35% post=1% us_per_row=2.02 quant=+/-0.11ms apparatus=0.71ms

Suite 7 passed, 0 failed, stylua --check clean, and both conformance harnesses pass on the final code — native all six phases, HTML5 green with quo=0% and the new guard_hits assertion holding on the wasm tier too.

All eight findings actioned. Three of them changed the shipped code, not just the prose — thank you, particularly for mutation-testing the guard and for catching that `apparatus` was reporting exactly double. ## The guard now keys on the initials, not the bare `"__` run Finding 3 was the one with real reach. Keying on `"__` left the *frame-level* scan in place for every `InitialSubscription` (unescaped `"__time_duration_micros__"`) — i.e. for every caller of the documented legacy `db:subscribe`, which is the ~5.6ms you measured. Taking the suggestion: ```lua if not (text:find('"__c', 1, true) or text:find('"__u', 1, true)) then return text end ``` Still a necessary condition — both `BIG_INT_FIELDS` keys spell out to `"__connection_id__` or `"__uuid__` — and `"__t` is not rescuable, so the micros envelope short-circuits too. A `TransactionUpdate` still hits, correctly: its `__connection_id__` is a value the scan exists to rescue. Because that condition is now keyed on the *initial*, a future `BIG_INT_FIELDS` key starting with another letter would be silently short-circuited. Noted at the table itself, since that is where someone would add one. ## `timed_calls` was reporting double Finding 1, taken verbatim: `local timed_calls = (rows + 1) + quote_calls`. Confirmed on the hand-written path — `apparatus=1.49ms` → **0.71ms**, matching the 0.74ms you measured on the typed path for the same 10001 timed calls. The comment claiming "one timed quoting scan per row" and "the exact size of the bias against the hand-written schema" is rewritten, since post-guard that bias is zero. ## `guard_hits ~= 0` added Finding 5 is right that `quote_calls == guard_hits` is `0 == 0` and cannot see a guard broadened until it hits everywhere. Added alongside, mirroring the wide-int guard four lines below. Both assertions kept: the equality catches a *swallowed* hit (a rescue silently skipped), the zero-check catches a *restored* scan. Passes on both tiers. ## `__uuid__` was genuinely uncovered Finding 4, taken verbatim. Verified it discriminates — mutating the guard to `find('"__connection_id__')`: ``` FAIL dev/test/test_protocol.lua: FAIL the guard does not defeat the uuid rescue either: expected ffffffff-ffff-ffff-ffff-ffffffffffff, got nil 6 passed, 1 failed ``` Finding 6 accepted as stated: the `PLAIN` assertion documents the contract rather than testing it, and the comment now says so instead of claiming it covers over-tightening. I also dropped two assertions I had duplicated from the pre-existing block 30 lines above (micros-stay-numbers, escaped-key). ## README: the construction row was the guard's own cost Finding 2 was correct and the fix needed a measurement, not a rewording. Paired `main`-vs-branch runs at `STDB_BULK_RUNS=5`, back to back in one session: ``` main frame med=30.1ms decode=13.8 quote=12.0 build=3.9 apparatus=1.49 main frame med=30.0ms decode=14.4 quote=12.1 build=3.8 apparatus=1.48 branch frame med=20.2ms decode=12.4 quote=0.0 build=7.1 apparatus=0.71 branch frame med=21.5ms decode=12.0 quote=0.0 build=8.9 apparatus=0.72 ``` `build` 3.85 → 8.0ms with no construction code touched, exactly as you said. Microbenchmarked the two contributors at 10000 rows on a row of this table's shape: ``` raw find, the guard as shipped 2.18 ms same, through the harness find wrapper 3.23 ms the old unguarded gsub, now skipped 6.19 ms ``` So construction is published as `main`'s 3.9ms, and the table gained an explicit `unbilled: the guard, and the harness's own wrapper | ~3.3ms` row rather than hiding ~3.3ms inside a bucket labelled construction. The codec bound moves to ~2.4x (from the ~3x I had, which was computed off the inflated residual). Finding 7: arithmetic fixed and re-measured rather than just recomputed — wasm 1000 rows is 22.0ms pooled over six samples, so 17ms is 77% of it. The wasm column is also corrected against a fresh `STDB_BULK_RUNS=3` run (`decode` 28 → 23.5ms, `build` 17.5 → 19.5ms, bound ~1.4x → ~1.3x). Native crossings re-swept with the final code, pooled: 8000 → 15.8ms, 8500 → 16.9, 9000 → 17.7, 45000 → 107.7. So 16.7ms at ~8500 rows and 100ms at ~42000, both stated to one significant figure now, with the observed 15-25% spread quoted beside them. The typed penalty lands at ~50% against the paired 20.9ms baseline. Finding 8: taken — full lines below, `apparatus` included, since that field is what exposed finding 1. ``` main BULK SUMMARY rows=10000 runs=5 frame min=29.6 med=30.1 max=33.8ms spread=14% med pre=0.5 decode=13.8 quote=12.0 build=3.9 post=0.1ms share pre=2% dec=46% quo=40% bld=13% post=0% us_per_row=3.01 quant=+/-0.11ms apparatus=1.49ms branch BULK SUMMARY rows=10000 runs=5 frame min=18.6 med=20.2 max=21.9ms spread=16% med pre=0.5 decode=12.4 quote=0.0 build=7.1 post=0.1ms share pre=2% dec=61% quo=0% bld=35% post=1% us_per_row=2.02 quant=+/-0.11ms apparatus=0.71ms ``` Suite `7 passed, 0 failed`, `stylua --check` clean, and both conformance harnesses pass on the final code — native all six phases, HTML5 green with `quo=0%` and the new `guard_hits` assertion holding on the wasm tier too.
yuris merged commit a6c2c663fc into main 2026-08-20 17:29:33 +00:00
Sign in to join this conversation.
No description provided.