ReviewOS

zig-utils/zig-js

Frontend: make strict parameter validation linear

#520
Closed chris opened this 9 days ago ·
9 days ago

Parent: #496 Related: #493, #460, #459

Defect

Parser.validateStrictParams compares each simple formal parameter with every earlier formal parameter. A valid strict function with n unique parameters therefore performs Θ(n²) name comparisons even though the parser already uses hash-indexed UniqueFormalParameters validation for arrows, methods, generators, async functions, and non-simple lists.

This is a user-controlled frontend latency surface. Long unique lists are valid JavaScript and must not be rejected or special-cased.

Required change

  • Add a deterministic representative/growth workload that isolates strict-parameter parsing.
  • Make strict duplicate detection expected-linear while preserving reserved-name, eval/arguments, destructuring, sloppy duplicate, generator/async, arrow, method, and abrupt syntax behavior.
  • Keep all state parser-invocation-local and allocation failure explicit.
  • Measure exact-parent CPU work and memory; retain only a demonstrated improvement.
  • Run focused frontend tests and the relevant test262 parser/function subtrees with no regressions.

No-workaround rules

No parameter-count blacklist, reduced syntax support, skipped duplicate checks, probabilistic equality, global mutable cache, or benchmark-only fast path.

9 days ago

Completed on main:\n\n- representative growth rows\n- parser-only growth runner\n- expected-linear strict-name index\n- freeable scratch lifetime\n- no-GIL and TSan scratch witness\n- validated exact-parent evidence\n\nExact-parent retired instructions are 0.2229x, 0.1531x, and 0.0733x at 1,024, 2,048, and 4,096 unique strict parameters. The stable 4,096 full eval parse-and-compile row improves from 12.288 ms to 1.554 ms (0.126x). The two smaller wall rows are intentionally diagnostic because their relative noise exceeds the publication threshold.\n\nMeasurement exposed that arena-backed hash buckets survived persistent-context warmups. The follow-up allocator correction retains arena ownership for tokens, AST nodes, and borrowed names, but allocates/deinitializes the non-owning index through Context scratch backing. It reduces median peak RSS from 52,150,272 to 48,758,784 bytes while retaining 0.9968x instructions versus the indexed parent. Energy remained below useful resolution and cache-miss attribution was unavailable, so no claims are made for either.\n\nStrict reserved-name and eval/arguments rejection, late duplicate rejection, sloppy simple-list duplicates, allocation failure, concurrent no-GIL parsing, and TSan are covered. The exact parent and final candidate both pass 1,275/1,275 function/method test262 cases (0 flips). Full unit validation is 1,546 passed / 1 expected skip / 0 failures or leaks.

9 days ago

Child #611 is complete on main: benchmark contract 62a340ef, runtime fix bd24f7f2.

Functions with no direct LexicallyDeclaredNames now skip an impossible attacker-sized parameter/body conflict index. Exact-parent instructions improve to 0.8035x ASCII, 0.8462x Unicode, 0.8568x escaped, and 0.8031x nested-shadow; 11 allocation requests and 278,656 bytes disappear in the ordinary 4K rows. The direct lexical slow-path control stays exact in allocations/bytes and 1.0013x instructions. Test262 remains 2,526 -> 2,526; full units pass 1,728 plus one expected skip.

Sign in to comment on this issue.