Skip to content

fix(hir): keep large JSON defines as serialized data - #10161

Closed
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/10151-large-json-define
Closed

proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/10151-large-json-define

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix multi-MB JSON defines that never finish LLVM code generation, while preserving ordinary lowering and static property layouts for mid-size records (Option 2 from the review).

The original 4,623,800-byte OpenCode snapshot expanded into 395 synthetic classes and 772,701,328 bytes of saved LLVM IR across 30 units. HIR-to-LLVM emission took roughly 11 seconds; the last LLVM unit did not finish in 600 seconds. Its entry function reached 8,445,662 instructions, with peak RSS of 13,667,064 KiB. The stall is in LLVM after literal expansion, not an AST-to-HIR lowering loop.

Changes

  • Flat arrays of primitives (numbers, strings, booleans, null): compact lowering at 1,024 value nodes or 64 KiB of string content.
  • Other JSON-compatible object/array literals: compact lowering at 24,576 value nodes or 1 MiB of key/string content. Mid-size records retain ordinary lowering and static shapes.
  • Thresholds count AST values and UTF-8 content, independently of function instruction budgets. A nested record/array anywhere in a candidate prevents admission to the lower tier.
  • Keep Expr::JsonParse(Expr::String(...)) at the original evaluation site, matching the JSON-import intrinsic from perf(compile): reduce generated bundle bloat #8418. Each evaluation creates a fresh value; untaken branches do not parse. Existing define substitution, typeof folding, and both define-dependent cache keys are unchanged.
  • Preserve key order, duplicate last-write behavior, escaping and negative zero. JavaScript-specific semantics (prototype setters, holes, spreads, effects, unsupported strings, non-finite values) retain ordinary lowering.
  • Add coverage for both threshold tiers, direct lowering of 400 typed records and a 64 KiB record, plus a reproducible benchmark generator. Update CLI documentation and the issue's changelog fragment.

The record-array cutoff was measured using the exact regressing shape (id, name, tags, w), with a freshly built ordinary-lowering control equivalent to base 8a058e2053:

Records Literal bytes Value nodes Ordinary no-link compile Compact no-link compile
4,800 307,740 33,601 475.17 s 0.35 s
6,400 412,540 44,801 >600 s, timeout 0.37 s

At 6,400 records, emission completed in 4.5 seconds with approximately 85.1 MiB estimated IR and a 1,446,386-instruction function before optimization; four of five LLVM units completed within a second. The remaining unit timed out. The selected node threshold switches this shape at 3,511 records, 27% below the eight-minute case and 45% below the timeout. It is 24 times the old node threshold; the text threshold is 16 times larger.

Related issue

Fixes #10151

Test plan

All builds and tests ran through ./remote.sh on the Linux host with LLVM 22.1.8. None ran on the Mac. The compiler and matching runtime/stdlib archives were copied together out of the shared Cargo target. The ordinary control was built with the compact hook removed; the final compiler uses the code in this revision. Both link the same runtime/stdlib sources and archives.

Final build/check commands (inside ./remote.sh):

export PERRY_BUILD_COMMIT=62bc84338176825534
cargo build --release -p perry -p perry-runtime-static -p perry-stdlib-static
mkdir -p /tmp/def/two-tier/final-bin
cp "$CARGO_TARGET_DIR/release/perry" "$CARGO_TARGET_DIR/release/libperry_runtime.a" "$CARGO_TARGET_DIR/release/libperry_stdlib.a" /tmp/def/two-tier/final-bin/
export PERRY_BIN=/tmp/def/two-tier/final-bin/perry
export PERRY_RUNTIME_DIR=/tmp/def/two-tier/final-bin
cargo test -p perry-hir --lib
cargo test --release -p perry --test issue_10151_large_json_define -- --nocapture
cargo test -p perry-hir --test shape_inference nested_object_literal_lowers_in_linear_time -- --nocapture
cargo fmt --all -- --check
./scripts/check_file_size.sh
python3 scripts/check_test_registration.py
python3 scripts/check_node_version_consistency.py --list
  • Release build succeeded (existing runtime dead-code and Redis future-compatibility warnings).
  • HIR: 411 passed, 1 ignored, including all ten JSON-literal unit tests (2.35 s).
  • Compile/run regressions: 4 passed (6.32 s), including >1 MiB data, cache invalidation, CLI undefined override, small/mid-size direct lowering, fresh nested values, shadowed JSON, array mutations, negative zero, duplicate keys and escaping.
  • Deep-literal regression passed (1.46 s). Formatting, file-size policy, registration and Node-version consistency passed.

