fix(json): parse deep nesting with a heap-backed stack - #7832
Merged
Conversation
📝 WalkthroughWalkthroughJSON parsing now switches from recursive parsing to heap-backed iterative parsing for deeply nested documents. It validates strings, numbers, and document boundaries, supports up to 500,000 nested levels, and reports malformed input or excessive depth with distinct errors. ChangesDeep JSON parsing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant JSONParse
participant try_parse_deep_iterative
participant JsonTape
participant materialize_iterative
JSONParse->>try_parse_deep_iterative: route deeply nested input
try_parse_deep_iterative->>JsonTape: build validated tape
JsonTape-->>try_parse_deep_iterative: validated tape
try_parse_deep_iterative->>materialize_iterative: materialize with heap-backed frames
materialize_iterative-->>JSONParse: root JSValue or RangeError
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Author
|
Quiet-M1 ordinary-payload performance check completed against base 1ee158d and PR head 642f653. Methodology:
Median wall-time results:
Ordinary recursive, lazy, roundtrip, and typed paths are all effectively flat. Marking ready for review. |
proggeramlug
marked this pull request as ready for review
August 11, 2026 11:29
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
crates/perry-runtime/src/json_tape_tests.rs (1)
118-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe test name promises array coverage, but the duplicate key discards the array.
The input is
{"a":[1,true,"x"],"a":{"b":2}}. The second"a"overwrites the first. The only assertion readsnested.b. The array[1,true,"x"]is materialized and then thrown away, so no assertion coversBuildFrame::Array,js_array_push, or theKIND_TRUEandKIND_STRINGarms.Add a distinct key for the array so all three arms are asserted.
♻️ Proposed test input and assertions
- let input = br#"{"a":[1,true,"x"],"a":{"b":2}}"#; + let input = br#"{"arr":[1,true,"x"],"a":{"b":2},"a":{"b":2}}"#;Then assert the array contents alongside the existing duplicate-key check:
let key_arr = crate::string::js_string_from_bytes(b"arr".as_ptr(), 3); let arr = crate::object::js_object_get_field_by_name(object, key_arr); let arr = (arr.bits() & crate::value::POINTER_MASK) as *mut crate::ArrayHeader; assert_eq!(unsafe { (*arr).length }, 3); assert_eq!( f64::from_bits(crate::array::js_array_get(arr, 0).bits()), 1.0 );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/json_tape_tests.rs` around lines 118 - 124, Update iterative_materializer_preserves_nested_objects_arrays_and_duplicate_keys so the input stores the array under a distinct key such as arr while retaining the duplicate a keys for overwrite coverage. Add assertions retrieving arr and verifying its length and materialized contents, including the numeric, true, and string elements, so BuildFrame::Array, js_array_push, KIND_TRUE, and KIND_STRING are exercised.changelog.d/7832-iterative-json-parse.md (1)
1-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the history and validation notes the fragment policy requires.
The coding guidelines state: "Add a PR-keyed changeset fragment without a version header for each change landing on
main; put detailed history and validation notes in the fragment."The filename is PR-keyed and there is no version header. Both are correct. The fragment carries no history and no validation notes.
The PR reports concrete validation that belongs here: a 300,000-level document parsed on a 2 MiB worker stack, and 12 alternating A/B pairs per benchmark with matching checksums showing
json_parse_1mbat +0.368%,bench_json_readonlyat +0.052%,bench_json_roundtripat +0.272%, andbench_json_typed_roundtripat −0.034%.Record the linked issue (
#7817) and these results. Keep the entry as one coherent description of the shipped behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@changelog.d/7832-iterative-json-parse.md` around lines 1 - 4, Update the changelog fragment’s single description to include issue `#7817`, the 300,000-level document validation on a 2 MiB worker stack, and the 12 alternating A/B benchmark pairs with matching checksums and all four reported performance results. Preserve the existing shipped-behavior summary, filename, and absence of a version header.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-runtime/src/json_tape_tests.rs`:
- Around line 128-133: Reorder the setup in the test so both
js_string_from_bytes calls for key_a and key_b occur before deriving the raw
object pointer from value. Keep value rooted as currently, then compute object
and perform the nested field lookups only after both potentially collecting
allocations have completed.
In `@crates/perry-runtime/src/json_tape/iterative.rs`:
- Around line 67-141: Update materialize_iterative in
crates/perry-runtime/src/json_tape/iterative.rs (lines 67-141) to root every
container completed by finish_frame in PARSE_ROOTS, then re-read the rooted
pointer before attaching it to its parent so frames never retain stale pointers
across allocations. In crates/perry-runtime/src/json/parse_api.rs (lines
130-175), verify gc_suppress prevents all collection and evacuation, including
emergency reclaim, throughout materialization; if not, root the deep path
intermediates there as well.
In `@crates/perry-runtime/src/json/mod.rs`:
- Around line 888-911: Initialize the thread-local GC at the start of the
closure in parses_three_hundred_thousand_levels_on_a_small_worker_stack by
calling crate::gc::ensure_gc_initialized() before nested_arrays,
js_string_from_bytes, or js_json_parse_result perform runtime allocation. Keep
the existing parsing and validation flow unchanged.
---
Nitpick comments:
In `@changelog.d/7832-iterative-json-parse.md`:
- Around line 1-4: Update the changelog fragment’s single description to include
issue `#7817`, the 300,000-level document validation on a 2 MiB worker stack, and
the 12 alternating A/B benchmark pairs with matching checksums and all four
reported performance results. Preserve the existing shipped-behavior summary,
filename, and absence of a version header.
In `@crates/perry-runtime/src/json_tape_tests.rs`:
- Around line 118-124: Update
iterative_materializer_preserves_nested_objects_arrays_and_duplicate_keys so the
input stores the array under a distinct key such as arr while retaining the
duplicate a keys for overwrite coverage. Add assertions retrieving arr and
verifying its length and materialized contents, including the numeric, true, and
string elements, so BuildFrame::Array, js_array_push, KIND_TRUE, and KIND_STRING
are exercised.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f5acadda-2afc-4d57-8647-f580a8773828
📒 Files selected for processing (7)
changelog.d/7832-iterative-json-parse.mdcrates/perry-runtime/src/json/mod.rscrates/perry-runtime/src/json/parse_api.rscrates/perry-runtime/src/json/parser.rscrates/perry-runtime/src/json_tape.rscrates/perry-runtime/src/json_tape/iterative.rscrates/perry-runtime/src/json_tape_tests.rs
Comment on lines
+128
to
+133
| let object = (value.bits() & crate::value::POINTER_MASK) as *const crate::ObjectHeader; | ||
| let key_a = crate::string::js_string_from_bytes(b"a".as_ptr(), 1); | ||
| let key_b = crate::string::js_string_from_bytes(b"b".as_ptr(), 1); | ||
| let nested = crate::object::js_object_get_field_by_name(object, key_a); | ||
| let nested = (nested.bits() & crate::value::POINTER_MASK) as *const crate::ObjectHeader; | ||
| let b = crate::object::js_object_get_field_by_name(nested, key_b); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
object can go stale across the two string allocations.
Line 128 derives the raw object pointer from the local value. Lines 129 and 130 then call js_string_from_bytes twice, after gc_unsuppress() on line 126. Each call allocates and can collect. Line 131 dereferences object.
value is rooted at line 125, so the collector rewrites the root slot. It does not rewrite the local value or the derived object pointer. A retrieved learning states that Rust locals are not GC roots and raw pointer locals are not reliable pins.
Create both keys before deriving object.
🛡️ Proposed reordering
- let object = (value.bits() & crate::value::POINTER_MASK) as *const crate::ObjectHeader;
let key_a = crate::string::js_string_from_bytes(b"a".as_ptr(), 1);
let key_b = crate::string::js_string_from_bytes(b"b".as_ptr(), 1);
+ let value = crate::json::parse_root_get(saved_roots);
+ let object = (value.bits() & crate::value::POINTER_MASK) as *const crate::ObjectHeader;
let nested = crate::object::js_object_get_field_by_name(object, key_a);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let object = (value.bits() & crate::value::POINTER_MASK) as *const crate::ObjectHeader; | |
| let key_a = crate::string::js_string_from_bytes(b"a".as_ptr(), 1); | |
| let key_b = crate::string::js_string_from_bytes(b"b".as_ptr(), 1); | |
| let nested = crate::object::js_object_get_field_by_name(object, key_a); | |
| let nested = (nested.bits() & crate::value::POINTER_MASK) as *const crate::ObjectHeader; | |
| let b = crate::object::js_object_get_field_by_name(nested, key_b); | |
| let key_a = crate::string::js_string_from_bytes(b"a".as_ptr(), 1); | |
| let key_b = crate::string::js_string_from_bytes(b"b".as_ptr(), 1); | |
| let value = crate::json::parse_root_get(saved_roots); | |
| let object = (value.bits() & crate::value::POINTER_MASK) as *const crate::ObjectHeader; | |
| let nested = crate::object::js_object_get_field_by_name(object, key_a); | |
| let nested = (nested.bits() & crate::value::POINTER_MASK) as *const crate::ObjectHeader; | |
| let b = crate::object::js_object_get_field_by_name(nested, key_b); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/json_tape_tests.rs` around lines 128 - 133, Reorder
the setup in the test so both js_string_from_bytes calls for key_a and key_b
occur before deriving the raw object pointer from value. Keep value rooted as
currently, then compute object and perform the nested field lookups only after
both potentially collecting allocations have completed.
Source: Learnings
Comment on lines
+67
to
+141
| /// Materialize a validated tape without consuming one native stack frame per | ||
| /// JSON container. Runtime GC must be suppressed by the caller: partially | ||
| /// built values live in this function's heap-backed work stack until their | ||
| /// parent container is complete. | ||
| pub(crate) unsafe fn materialize_iterative(tape: &[TapeEntry], bytes: &[u8]) -> Option<JSValue> { | ||
| let source = TapeSource::Borrowed { tape, bytes }; | ||
| let mut frames = Vec::new(); | ||
| let mut root = None; | ||
|
|
||
| for entry in tape.iter().copied() { | ||
| match entry.kind { | ||
| KIND_OBJ_START => frames.push(BuildFrame::Object { | ||
| keys: Vec::new(), | ||
| values: Vec::new(), | ||
| }), | ||
| KIND_ARR_START => frames.push(BuildFrame::Array(Vec::new())), | ||
| KIND_KEY => { | ||
| let Some(BuildFrame::Object { keys, values }) = frames.last_mut() else { | ||
| return None; | ||
| }; | ||
| if keys.len() != values.len() { | ||
| return None; | ||
| } | ||
| let key = decode_key_to_interned_string(&source, entry.offset as usize); | ||
| if key.is_null() { | ||
| return None; | ||
| } | ||
| keys.push(key); | ||
| } | ||
| KIND_STRING => { | ||
| let value = materialize_string_value(&source, entry.offset as usize); | ||
| if !attach_value(&mut frames, &mut root, value) { | ||
| return None; | ||
| } | ||
| } | ||
| KIND_NUMBER => { | ||
| let value = materialize_number(&source, entry.offset as usize); | ||
| if !attach_value(&mut frames, &mut root, value) { | ||
| return None; | ||
| } | ||
| } | ||
| KIND_TRUE | KIND_FALSE | KIND_NULL => { | ||
| let value = match entry.kind { | ||
| KIND_TRUE => JSValue::bool(true), | ||
| KIND_FALSE => JSValue::bool(false), | ||
| _ => JSValue::null(), | ||
| }; | ||
| if !attach_value(&mut frames, &mut root, value) { | ||
| return None; | ||
| } | ||
| } | ||
| KIND_OBJ_END | KIND_ARR_END => { | ||
| let frame = frames.pop()?; | ||
| if !matches!( | ||
| (&frame, entry.kind), | ||
| (BuildFrame::Object { .. }, KIND_OBJ_END) | ||
| | (BuildFrame::Array(_), KIND_ARR_END) | ||
| ) { | ||
| return None; | ||
| } | ||
| let value = finish_frame(frame)?; | ||
| if !attach_value(&mut frames, &mut root, value) { | ||
| return None; | ||
| } | ||
| } | ||
| _ => return None, | ||
| } | ||
| } | ||
|
|
||
| if frames.is_empty() { | ||
| root | ||
| } else { | ||
| None | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
The deep parse path protects unrooted intermediate containers with GC suppression alone. materialize_iterative holds every pending container in plain Rust Vec frames that the collector does not scan, and try_parse_deep_iterative wraps that call in gc_suppress/gc_unsuppress without pushing any intermediate into PARSE_ROOTS. The recursive path does both: DirectParser::parse_array and DirectParser::parse_object_untyped suppress GC and root each intermediate, then re-read the pointer from the root slot after every allocation. At MAX_ITERATIVE_NESTING_DEPTH, up to 500,000 container pointers are live and invisible to the collector at once.
crates/perry-runtime/src/json_tape/iterative.rs#L67-L141: push each container completed byfinish_frameintoPARSE_ROOTSand re-read it from the root slot before attaching it to its parent, so an evacuation duringjs_array_alloc,js_array_push,js_object_alloc, orjs_object_set_field_by_namecannot leave stale pointers inframes.crates/perry-runtime/src/json/parse_api.rs#L130-L175: confirm that suppression blocks every collection and evacuation path, including emergency reclaim, for the whole materialization window. If it does not, this caller must root the deep path's intermediates the same way the recursive path does.
📍 Affects 2 files
crates/perry-runtime/src/json_tape/iterative.rs#L67-L141(this comment)crates/perry-runtime/src/json/parse_api.rs#L130-L175
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/json_tape/iterative.rs` around lines 67 - 141,
Update materialize_iterative in crates/perry-runtime/src/json_tape/iterative.rs
(lines 67-141) to root every container completed by finish_frame in PARSE_ROOTS,
then re-read the rooted pointer before attaching it to its parent so frames
never retain stale pointers across allocations. In
crates/perry-runtime/src/json/parse_api.rs (lines 130-175), verify gc_suppress
prevents all collection and evacuation, including emergency reclaim, throughout
materialization; if not, root the deep path intermediates there as well.
Source: Learnings
Comment on lines
+888
to
+911
| #[test] | ||
| fn parses_three_hundred_thousand_levels_on_a_small_worker_stack() { | ||
| const DEPTH: usize = 300_000; | ||
| std::thread::Builder::new() | ||
| .name("json-deep-worker".into()) | ||
| .stack_size(2 * 1024 * 1024) | ||
| .spawn(|| { | ||
| let input = nested_arrays(DEPTH, b'7'); | ||
| let text = js_string_from_bytes(input.as_ptr(), input.len() as u32); | ||
| let mut value = unsafe { js_json_parse_result(text) } | ||
| .expect("deep JSON must parse on a worker-sized stack"); | ||
|
|
||
| for level in 0..DEPTH { | ||
| assert!(value.is_pointer(), "level {level} must be an array"); | ||
| let array = (value.bits() & POINTER_MASK) as *const crate::ArrayHeader; | ||
| assert_eq!(unsafe { (*array).length }, 1, "level {level}"); | ||
| value = crate::array::js_array_get(array, 0); | ||
| } | ||
| assert_eq!(f64::from_bits(value.bits()), 7.0); | ||
| }) | ||
| .expect("worker thread starts") | ||
| .join() | ||
| .expect("worker parse does not panic"); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether perry-runtime supports allocation and GC on non-main threads.
set -euo pipefail
rg -nP -C 6 '\bfn\s+gc_init\s*\(' crates/perry-runtime/src
rg -nP -C 6 'main_thread|is_main_thread|runtime_main_thread' crates/perry-runtime/src --glob '*.rs'
# Do any other perry-runtime tests spawn threads and allocate?
rg -nP -C 6 'thread::Builder::new|std::thread::spawn' crates/perry-runtime/src --glob '*.rs'
# How are perry-runtime tests actually invoked?
rg -nP -C 3 'RUST_TEST_THREADS' --hiddenRepository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- gc initialization ---'
sed -n '620,735p' crates/perry-runtime/src/gc/mod.rs
sed -n '570,680p' crates/perry-runtime/src/gc/schedule.rs
echo '--- initialization and allocation call sites ---'
rg -n 'ensure_gc_initialized|gc_init\(|gc_check_trigger|gc_schedule|alloc|allocate' crates/perry-runtime/src/gc crates/perry-runtime/src/json/mod.rs --glob '*.rs' \
| head -n 240
echo '--- test and runtime thread usage ---'
rg -n 'thread::Builder::new|std::thread::spawn|spawn\(\|\||js_json_parse_result|js_string_from_bytes|js_array_get' crates/perry-runtime/src --glob '*.rs' \
| head -n 240
echo '--- test-thread configuration ---'
rg -n 'RUST_TEST_THREADS|cargo test.*perry-runtime|perry-runtime.*test' . --hidden \
-g '!target/**' -g '!node_modules/**' | head -n 160
echo '--- json test context ---'
sed -n '820,930p' crates/perry-runtime/src/json/mod.rsRepository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- gc initialization ---'
sed -n '620,735p' crates/perry-runtime/src/gc/mod.rs
sed -n '570,680p' crates/perry-runtime/src/gc/schedule.rs
echo '--- initialization and allocation call sites ---'
rg -n 'ensure_gc_initialized|gc_init\(|gc_check_trigger|gc_schedule|alloc|allocate' crates/perry-runtime/src/gc crates/perry-runtime/src/json/mod.rs --glob '*.rs' \
| head -n 240
echo '--- test and runtime thread usage ---'
rg -n 'thread::Builder::new|std::thread::spawn|js_json_parse_result|js_string_from_bytes|js_array_get' crates/perry-runtime/src --glob '*.rs' \
| head -n 240
echo '--- test-thread configuration ---'
rg -n 'RUST_TEST_THREADS|cargo test.*perry-runtime|perry-runtime.*test' . --hidden \
-g '!target/**' -g '!node_modules/**' | head -n 160
echo '--- json test context ---'
sed -n '820,930p' crates/perry-runtime/src/json/mod.rsRepository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- string allocation path ---'
rg -n -C 12 'pub extern .*js_string_from_bytes|pub fn js_string_from_bytes|fn js_string_from_bytes|ensure_gc_initialized' \
crates/perry-runtime/src/string.rs crates/perry-runtime/src/array.rs crates/perry-runtime/src/object.rs \
crates/perry-runtime/src/gc/mod.rs
echo '--- JSON parse entry and allocation helpers ---'
rg -n -C 16 'js_json_parse_result|parse_json|parse_value|js_array_alloc|js_array_alloc_with_length|gc_malloc' \
crates/perry-runtime/src/json/mod.rs --glob '*.rs' | head -n 320
echo '--- deep test and nearby setup ---'
rg -n -C 24 'parses_three_hundred_thousand_levels_on_a_small_worker_stack|nested_arrays|json-deep-worker' \
crates/perry-runtime/src/json/mod.rs
echo '--- spawned-thread tests that allocate runtime values ---'
rg -n -C 16 'thread::Builder::new|std::thread::spawn' crates/perry-runtime/src \
--glob '*.rs' \
-g '!CHANGELOG.md' \
| rg -n -B 16 -A 16 'test|js_string_from_bytes|js_array_alloc|gc_malloc|js_json_parse_result' \
| head -n 320
echo '--- runtime initialization documentation ---'
rg -n -C 8 'spawned worker|worker thread|every thread|current thread|thread-local scanner|allocation.*thread' \
crates/perry-runtime/src/gc crates/perry-runtime/src/string.rs crates/perry-runtime/src/json/mod.rs \
--glob '*.rs' | head -n 240Repository: PerryTS/perry
Length of output: 5743
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- module files ---'
fd -t f . crates/perry-runtime/src | rg '/(string|array|object)(/mod)?\.rs$|json/mod\.rs$'
echo '--- initialization call sites ---'
rg -n -C 10 'ensure_gc_initialized' crates/perry-runtime/src --glob '*.rs' \
-g '!CHANGELOG.md' | head -n 260
echo '--- string and array allocation definitions ---'
rg -n -C 18 'js_string_from_bytes|js_array_alloc_with_length|js_array_alloc\(' \
crates/perry-runtime/src/string crates/perry-runtime/src/array \
--glob '*.rs' | head -n 360
echo '--- JSON parse definitions and deep test ---'
rg -n -C 20 'js_json_parse_result|parses_three_hundred_thousand_levels_on_a_small_worker_stack|nested_arrays' \
crates/perry-runtime/src/json/mod.rs | head -n 300
echo '--- exact runtime test thread calls ---'
rg -n -C 14 'thread::Builder::new|std::thread::spawn' crates/perry-runtime/src \
--glob '*.rs' -g '!CHANGELOG.md' \
| head -n 360Repository: PerryTS/perry
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- string allocation implementation ---'
rg -n 'fn string_storage_alloc|fn js_string_from_bytes|pub .*js_string_from_bytes|gc_malloc|ensure_gc_initialized' \
crates/perry-runtime/src/string/alloc.rs crates/perry-runtime/src/string/mod.rs
sed -n '1,220p' crates/perry-runtime/src/string/alloc.rs
echo '--- array allocation implementation ---'
rg -n 'fn js_array_alloc|pub .*js_array_alloc|gc_malloc|ensure_gc_initialized' \
crates/perry-runtime/src/array/alloc.rs
sed -n '1,180p' crates/perry-runtime/src/array/alloc.rs
echo '--- JSON parse entry points ---'
rg -n 'js_json_parse_result|js_json_parse|parse_json|parse_value|js_get_global_this|ensure_gc_initialized' \
crates/perry-runtime/src/json/mod.rs
sed -n '2200,2340p' crates/perry-runtime/src/json/mod.rs
echo '--- deep test exact context ---'
rg -n 'parses_three_hundred_thousand_levels_on_a_small_worker_stack|nested_arrays|json-deep-worker' \
crates/perry-runtime/src/json/mod.rsRepository: PerryTS/perry
Length of output: 19795
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- JSON module files and parse symbol ---'
fd -t f . crates/perry-runtime/src/json
rg -n -C 20 'pub .*js_json_parse_result|fn js_json_parse_result|js_json_parse_result' \
crates/perry-runtime/src/json --glob '*.rs'
echo '--- deep test context ---'
sed -n '828,915p' crates/perry-runtime/src/json/mod.rs
echo '--- storage allocation implementation ---'
sed -n '470,530p' crates/perry-runtime/src/string/mod.rs
rg -n -C 12 'fn arena_alloc_gc|pub .*arena_alloc_gc|ensure_gc_initialized' \
crates/perry-runtime/src/arena crates/perry-runtime/src/gc --glob '*.rs' | head -n 220
echo '--- parse initialization references ---'
rg -n -C 10 'ensure_gc_initialized|js_get_global_this|gc_init|gc_check_trigger' \
crates/perry-runtime/src/json --glob '*.rs'Repository: PerryTS/perry
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
json = Path("crates/perry-runtime/src/json/mod.rs").read_text()
parse = Path("crates/perry-runtime/src/json/parse_api.rs").read_text()
string_alloc = Path("crates/perry-runtime/src/string/alloc.rs").read_text()
arena = Path("crates/perry-runtime/src/arena/allocators.rs").read_text()
thread = Path("crates/perry-runtime/src/thread.rs").read_text()
gc = Path("crates/perry-runtime/src/gc/mod.rs").read_text()
worker = re.search(
r'fn parses_three_hundred_thousand_levels_on_a_small_worker_stack\(\).*?'
r'\.spawn\(\|\| \{(?P<body>.*?)\n\s*\}\)\s*\.expect',
json,
re.S,
)
assert worker, "deep worker test not found"
worker_body = worker.group("body")
string_fn = re.search(
r'pub extern "C" fn js_string_from_bytes\(.*?\n\}',
string_alloc,
re.S,
)
assert string_fn
string_body = string_fn.group(0)
parse_fn = re.search(
r'pub unsafe fn js_json_parse_result\(.*?(?=\n\}\n)',
parse,
re.S,
)
assert parse_fn
parse_body = parse_fn.group(0)
arena_fn = re.search(
r'pub fn arena_alloc_gc\(.*?(?=\n\}\n)',
arena,
re.S,
)
assert arena_fn
arena_body = arena_fn.group(0)
print("worker calls ensure_gc_initialized:",
"ensure_gc_initialized" in worker_body)
print("worker allocates string before parse:",
worker_body.index("js_string_from_bytes") < worker_body.index("js_json_parse_result"))
print("js_string_from_bytes calls ensure_gc_initialized:",
"ensure_gc_initialized" in string_fn.group(0))
print("js_json_parse_result calls ensure_gc_initialized:",
"ensure_gc_initialized" in parse_fn.group(0))
print("js_json_parse_result calls gc_check_trigger:",
"gc_check_trigger" in parse)
print("arena_alloc_gc calls ensure_gc_initialized:",
"ensure_gc_initialized" in arena_fn.group(0))
print("runtime worker initializes before allocation:",
thread.index("ensure_gc_initialized") < thread.index("js_closure_alloc"))
print("GC initialization state is thread-local:",
"thread_local!" in gc and "GC_INIT_DONE" in gc)
PYRepository: PerryTS/perry
Length of output: 546
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
json = Path("crates/perry-runtime/src/json/mod.rs").read_text()
parse = Path("crates/perry-runtime/src/json/parse_api.rs").read_text()
string_alloc = Path("crates/perry-runtime/src/string/alloc.rs").read_text()
arena = Path("crates/perry-runtime/src/arena/allocators.rs").read_text()
thread = Path("crates/perry-runtime/src/thread.rs").read_text()
gc = Path("crates/perry-runtime/src/gc/mod.rs").read_text()
worker = re.search(
r'fn parses_three_hundred_thousand_levels_on_a_small_worker_stack\(\).*?'
r'\.spawn\(\|\| \{(?P<body>.*?)\n\s*\}\)\s*\.expect',
json,
re.S,
)
assert worker, "deep worker test not found"
worker_body = worker.group("body")
string_fn = re.search(
r'pub extern "C" fn js_string_from_bytes\(.*?\n\}',
string_alloc,
re.S,
)
assert string_fn
parse_fn = re.search(
r'pub unsafe fn js_json_parse_result\(.*?(?=\n\}\n)',
parse,
re.S,
)
assert parse_fn
arena_fn = re.search(
r'pub fn arena_alloc_gc\(.*?(?=\n\}\n)',
arena,
re.S,
)
assert arena_fn
print("worker calls ensure_gc_initialized:",
"ensure_gc_initialized" in worker_body)
print("worker allocates string before parse:",
worker_body.index("js_string_from_bytes") < worker_body.index("js_json_parse_result"))
print("js_string_from_bytes calls ensure_gc_initialized:",
"ensure_gc_initialized" in string_fn.group(0))
print("js_json_parse_result calls ensure_gc_initialized:",
"ensure_gc_initialized" in parse_fn.group(0))
print("js_json_parse_result calls gc_check_trigger:",
"gc_check_trigger" in parse)
print("arena_alloc_gc calls ensure_gc_initialized:",
"ensure_gc_initialized" in arena_fn.group(0))
print("runtime worker initializes before allocation:",
thread.index("ensure_gc_initialized") < thread.index("js_closure_alloc"))
print("GC initialization state is thread-local:",
"thread_local!" in gc and "GC_INIT_DONE" in gc)
PYRepository: PerryTS/perry
Length of output: 546
Initialize GC on the spawned worker before runtime allocation. ensure_gc_initialized() is thread-local. This closure calls js_string_from_bytes before parsing, and js_json_parse_result can trigger GC. Call crate::gc::ensure_gc_initialized() before these calls. RUST_TEST_THREADS=1 does not replace per-thread initialization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/json/mod.rs` around lines 888 - 911, Initialize the
thread-local GC at the start of the closure in
parses_three_hundred_thousand_levels_on_a_small_worker_stack by calling
crate::gc::ensure_gc_initialized() before nested_arrays, js_string_from_bytes,
or js_json_parse_result perform runtime allocation. Keep the existing parsing
and validation flow unchanged.
Sources: Coding guidelines, Learnings
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #7817. JSON.parse now switches from the recursive fast path to the existing flat tape representation once nesting passes the worker-stack threshold, then materializes that tape with an explicit heap-backed work stack. The reported 300,000-level document parses on a 2 MiB worker stack.
A separate 500,000-level iterative nesting budget keeps resource use finite and reports a catchable RangeError.
Changes
Related issue
Closes #7817. Follows the crash guard in #7816.
Test plan
The quiet-mini benchmark is pending because its required lock is held by strcat-agent; no performance numbers are claimed here yet.
Screenshots / output
N/A (runtime parser change).
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
RangeErrorinstead of risking parser failure.