diff --git a/changelog.d/10168-json-parse-depth-in-descent.md b/changelog.d/10168-json-parse-depth-in-descent.md new file mode 100644 index 0000000000..cb8b737c8e --- /dev/null +++ b/changelog.d/10168-json-parse-depth-in-descent.md @@ -0,0 +1,7 @@ +### perf(json): bound nesting inside the direct parser instead of pre-scanning every document + +`JSON.parse` re-read every byte of a direct-parsed document before parsing it, to decide whether the recursive descent could overflow the native stack (`nesting_depth_exceeds`, the 1000-level handoff to the heap-stack parser). The "already validated" shortcut that was meant to skip the re-scan on repeated parses lived on the string-token reuse cache, which is only populated for a source under 2 MB that contains one large string value, so no record document ever hit it: every parse of a 20 MB record array, of a record object of any size, and (since the traversal-feedback change) every eagerly re-routed scan paid a whole-document scan on top of the parse. `sample` attributed 6.2 % of `records_array_20m:roundtrip` to the scan alone. + +`DirectParser` now counts open containers as it descends (`enter_container`/`leave_container`, on every recursive entry including the shaped-record path and the typed top-level array) and aborts with `depth_exceeded` when the 1000th nested container would open. Valid documents never scan. A failed direct parse is re-routed to the heap-stack parser when it hit the bound or when the cold classifier says the document is deep, so malformed deep input keeps the exact error kinds it had (the cross-entry error-ordering test is unchanged). The only remaining pre-scan is the forced-tape-above-16 MB case, where an over-budget document must fail before its native tape is reserved. The `direct_depth_validated` cache flag and its two accessors are gone. + +Measured on the same tree (two builds, one self-contained worker per arm, interleaved best-of-3, `/usr/bin/time -l`, loaded shared host): direct-parse rows drop 10-13 % CPU (`records_array_20m` parse 164.6 → 145.7 ms, sparse 165.0 → 143.3, scan 169.2 → 148.3, roundtrip 195.8 → 186.2; `records_object_20m:parse` 166.0 → 144.1; `records_object_8m:parse` 209.2 → 186.5; `records_object_1m:parse` 172.6 → 152.9; the eagerly re-routed `records_array_16k`/`1m`/`8m` scan rows 133.9 → 119.9 / 170.9 → 154.2 / 145.3 → 131.3), the control rows are unchanged (lazy-tape `records_array_1m` parse 168.8 → 168.2, roundtrip 177.7 → 177.5; `wide_1m:parse` 173.9 → 173.8; `small_record:parse` 161.7 → 162.4), and peak RSS is identical on every row. diff --git a/crates/perry-runtime/src/json/mod.rs b/crates/perry-runtime/src/json/mod.rs index 0951812113..1098de6356 100644 --- a/crates/perry-runtime/src/json/mod.rs +++ b/crates/perry-runtime/src/json/mod.rs @@ -87,9 +87,8 @@ pub(crate) unsafe fn test_json_stringify_record_output(bits: u64) -> Option ! { crate::exception::js_throw(range_error_value(message)) } -/// Select the heap-stack parser before recursive validation or materialization -/// gets close to the smallest worker-thread stack. -/// -/// `js_json_parse` and `js_json_parse_result` are separate implementations of -/// the same flow, and the typed-array path is a third. Sharing the decision is -/// what keeps them from drifting — the first version of this fix guarded only -/// one of the three and appeared to do nothing at all, because the entry point -/// codegen actually calls was one of the other two. +/// Whole-document nesting classifier for the cold paths only: a direct parse +/// that failed (malformed, or deeper than the native bound?) and a forced tape +/// above the lazy size ceiling. Valid documents never pay it: `DirectParser` +/// bounds its own recursion (`enter_container`), so the three parse entries +/// (`js_json_parse`, `js_json_parse_result`, the typed-array path) share one +/// decision inside the descent instead of a scan each. The first version of +/// the depth fix guarded only one of the three and appeared to do nothing at +/// all, because the entry point codegen actually calls was one of the others. fn requires_iterative_parse(bytes: &[u8]) -> bool { // Every nesting level requires an opening byte, even in malformed input. // A scalar root is parsed and then `finish` rejects any second token, so @@ -119,6 +119,87 @@ fn json_parse_entry_depth_bound_preserves_the_first_excess_opening() { assert!(!requires_iterative_parse(quoted.as_bytes())); } +#[cfg(test)] +fn direct_parse_depth_exceeded(input: &[u8], shape_keys: Option<&[u8]>) -> bool { + let saved_roots = parse_root_save_len(); + let exceeded = { + let _suppress = crate::gc::GcSuppressScope::new(); + unsafe { + match shape_keys { + Some(keys) => { + let shape = build_shape_hint(keys.as_ptr(), keys.len() as u32, 1) + .expect("one packed key builds a shape hint"); + let mut parser = DirectParser::with_shape(input, shape); + parser.parse_array_typed(); + let _ = parser.finish(); + parser.depth_exceeded() + } + None => { + let mut parser = DirectParser::new(input); + parser.parse_value(); + let _ = parser.finish(); + parser.depth_exceeded() + } + } + } + }; + parse_root_restore(saved_roots); + exceeded +} + +/// The descent replaces the whole-document pre-scan, so it must draw the +/// same line: at the bound stays direct, one past it aborts with the flag, +/// and neither closers, quoted openers nor a shallow syntax error count. +/// +/// The bound is sized for the release runtime's frames; a debug test build's +/// parser frames are several times larger, so the 1000-level cases run on a +/// roomy worker thread rather than the harness's default stack. +#[test] +fn direct_parser_bounds_nesting_inside_the_descent() { + std::thread::Builder::new() + .name("json-depth-bound".into()) + .stack_size(256 * 1024 * 1024) + .spawn(direct_parser_bounds_nesting_inside_the_descent_body) + .expect("worker thread starts") + .join() + .expect("depth-bound checks do not panic"); +} + +#[cfg(test)] +fn direct_parser_bounds_nesting_inside_the_descent_body() { + let limit = crate::json::parser::MAX_RECURSIVE_NESTING_DEPTH; + let mut at_bound = vec![b'['; limit]; + at_bound.extend(std::iter::repeat_n(b']', limit)); + assert!(!direct_parse_depth_exceeded(&at_bound, None)); + assert!(direct_parse_depth_exceeded(&vec![b'['; limit + 1], None)); + assert!(direct_parse_depth_exceeded( + &b"{\"a\":".repeat(limit + 1), + None + )); + assert!(!direct_parse_depth_exceeded(&vec![b'}'; limit + 1], None)); + let quoted = format!("\"{}\"", "[".repeat(limit + 1)); + assert!(!direct_parse_depth_exceeded(quoted.as_bytes(), None)); + assert!(!direct_parse_depth_exceeded(b"[?,[[[[", None)); + + // The shaped-record path counts the outer array and every record level. + let typed_records = |records: usize| { + let mut input = b"[".to_vec(); + input.extend_from_slice(&b"{\"x\":".repeat(records)); + input.push(b'1'); + input.extend(std::iter::repeat_n(b'}', records)); + input.push(b']'); + input + }; + assert!(direct_parse_depth_exceeded( + &typed_records(limit), + Some(b"x\0") + )); + assert!(!direct_parse_depth_exceeded( + &typed_records(limit - 1), + Some(b"x\0") + )); +} + fn exceeds_iterative_budget(bytes: &[u8]) -> bool { crate::json::parser::nesting_depth_exceeds( bytes, @@ -243,13 +324,6 @@ unsafe fn parse_result_slow(text_ptr: *const StringHeader, len: usize) -> Result } } } - if !cached_parse_source_is_direct(text_ptr, len) && requires_iterative_parse(bytes) { - if exceeds_iterative_budget(bytes) { - return Err(range_error_value(&iterative_budget_message())); - } - return try_parse_deep_iterative(text_ptr, len) - .ok_or_else(|| syntax_error_value("JSON parse error: malformed deep document")); - } // #7341: root the source string BEFORE the collection points, then // re-derive the input slice from the rooted value. @@ -280,8 +354,8 @@ unsafe fn parse_result_slow(text_ptr: *const StringHeader, len: usize) -> Result let mut parser = DirectParser::new_batched_from_string(bytes, source); let result = parser.parse_value(); let parse_ok = parser.finish(); + let depth_exceeded = parser.depth_exceeded(); if parse_ok { - validate_cached_parse_source(source, len); remember_parse_object_template(source, len, result); } parse_root_push(result); @@ -289,11 +363,18 @@ unsafe fn parse_result_slow(text_ptr: *const StringHeader, len: usize) -> Result super::stringify_flat::finish_parse_gc_accounting(); crate::gc::gc_schedule_parse_boundary_collection_if_pressure(); gc_allocation.finish(); + // Re-derive the source from its root before releasing it: a failed parse + // may still hand the document to the heap-stack parser, which installs + // its own root, and nothing in between collects. + let text = parse_root_get(text_root).as_string_ptr(); parse_root_restore(text_root); super::parse_scalar::clear_oversized_key_cache(); if !parse_ok { + if failed_direct_parse_is_deep(depth_exceeded, text, len) { + return parse_deep_or_error(text, len); + } return Err(syntax_error_value("JSON parse error: malformed input")); } @@ -397,13 +478,10 @@ unsafe fn parse_slow(text_ptr: *const StringHeader, len: usize) -> JSValue { } let use_tape = tape_route_eligible(len, bytes); // The tape's explicit stack proves shallow/deep admission in its syntax - // pass. Keep the preflight for direct parses and forced oversized tapes: - // a huge over-budget input must fail before reserving its native tape. - let preflight_depth = !use_tape || len > LAZY_MAX_BLOB_BYTES; - if preflight_depth - && !cached_parse_source_is_direct(text_ptr, len) - && requires_iterative_parse(bytes) - { + // pass, and the direct parser bounds its own descent. Only a forced tape + // above the lazy size ceiling still needs the whole-document scan: a huge + // over-budget input must fail before reserving its native tape. + if use_tape && len > LAZY_MAX_BLOB_BYTES && requires_iterative_parse(bytes) { return parse_deep_or_throw(text_ptr, len); } @@ -464,16 +542,9 @@ unsafe fn parse_slow(text_ptr: *const StringHeader, len: usize) -> JSValue { parse_root_restore(text_root); return result; } - // A malformed or over-budget tape must not enter recursion without - // the old depth check. Re-derive the source after the entry collection. - let text = parse_root_get(text_root).as_string_ptr(); - let bytes = std::slice::from_raw_parts(crate::string::string_data(text), len); - if !preflight_depth && requires_iterative_parse(bytes) { - // No collection occurs between releasing this root and the deep - // helper installing its own source root. - parse_root_restore(text_root); - return parse_deep_or_throw(text, len); - } + // A declined tape (malformed, or past the iterative budget) falls + // through to the direct parser, whose own depth accounting hands deep + // input to the heap-stack parser after the failed descent. } // #64 follow-up: opportunistic pre-parse cleanup. When parse runs in a @@ -525,8 +596,8 @@ unsafe fn parse_slow(text_ptr: *const StringHeader, len: usize) -> JSValue { let mut parser = DirectParser::new_batched_from_string(bytes, source); let result = parser.parse_value(); let parse_ok = parser.finish(); + let depth_exceeded = parser.depth_exceeded(); if parse_ok { - validate_cached_parse_source(source, len); remember_parse_object_template(source, len, result); } parse_root_push(result); @@ -538,6 +609,10 @@ unsafe fn parse_slow(text_ptr: *const StringHeader, len: usize) -> JSValue { super::stringify_flat::finish_parse_gc_accounting(); crate::gc::gc_schedule_parse_boundary_collection_if_pressure(); gc_allocation.finish(); + // Re-derive the source from its root before releasing it: a failed parse + // may still hand the document to the heap-stack parser, which installs + // its own root, and nothing in between collects. + let text = parse_root_get(text_root).as_string_ptr(); parse_root_restore(text_root); // Keep key intern cache across parses — scan_parse_roots marks cached @@ -547,6 +622,9 @@ unsafe fn parse_slow(text_ptr: *const StringHeader, len: usize) -> JSValue { super::parse_scalar::clear_oversized_key_cache(); if !parse_ok { + if failed_direct_parse_is_deep(depth_exceeded, text, len) { + return parse_deep_or_throw(text, len); + } throw_syntax_error("JSON parse error: malformed input"); } @@ -566,6 +644,31 @@ unsafe fn parse_deep_or_throw(text: *const StringHeader, len: usize) -> JSValue } } +/// The `Result` form of `parse_deep_or_throw`, for `js_json_parse_result`. +unsafe fn parse_deep_or_error(text: *const StringHeader, len: usize) -> Result { + let bytes = std::slice::from_raw_parts(crate::string::string_data(text), len); + if exceeds_iterative_budget(bytes) { + return Err(range_error_value(&iterative_budget_message())); + } + try_parse_deep_iterative(text, len) + .ok_or_else(|| syntax_error_value("JSON parse error: malformed deep document")) +} + +/// A failed direct parse goes to the heap-stack parser when the descent +/// aborted at its bound, or when the document nests past that bound anyway: +/// malformed deep input keeps reporting through the path it always used, and +/// only failed parses pay the whole-document scan. +unsafe fn failed_direct_parse_is_deep( + depth_exceeded: bool, + text: *const StringHeader, + len: usize, +) -> bool { + depth_exceeded || { + let bytes = std::slice::from_raw_parts(crate::string::string_data(text), len); + requires_iterative_parse(bytes) + } +} + /// v0.5.210: tape-mode selector. Cached at first JSON.parse so we /// pay the env-var lookup once per process, not once per parse. #[derive(Copy, Clone)] @@ -725,12 +828,6 @@ pub unsafe extern "C" fn js_json_parse_typed_array( return js_json_parse(text_ptr); } - // Deep input uses the generic entry's heap-stack fallback. The shape fast - // path is deliberately retained for ordinary payloads only. - if requires_iterative_parse(bytes) { - return js_json_parse(text_ptr); - } - // Same pre-parse cleanup + GC suppression as `js_json_parse` — // root before the collection point and re-derive the source bytes after it. let text_root = parse_root_push(JSValue::string_ptr(text_ptr as *mut StringHeader)); @@ -764,16 +861,24 @@ pub unsafe extern "C" fn js_json_parse_typed_array( let mut parser = DirectParser::with_shape(bytes, shape); let result = parser.parse_array_typed(); let parse_ok = parser.finish(); + let depth_exceeded = parser.depth_exceeded(); parse_root_push(result); crate::gc::gc_unsuppress(); super::stringify_flat::finish_parse_gc_accounting(); gc_allocation.finish(); + // Re-derive the source from its root before releasing it: a failed parse + // may still hand the document to the heap-stack parser, which installs + // its own root, and nothing in between collects. + let text = parse_root_get(text_root).as_string_ptr(); parse_root_restore(text_root); super::parse_scalar::clear_oversized_key_cache(); if !parse_ok { + if failed_direct_parse_is_deep(depth_exceeded, text, len) { + return parse_deep_or_throw(text, len); + } throw_syntax_error("JSON parse error: malformed input"); } diff --git a/crates/perry-runtime/src/json/parse_reuse.rs b/crates/perry-runtime/src/json/parse_reuse.rs index d5b6b1e4b0..b5da700088 100644 --- a/crates/perry-runtime/src/json/parse_reuse.rs +++ b/crates/perry-runtime/src/json/parse_reuse.rs @@ -19,7 +19,6 @@ struct ParseStringCacheEntry { token_start: u32, token_end: u32, value: *const StringHeader, - direct_depth_validated: bool, } #[derive(Clone, Copy)] @@ -120,7 +119,6 @@ pub(crate) fn remember_parse_string( token_start: token_start as u32, token_end: token_end as u32, value, - direct_depth_validated: false, }); crate::gc::runtime_write_barrier_root_nanbox( crate::JSValue::string_ptr(source.cast_mut()).bits(), @@ -131,31 +129,6 @@ pub(crate) fn remember_parse_string( }); } -#[inline] -pub(crate) fn cached_parse_source_is_direct( - source: *const StringHeader, - source_len: usize, -) -> bool { - PARSE_STRING_CACHE.with(|cache| { - cache.borrow().as_ref().is_some_and(|entry| { - entry.source == source - && entry.source_len as usize == source_len - && entry.direct_depth_validated - }) - }) -} - -#[inline] -pub(crate) fn validate_cached_parse_source(source: *const StringHeader, source_len: usize) { - PARSE_STRING_CACHE.with(|cache| { - if let Some(entry) = cache.borrow_mut().as_mut() { - if entry.source == source && entry.source_len as usize == source_len { - entry.direct_depth_validated = true; - } - } - }); -} - #[inline] unsafe fn parse_template_value(value: JSValue) -> Option { if !value.is_pointer() { @@ -335,7 +308,6 @@ pub(crate) fn test_seed_root_scanner_slots( token_start: 0, token_end: 1, value: string_value, - direct_depth_validated: false, }); }); let mut values = [EMPTY_PARSE_TEMPLATE_VALUE; PARSE_OBJECT_TEMPLATE_MAX_FIELDS]; diff --git a/crates/perry-runtime/src/json/parser.rs b/crates/perry-runtime/src/json/parser.rs index 832b3f4641..f22fa618f5 100644 --- a/crates/perry-runtime/src/json/parser.rs +++ b/crates/perry-runtime/src/json/parser.rs @@ -259,6 +259,14 @@ pub(crate) struct DirectParser<'a> { input: &'a [u8], pos: usize, valid: bool, + /// Containers open on the native stack. The descent bounds itself at + /// `MAX_RECURSIVE_NESTING_DEPTH` instead of relying on a whole-document + /// nesting pre-scan, which re-read every byte of a large record document + /// on every parse. + depth: usize, + /// Set when the bound aborted the parse. The value is invalid; the entry + /// points hand such input to the heap-stack parser. + depth_exceeded: bool, /// Issue #179 typed-parse: if Some, the top-level value is /// expected to be `Array` matching this shape. Each /// record uses the fast path; mismatches silently fall through @@ -298,6 +306,8 @@ impl<'a> DirectParser<'a> { input, pos: 0, valid: true, + depth: 0, + depth_exceeded: false, shape: None, hot_shape_len: 0, hot_shape_keys: [std::ptr::null(); 8], @@ -347,6 +357,8 @@ impl<'a> DirectParser<'a> { input, pos: 0, valid: true, + depth: 0, + depth_exceeded: false, shape: Some(shape), hot_shape_len: 0, hot_shape_keys: [std::ptr::null(); 8], @@ -460,12 +472,57 @@ impl<'a> DirectParser<'a> { JSValue::null() } + /// True once the descent aborted at the native recursion bound. + #[inline] + pub(crate) fn depth_exceeded(&self) -> bool { + self.depth_exceeded + } + + /// Account one container on the native stack, or abort the parse at the + /// bound. Every recursive container entry goes through this, including + /// the shaped-record path, so the three parse entries cannot drift. + #[inline(always)] + fn enter_container(&mut self) -> bool { + if self.depth >= MAX_RECURSIVE_NESTING_DEPTH { + self.depth_exceeded = true; + self.valid = false; + return false; + } + self.depth += 1; + true + } + + #[inline(always)] + fn leave_container(&mut self) { + self.depth -= 1; + } + + #[inline] + unsafe fn parse_array_nested(&mut self) -> JSValue { + if !self.enter_container() { + return JSValue::null(); + } + let value = self.parse_array(); + self.leave_container(); + value + } + + #[inline] + unsafe fn parse_object_shaped_nested(&mut self, shape: *const ObjectShapeHint) -> JSValue { + if !self.enter_container() { + return JSValue::null(); + } + let value = self.parse_object_shaped(&*shape); + self.leave_container(); + value + } + pub(crate) unsafe fn parse_value(&mut self) -> JSValue { self.skip_whitespace(); match self.peek() { Some(b'"') => self.parse_string_value(), Some(b'{') => self.parse_object(), - Some(b'[') => self.parse_array(), + Some(b'[') => self.parse_array_nested(), Some(b't') => self.parse_true(), Some(b'f') => self.parse_false(), Some(b'n') => self.parse_null(), @@ -873,6 +930,16 @@ impl<'a> DirectParser<'a> { // without the array-outer shape). return self.parse_value_generic(); } + if !self.enter_container() { + return JSValue::null(); + } + let value = self.parse_array_typed_body(); + self.leave_container(); + value + } + + /// The top-level typed array; its container level is already accounted. + unsafe fn parse_array_typed_body(&mut self) -> JSValue { self.advance(); self.skip_whitespace(); @@ -906,7 +973,7 @@ impl<'a> DirectParser<'a> { // Per-element: shaped object or generic value (if element // isn't an object, fall back). let value = if self.peek() == Some(b'{') { - self.parse_object_shaped(&*shape_ptr) + self.parse_object_shaped_nested(shape_ptr) } else { self.parse_value_generic() }; @@ -941,8 +1008,8 @@ impl<'a> DirectParser<'a> { self.skip_whitespace(); match self.peek() { Some(b'"') => self.parse_string_value(), - Some(b'{') => self.parse_object_untyped(), - Some(b'[') => self.parse_array(), + Some(b'{') => self.parse_object(), + Some(b'[') => self.parse_array_nested(), Some(b't') => self.parse_true(), Some(b'f') => self.parse_false(), Some(b'n') => self.parse_null(), @@ -957,7 +1024,12 @@ impl<'a> DirectParser<'a> { // `parse_object` here the only callers are (a) untyped parses // and (b) nested objects inside a shaped record — both want // generic behavior. Delegate to `parse_object_untyped`. - self.parse_object_untyped() + if !self.enter_container() { + return JSValue::null(); + } + let value = self.parse_object_untyped(); + self.leave_container(); + value } pub(crate) unsafe fn parse_object_untyped(&mut self) -> JSValue {