Reproduction commands, with each snapshot stored as the JS-expression value of BIG in its directory's perry.json:

cd /tmp/def/two-tier/original-snapshot
/usr/bin/time -v timeout 60 "$PERRY_BIN" compile main.ts --output main-final --no-auto-optimize --cache-dir cache-final
./main-final
cd /tmp/def/two-tier/snapshot
/usr/bin/time -v timeout 60 "$PERRY_BIN" compile main.ts --output main-final --no-auto-optimize --cache-dir cache-final
./main-final

The preserved 4,623,800-byte snapshot compiles and links in 1.52 s, printing 213. The installed snapshot has since refreshed to 4,637,074 bytes (still 213 providers); it compiles and links in 1.54 s, also printing 213. Both use fresh caches. Peak RSS is about 410 MiB.

Real OpenCode verification keeps the installed perry.json unchanged and uses a temporary entry in packages/opencode importing ../core/src/models-dev:

cd "$OPENCODE_SRC/packages/opencode"
/usr/bin/time -v timeout 600 env PERRY_DISABLE_BUILD_CACHE=1 PERRY_MODULE_JOBS=8 PERRY_DISABLE_WELL_KNOWN=1 PERRY_CODEGEN_PROGRESS=all "$PERRY_BIN" compile perry-10151-models.ts --platform bun --no-link --no-auto-optimize --output /tmp/def/two-tier/oc-final/main.o --cache-dir /tmp/def/oc-models-cache

The final compiler regenerated packages/core/src/models-dev.ts in 10.6 s (its matching object was held out of the cache to force codegen). The surrounding compile timed out at 600 s later in unrelated Effect dependency codegen. Resuming with the same final compiler and object cache completed all 621 native modules, exit 0, in 60.42 s:

/usr/bin/time -v timeout 900 env -u PERRY_NO_CACHE -u PERRY_DISABLE_BUILD_CACHE PERRY_MODULE_JOBS=8 PERRY_DISABLE_WELL_KNOWN=1 PERRY_CODEGEN_PROGRESS=1 "$PERRY_BIN" compile perry-10151-models.ts --platform bun --no-link --no-auto-optimize --output /tmp/def/two-tier/oc-final/main.o --cache-dir /tmp/def/oc-models-cache

An additional ordinary-lowering 3,200-record probe exceeded its 180-second diagnostic timeout on the busy host. The selected threshold is a data-size heuristic below the measured 4,800/6,400-record cliff, not a universal compilation deadline for all smaller programs.

The no-link pipeline still resolves linker support archives; PERRY_DISABLE_WELL_KNOWN=1 avoids building those unrelated archives. The temporary entry is removed afterward. The full 7,800-module application was not built.

Runtime A/B uses benchmarks/large_json_literals/generate.py, the exact audit fixture, separate caches, and five interleaved runs. Commands and cutoff reproduction are documented in that directory's README.

Hot loop Ordinary control Final two-tier rule
Separate 2,000-number array 71 ms 71 ms
Separate 400 typed records 295 ms 294 ms
Combined audit file: numbers 290 ms 291 ms
Combined audit file: records 292 ms 291 ms

Performance limitation: the old broad rule measured 71 ms / 938 ms in the combined file. This revision restores record-read performance, but does not retain that combined file's numeric speedup. HIR confirms the numeric array still takes JsonParse; ordinary record initialization makes the same unit exceed LLVM's existing 100k-instruction O0 machine-code limit (about 622k instructions). The independent number probe shows no intrinsic runtime improvement on this host. Changing initialization/unit splitting would be additional codegen work; this revision keeps the requested two-tier scope. This limitation is reported explicitly for re-audit.

All numeric and record checksums match. The original semantic probe (mutations, key order, fresh arrays, Float64Array and Map) produced identical stdout across the ordinary control, old broad rule and revised rule. The combined file takes 58.90 s to compile under the final rule versus 60.07 s for ordinary lowering; mid-size record codegen is intentionally retained.

Checklist

  • Added regression coverage for both tiers and mid-size static shapes.
  • Measured the record-array LLVM cutoff with margin below it.
  • Verified both original and current installed OpenCode defines.
  • Ran the requested remote build, tests and formatting/file-size checks.
  • Documented the A/B results, including the numeric-performance limitation.
  • Updated CLI docs and changelog fragment.
  • No version bump or edits to CLAUDE.md / CHANGELOG.md.
  • Read CONTRIBUTING.md and follow its PR conventions.

Summary by CodeRabbit

  • Performance

    • Large JSON-compatible literals and build-time defines are processed more efficiently.
    • Flat primitive arrays use lower size thresholds, while larger objects and nested arrays use higher thresholds to preserve efficient property access for mid-sized records.
  • Bug Fixes

    • Repeated evaluations continue to produce fresh values, with unused branches remaining unevaluated.
    • Property order, escaping, numeric edge cases, and nested JSON data are preserved.
    • JavaScript-specific constructs retain standard expression behavior.
  • Documentation

    • Expanded guidance on thresholds and supported JSON-compatible literals.

@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: a5dfbacf-447a-45e1-b716-0acbb5c20be8

📥 Commits

Reviewing files that changed from the base of the PR and between d0bb9b1 and 699ae8f.

📒 Files selected for processing (7)
  • benchmarks/large_json_literals/README.md
  • benchmarks/large_json_literals/generate.py
  • changelog.d/10151-large-json-define.md
  • crates/perry-hir/src/lower/lower_expr/json_literal.rs
  • crates/perry-hir/src/lower/lower_expr/json_literal/tests.rs
  • crates/perry/tests/issue_10151_large_json_define.rs
  • docs/src/cli/flags.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • changelog.d/10151-large-json-define.md
  • docs/src/cli/flags.md

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Large JSON-compatible object and array literals now use serialized JSON parsing at evaluation time when they exceed shape-specific thresholds. Unsupported JavaScript semantics retain ordinary lowering. Tests, documentation, and benchmarks cover the new behavior.

Changes

Large JSON literal lowering

Layer / File(s) Summary
Detection, serialization, and lowering
crates/perry-hir/src/lower/lower_expr.rs, crates/perry-hir/src/lower/lower_expr/json_literal.rs
Flat primitive arrays use the lower threshold. Other object and array literals use the higher threshold. Supported literals emit Expr::JsonParse(Expr::String(...)); unsupported constructs use ordinary lowering.
Lowering behavior tests
crates/perry-hir/src/lower/lower_expr/json_literal/tests.rs
Tests cover both thresholds, static-shape preservation, serialization fidelity, JavaScript-specific fallbacks, depth limits, and typeof folding.
Define integration and runtime validation
crates/perry/tests/issue_10151_large_json_define.rs
Integration tests cover compilation, cache invalidation, CLI precedence, fresh values, mutation isolation, primitive arrays, numeric edge cases, and source-literal serialization.
Threshold documentation and benchmarks
docs/src/cli/flags.md, changelog.d/10151-large-json-define.md, benchmarks/large_json_literals/*
Documentation describes the two threshold tiers. Benchmark tooling and results cover compile-time and runtime behavior for numeric arrays and typed records.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Compiler
  participant json_literal
  participant GeneratedCode
  participant JSONParser
  Compiler->>json_literal: inspect object or array literal
  json_literal->>GeneratedCode: emit serialized string and JsonParse expression
  GeneratedCode->>JSONParser: parse serialized value at evaluation site
  JSONParser-->>GeneratedCode: return fresh object or array
Loading

Merge Risk: ⚪ Minimal · up to 699ae

No unresolved issue remains that should block merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 5 files. (3 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #10151 requires large JSON defines to avoid massive IR, preserve embedded data behavior, preserve define cache correctness, and include a regression above 1 MiB. json_literal.rs lowers support…
Out of Scope Changes check ✅ Passed The changes are limited to large JSON literal lowering, related HIR and integration tests, documentation, changelog text, and a benchmark generator and README. These changes support Issue #10151 by do…
Title check ✅ Passed The title is concise, specific, and accurately summarizes the main HIR change: lowering large JSON defines as serialized data.
Description check ✅ Passed The description includes the required Summary, Changes, Related issue, Test plan, and Checklist sections. It provides detailed implementation context, test commands, results, benchmarks, and confirms …
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 5 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Audit of head d0bb9b1fd5 on macOS: correct, but holding for a maintainer decision on a compute trade-off.

Validation (train branch = this head + version bump, cherry-picked onto 8a058e2053):

  • cargo test --release -p perry-hir: 657 passed. -p perry-codegen: 1984 passed. --test issue_10151_large_json_define: 4 passed (after building the -static wrappers).
  • fmt, file size, test registration, RUSTFLAGS=-Dwarnings cargo check -p perry --bins: pass.
  • Semantic probe: a 2000-element typed number[] (hot sum, index write, push, indexOf), 400 typed nested records including -0, fresh arrays per call, and Float64Array/Map built from large literals. Output is byte-identical to Node with this head and with main's compiler.

The scope matters here. The switch is in lower_expr_impl, so it applies to every object/array literal of ≥1024 nodes or ≥64 KiB, not just defines. Nothing in benchmarks/ or test-files/ is that large, so CI cannot show the effect. Measured A/B on the same machine, 5 interleaved rounds, main compiler/archives at 8a058e2053 vs this head:

same source main this PR
compile ~2 min ~1 s
binary 37.4 MB 14.2 MB
hot loop over a 2000-element number[] literal (20k passes) ~452 ms ~166 ms (2.7× faster)
hot property reads over a 400-record typed literal (q.w + q.tags.length + q.id, 20k passes) ~428 ms ~1063 ms (2.5× slower)
peak RSS 35 MB 32 MB

The record slowdown is not new code. On main, the same loop over JSON.parse of the identical text already takes ~1080 ms. So this PR routes large record literals from the fast literal-shape path onto the existing JSON.parse object representation, whose property reads are ~2.5× slower.

Options as I see them:

  1. Land as-is, and file the JSON.parse record property-access gap separately. It also affects JSON imports (perf(compile): reduce generated bundle bloat #8418) and every runtime JSON workload.
  2. Keep the compact path for arrays of primitives (a pure win above) and for literals above the size where LLVM actually stalls, while leaving mid-sized record literals on ordinary lowering until JSON.parse records read at literal-shape speed.

Given the standing "keep best compute" rule, I'm not landing a 2.5× hot-path regression unilaterally. Tell me which option to take, or push a revision, and I'll land it.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Implemented Option 2 in 699ae8f.

  • Flat primitive arrays: compact at 1,024 value nodes or 64 KiB of string content.
  • Other JSON-compatible literals, including records: compact at 24,576 nodes or 1 MiB of key/string content. Mid-size records keep ordinary lowering and static property layouts.

Cutoff calibration used the audit's exact id/name/tags/w record-array shape. Ordinary lowering took 475.17 s at 4,800 records (33,601 nodes / 307,740 source bytes), and timed out at 600 s at 6,400 records (44,801 nodes / 412,540 bytes). At 6,400, emission finished in 4.5 s with about 85.1 MiB estimated IR; LLVM stalled on the remaining unit. The new rule compiles these in 0.35 s / 0.37 s. It switches this shape at 3,511 records, 27% below the eight-minute case and 45% below the timeout; the general node/text thresholds are 24× / 16× the primitive thresholds.

Five interleaved Linux A/B runs with the final compiler and matching archives:

Hot loop Ordinary/main control Revised
Separate 2,000-number array 71 ms 71 ms
Separate 400 typed records 295 ms 294 ms
Combined audit file: numbers 290 ms 291 ms
Combined audit file: records 292 ms 291 ms

Re-audit caveat: the requested combined-file numeric speedup is not retained. The old broad rule measured 71 ms / 938 ms in that file. The revision fixes the record-read regression, but ordinary record initialization pushes the same unit over LLVM's existing 100k-instruction O0 machine-code limit (~622k instructions), slowing the numeric loop too. HIR confirms that its numeric array still uses JsonParse. Separate number-only entrypoints show no intrinsic runtime gain on this host. I kept this revision focused on the two-tier rule and documented this limitation rather than claiming both old A/B wins; initialization/unit splitting would be additional codegen work.

The preserved 4,623,800-byte / 213-provider OpenCode define compiles and links in 1.52 s, printing 213. The installed snapshot has refreshed to 4,637,074 bytes, still 213 providers; it takes 1.54 s and also prints 213. The actual packages/core/src/models-dev.ts regenerated in 10.6 s with the installed perry.json. The surrounding Effect graph hit the 600 s diagnostic limit; resuming the cache completed 621/621 native modules, exit 0, in 60.42 s.

All via ./remote.sh: 411 HIR tests passed, 1 ignored; 4 compile/run regressions passed; the 6,000-level deep-literal test passed; cargo fmt, file-size policy, test registration and Node-version consistency passed. Tests, CLI docs and changelog now describe both tiers. The PR description contains the commands and results; benchmarks/large_json_literals/ contains the reproducible generator and measurement notes. No versions or CHANGELOG.md changed.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed via merge train #10172 (rebase-merged; main 9b911855f8, tree identical to the train). Head 699ae8f665 was cherry-picked patch-identical onto b5a82cfeae, plus one README commit that replaces the lane-internal "do not build or run the probes on the Mac" sentence with host-neutral A/B guidance, and the version bump to 0.5.1547. The macOS re-audit is in #10172: the 400-record property loop is back at main's speed (432–444 ms vs 432–442 ms), records-400 stays on ordinary lowering, and records-6400 compiles compact in 0.94 s. The CI attribution is there too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant