From 1d78afda4ed3807abc752ff238d02af3fd983c5f Mon Sep 17 00:00:00 2001 From: TalBarYakar Date: Mon, 5 Oct 2026 14:59:32 +0300 Subject: [PATCH 01/11] (Ram usage redusction): count objects size so allocation could be precise --- fuzz/fuzz_targets/fuzz_json_de.rs | 12 ++- src/de.rs | 148 ++++++++++++++++++++++++++---- tests/precount.rs | 116 +++++++++++++++++++++++ 3 files changed, 258 insertions(+), 18 deletions(-) create mode 100644 tests/precount.rs diff --git a/fuzz/fuzz_targets/fuzz_json_de.rs b/fuzz/fuzz_targets/fuzz_json_de.rs index b5a35cff..a4fdf372 100644 --- a/fuzz/fuzz_targets/fuzz_json_de.rs +++ b/fuzz/fuzz_targets/fuzz_json_de.rs @@ -1,12 +1,20 @@ #![no_main] use arbitrary_json::ArbitraryValue; -use ijson::IValue; +use ijson::{IValue, IValueDeserSeed}; use libfuzzer_sys::fuzz_target; use serde::Deserialize; fuzz_target!(|value: ArbitraryValue| { let json_string = value.to_string(); let mut deserializer = serde_json::Deserializer::from_str(&json_string); - let _ = IValue::deserialize(&mut deserializer); + let ordinary = IValue::deserialize(&mut deserializer); + let mut deserializer = serde_json::Deserializer::from_str(&json_string); + let counted = + IValueDeserSeed::new(None).deserialize_with_object_hints(&mut deserializer, &json_string); + match (ordinary, counted) { + (Ok(ordinary), Ok(counted)) => assert_eq!(ordinary, counted), + (Err(_), Err(_)) => {} + _ => panic!("object hints changed whether JSON parsing succeeds"), + } }); diff --git a/src/de.rs b/src/de.rs index 2270aae5..cb8999ee 100644 --- a/src/de.rs +++ b/src/de.rs @@ -38,6 +38,75 @@ impl IValueDeserSeed { } } +// A lexical sizing pass only: serde_json still performs all validation. +// One count per object, in preorder; delimiters inside strings are ignored. +fn object_counts(json: &str) -> Vec { + let mut counts = Vec::new(); + // Objects store their index in counts; arrays only mark a nesting level. + let mut stack = Vec::new(); + let mut quoted = false; + let mut escaped = false; + for byte in json.bytes() { + if quoted { + if escaped { + escaped = false; + } else if byte == b'\\' { + escaped = true; + } else if byte == b'"' { + quoted = false; + } + continue; + } + // Bound the sizing pass even for malformed or deeply nested input. + // Returning no hints leaves depth handling to the deserializer. + if matches!(byte, b'{' | b'[') && stack.len() == 128 { + return Vec::new(); + } + match byte { + b'"' => quoted = true, + b'{' => { + stack.push(Some(counts.len())); + counts.push(0); + } + b'[' => stack.push(None), + b'}' | b']' => { + stack.pop(); + } + b':' => { + if let Some(Some(index)) = stack.last() { + counts[*index] += 1; + } + } + _ => {} + } + } + counts +} + +impl IValueDeserSeed { + /// Deserializes JSON with object capacities counted before parsing. + /// + /// `deserializer` must traverse the same JSON text in input order, starting + /// at its beginning. Counts only affect allocation; validation and recursion + /// limits remain the deserializer's responsibility. The caller must check for + /// trailing input (for example, with `serde_json::Deserializer::end`). + /// Arrays retain their existing allocation behavior. + pub fn deserialize_with_object_hints<'de, D>( + self, + deserializer: D, + json: &str, + ) -> Result + where + D: Deserializer<'de>, + { + let mut counts = object_counts(json).into_iter(); + deserializer.deserialize_any(ValueVisitor { + fpha_config: self.fpha_config, + counts: Some(&mut counts), + }) + } +} + impl<'de> DeserializeSeed<'de> for IValueDeserSeed { type Value = IValue; @@ -82,7 +151,10 @@ impl<'de> Deserialize<'de> for IArray { where D: Deserializer<'de>, { - deserializer.deserialize_seq(ArrayVisitor { fpha_config: None }) + deserializer.deserialize_seq(ArrayVisitor { + fpha_config: None, + counts: None, + }) } } @@ -91,21 +163,40 @@ impl<'de> Deserialize<'de> for IObject { where D: Deserializer<'de>, { - deserializer.deserialize_map(ObjectVisitor { fpha_config: None }) + deserializer.deserialize_map(ObjectVisitor { + fpha_config: None, + counts: None, + }) } } -struct ValueVisitor { +struct ValueVisitor<'a> { fpha_config: Option, + counts: Option<&'a mut std::vec::IntoIter>, } -impl ValueVisitor { +impl ValueVisitor<'_> { fn new(fpha_config: Option) -> Self { - ValueVisitor { fpha_config } + ValueVisitor { + fpha_config, + counts: None, + } } } -impl<'de> Visitor<'de> for ValueVisitor { +// Reuse the visitor as a seed so nested values borrow the same count iterator. +impl<'de> DeserializeSeed<'de> for ValueVisitor<'_> { + type Value = IValue; + + fn deserialize(self, deserializer: D) -> Result + where + D: Deserializer<'de>, + { + deserializer.deserialize_any(self) + } +} + +impl<'de> Visitor<'de> for ValueVisitor<'_> { type Value = IValue; fn expecting(&self, formatter: &mut Formatter) -> fmt::Result { @@ -152,7 +243,7 @@ impl<'de> Visitor<'de> for ValueVisitor { where D: Deserializer<'de>, { - IValueDeserSeed::new(self.fpha_config).deserialize(deserializer) + self.deserialize(deserializer) } #[inline] @@ -167,6 +258,7 @@ impl<'de> Visitor<'de> for ValueVisitor { { ArrayVisitor { fpha_config: self.fpha_config, + counts: self.counts, } .visit_seq(visitor) .map(Into::into) @@ -178,6 +270,7 @@ impl<'de> Visitor<'de> for ValueVisitor { { ObjectVisitor { fpha_config: self.fpha_config, + counts: self.counts, } .visit_map(visitor) .map(Into::into) @@ -248,11 +341,12 @@ impl<'de> Visitor<'de> for StringVisitor { } } -struct ArrayVisitor { +struct ArrayVisitor<'a> { fpha_config: Option, + counts: Option<&'a mut std::vec::IntoIter>, } -impl<'de> Visitor<'de> for ArrayVisitor { +impl<'de> Visitor<'de> for ArrayVisitor<'_> { type Value = IArray; fn expecting(&self, formatter: &mut Formatter) -> fmt::Result { @@ -260,13 +354,16 @@ impl<'de> Visitor<'de> for ArrayVisitor { } #[inline] - fn visit_seq(self, mut visitor: V) -> Result + fn visit_seq(mut self, mut visitor: V) -> Result where V: SeqAccess<'de>, { let mut arr = IArray::with_capacity(visitor.size_hint().unwrap_or(0)) .map_err(|_| SError::custom("Failed to allocate array"))?; - while let Some(v) = visitor.next_element_seed(IValueDeserSeed::new(self.fpha_config))? { + while let Some(v) = visitor.next_element_seed(ValueVisitor { + fpha_config: self.fpha_config, + counts: self.counts.as_deref_mut(), + })? { match self.fpha_config { Some(FPHAConfig { fpha_type }) => arr.push_with_fp_type(v, fpha_type), None => arr.push(v).map_err(Into::into), @@ -277,28 +374,39 @@ impl<'de> Visitor<'de> for ArrayVisitor { } } -struct ObjectVisitor { +struct ObjectVisitor<'a> { fpha_config: Option, + counts: Option<&'a mut std::vec::IntoIter>, } -impl<'de> Visitor<'de> for ObjectVisitor { +impl<'de> Visitor<'de> for ObjectVisitor<'_> { type Value = IObject; fn expecting(&self, formatter: &mut Formatter) -> fmt::Result { formatter.write_str("JSON object") } - fn visit_map(self, mut visitor: V) -> Result + fn visit_map(mut self, mut visitor: V) -> Result where V: MapAccess<'de>, { - let mut obj = IObject::with_capacity(visitor.size_hint().unwrap_or(0)) + // Consume the parent's count before visiting any of its children. + let capacity = self.counts.as_deref_mut().and_then(Iterator::next); + let mut obj = IObject::with_capacity(capacity.or_else(|| visitor.size_hint()).unwrap_or(0)) .map_err(|_| SError::custom("Failed to allocate object"))?; while let Some(k) = visitor.next_key::()? { - let v = visitor.next_value_seed(IValueDeserSeed::new(self.fpha_config))?; + let v = visitor.next_value_seed(ValueVisitor { + fpha_config: self.fpha_config, + counts: self.counts.as_deref_mut(), + })?; obj.insert(k, v) .map_err(|e| SError::custom(e.to_string()))?; } + // Repeated keys overwrite earlier values, but the sizing pass counts + // every occurrence. Release those slots only on the hinted path. + if capacity.is_some() && obj.capacity() > obj.len() { + obj.shrink_to_fit(); + } Ok(obj) } } @@ -1072,6 +1180,14 @@ mod tests { use crate::array::ArraySliceRef; use serde::de::DeserializeSeed; + #[test] + fn object_counts_bounds_nesting() { + assert_eq!(object_counts(r#"{"a":[{"b":1}],"c":{}}"#), vec![2, 1, 0]); + assert!(object_counts(&"[".repeat(129)).is_empty()); + // Discard earlier hints too: partial counts would misalign later objects. + assert!(object_counts(&format!("{{\"a\":{{}},\"b\":{}", "[".repeat(128))).is_empty()); + } + #[test] fn test_deserialize_with_f64_fp() { let json = r#"[1.5, 2.5, 3.5]"#; diff --git a/tests/precount.rs b/tests/precount.rs new file mode 100644 index 00000000..5ab8c95c --- /dev/null +++ b/tests/precount.rs @@ -0,0 +1,116 @@ +use ijson::{array::ArraySliceRef, FPHAConfig, FloatType, IValue, IValueDeserSeed}; +use serde::de::DeserializeSeed; + +fn parse(s: &str, counted: bool) -> Result { + let mut de = serde_json::Deserializer::from_str(s); + let value = if counted { + IValueDeserSeed::new(None).deserialize_with_object_hints(&mut de, s)? + } else { + IValueDeserSeed::new(None).deserialize(&mut de)? + }; + de.end()?; + Ok(value) +} + +#[test] +fn counted_objects_preserve_values_errors_and_capacity() { + for input in [ + r#"{"a":1,"b":2,"c":3,"d":4,"e":5}"#, + r#"{"x":[{}, {"nested":{"a":null,"b":true}}],"str":"{[colon:]}"}"#, + r#"{"escaped\"key":"slash\\\" colon: } {","unicode":"שלום","n":-1.25e4}"#, + r#"[{"a":1,"a":{"b":2}},{"z":[]},[1,2,3]]"#, + "{}", + "[]", + "null", + "123", + "\"hello\"", + ] { + let counted = parse(input, true).unwrap(); + let ordinary = parse(input, false).unwrap(); + assert_eq!( + serde_json::to_value(&counted).unwrap(), + serde_json::to_value(&ordinary).unwrap() + ); + } + let input = r#"{"a":1,"b":2,"c":3,"d":4,"e":5}"#; + let mut value = parse(input, true).unwrap(); + assert_eq!(value.as_object().unwrap().capacity(), 5); + assert_eq!( + parse(input, false).unwrap().as_object().unwrap().capacity(), + 8 + ); + value + .as_object_mut() + .unwrap() + .insert("f", IValue::NULL) + .unwrap(); + assert_eq!(value.as_object().unwrap().len(), 6); + let nested = parse(r#"{"x":[{"a":1,"b":2,"c":3}] }"#, true).unwrap(); + assert_eq!(nested.as_object().unwrap().capacity(), 1); + assert_eq!(nested["x"][0].as_object().unwrap().capacity(), 3); + for input in [ + "", + "{", + "[", + "{\"a\":}", + "{\"a\":1,}", + "[1,]", + "{} {}", + "{\"a\":\"\\q\"}", + "{\"a\":1]", + "\"unterminated", + ] { + assert!(parse(input, true).is_err(), "{input}"); + assert!(parse(input, false).is_err(), "{input}"); + } + let deep = format!("{}0{}", "[".repeat(150), "]".repeat(150)); + assert!(parse(&deep, true).is_err()); +} + +#[test] +fn repeated_keys_release_unused_slots_and_preserve_last_value() { + // Escaped names compare equal after decoding. Replaced values may themselves + // contain objects, whose counts must still be consumed before the next field. + let input = r#"{"a":{"x":1},"\u0061":{"y":2,"z":3},"next":{"p":4}}"#; + let value = parse(input, true).unwrap(); + assert_eq!(value, parse(input, false).unwrap()); + assert_eq!(value.as_object().unwrap().capacity(), 2); + assert_eq!(value["a"].as_object().unwrap().capacity(), 2); + assert_eq!(value["next"].as_object().unwrap().capacity(), 1); + + let input = format!("{{{}\"same\":2}}", "\"same\":1,".repeat(100_000)); + let value = parse(&input, true).unwrap(); + assert_eq!(value.as_object().unwrap().capacity(), 1); + assert_eq!(value, parse(r#"{"same":2}"#, false).unwrap()); +} + +#[test] +fn counted_objects_preserve_typed_arrays() { + let input = r#"{"values":[0.5,1.0,1.5],"nested":[{"x":1,"y":2}]}"#; + for fp_type in [ + FloatType::F16, + FloatType::BF16, + FloatType::F32, + FloatType::F64, + ] { + let seed = || IValueDeserSeed::new(Some(FPHAConfig::new_with_type(fp_type))); + let mut de = serde_json::Deserializer::from_str(input); + let value = seed() + .deserialize_with_object_hints(&mut de, input) + .unwrap(); + de.end().unwrap(); + let ordinary = seed() + .deserialize(&mut serde_json::Deserializer::from_str(input)) + .unwrap(); + assert_eq!(value, ordinary); + assert_eq!(value["nested"][0].as_object().unwrap().capacity(), 2); + let array = value["values"].as_array().unwrap().as_slice(); + assert!(matches!( + (fp_type, array), + (FloatType::F16, ArraySliceRef::F16(_)) + | (FloatType::BF16, ArraySliceRef::BF16(_)) + | (FloatType::F32, ArraySliceRef::F32(_)) + | (FloatType::F64, ArraySliceRef::F64(_)) + )); + } +} From 91f8ce9a2c6d5c5c4a3966927a269d8c22b1c7e7 Mon Sep 17 00:00:00 2001 From: TalBarYakar Date: Mon, 5 Oct 2026 14:59:33 +0300 Subject: [PATCH 02/11] avoid counting every byte in a string --- Cargo.toml | 1 + src/de.rs | 31 +++++++++++++++++-------------- tests/precount.rs | 22 ++++++++++++++++++++++ 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ef2183a8..293e7fdf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ serde_json = "1.0.59" [dependencies] +memchr = "2" hashbrown = "0.13.2" dashmap = { version = "5.4", features = ["raw-api"] } lazy_static = "1.4.0" diff --git a/src/de.rs b/src/de.rs index cb8999ee..b8bc3717 100644 --- a/src/de.rs +++ b/src/de.rs @@ -44,26 +44,29 @@ fn object_counts(json: &str) -> Vec { let mut counts = Vec::new(); // Objects store their index in counts; arrays only mark a nesting level. let mut stack = Vec::new(); - let mut quoted = false; - let mut escaped = false; - for byte in json.bytes() { - if quoted { - if escaped { - escaped = false; - } else if byte == b'\\' { - escaped = true; - } else if byte == b'"' { - quoted = false; - } - continue; - } + let mut bytes = json.as_bytes(); + while let Some((&byte, rest)) = bytes.split_first() { + bytes = rest; // Bound the sizing pass even for malformed or deeply nested input. // Returning no hints leaves depth handling to the deserializer. if matches!(byte, b'{' | b'[') && stack.len() == 128 { return Vec::new(); } match byte { - b'"' => quoted = true, + b'"' => loop { + // Search string contents in chunks. A backslash escapes exactly + // the next byte for quote detection, including another backslash. + let Some(index) = memchr::memchr2(b'"', b'\\', bytes) else { + return counts; // The deserializer rejects unterminated strings. + }; + let delimiter = bytes[index]; + bytes = &bytes[index + 1..]; + if delimiter == b'"' { + break; + } + // A trailing backslash is invalid JSON, but must not panic here. + bytes = bytes.get(1..).unwrap_or_default(); + }, b'{' => { stack.push(Some(counts.len())); counts.push(0); diff --git a/tests/precount.rs b/tests/precount.rs index 5ab8c95c..bdc4cdc5 100644 --- a/tests/precount.rs +++ b/tests/precount.rs @@ -114,3 +114,25 @@ fn counted_objects_preserve_typed_arrays() { )); } } + +#[test] +fn string_skipping_preserves_escape_boundaries() { + // Exercise short strings and positions around SIMD chunk boundaries. + for padding in [0, 1, 15, 16, 31, 32, 63, 64, 65, 256, 4096] { + for slashes in 0..8 { + for suffix in ["", "\"", "\" :{}[] שלום"] { + let text = format!("{}{}{}", "x".repeat(padding), "\\".repeat(slashes), suffix); + let text = serde_json::to_string(&text).unwrap(); + let input = format!("{{{text}:{text},\"next\":{{\"a\":1,\"b\":2}}}}"); + let value = parse(&input, true).unwrap(); + assert_eq!(value, parse(&input, false).unwrap()); + assert_eq!(value.as_object().unwrap().capacity(), 2); + assert_eq!(value["next"].as_object().unwrap().capacity(), 2); + } + } + } + for input in ["\"", "\"abc\\", "{\"key\":\"abc\\", "\"\\q\"", "\"\\u12\""] { + assert!(parse(input, true).is_err(), "{input}"); + assert!(parse(input, false).is_err(), "{input}"); + } +} From f0fac2add34f0fc25b22fe85e0797b72d0a6b937 Mon Sep 17 00:00:00 2001 From: TalBarYakar Date: Mon, 5 Oct 2026 14:59:33 +0300 Subject: [PATCH 03/11] use some data structure to prevent double iter to obtain num of fields and aplly them into the object --- Cargo.toml | 1 - fuzz/fuzz_targets/fuzz_json_de.rs | 9 +- src/de.rs | 165 +++++++++++++++--------------- tests/precount.rs | 22 ++-- 4 files changed, 96 insertions(+), 101 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 293e7fdf..ef2183a8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,6 @@ serde_json = "1.0.59" [dependencies] -memchr = "2" hashbrown = "0.13.2" dashmap = { version = "5.4", features = ["raw-api"] } lazy_static = "1.4.0" diff --git a/fuzz/fuzz_targets/fuzz_json_de.rs b/fuzz/fuzz_targets/fuzz_json_de.rs index a4fdf372..92192774 100644 --- a/fuzz/fuzz_targets/fuzz_json_de.rs +++ b/fuzz/fuzz_targets/fuzz_json_de.rs @@ -10,11 +10,10 @@ fuzz_target!(|value: ArbitraryValue| { let mut deserializer = serde_json::Deserializer::from_str(&json_string); let ordinary = IValue::deserialize(&mut deserializer); let mut deserializer = serde_json::Deserializer::from_str(&json_string); - let counted = - IValueDeserSeed::new(None).deserialize_with_object_hints(&mut deserializer, &json_string); - match (ordinary, counted) { - (Ok(ordinary), Ok(counted)) => assert_eq!(ordinary, counted), + let buffered = IValueDeserSeed::new(None).deserialize_compact_objects(&mut deserializer); + match (ordinary, buffered) { + (Ok(ordinary), Ok(buffered)) => assert_eq!(ordinary, buffered), (Err(_), Err(_)) => {} - _ => panic!("object hints changed whether JSON parsing succeeds"), + _ => panic!("buffered objects changed whether JSON parsing succeeds"), } }); diff --git a/src/de.rs b/src/de.rs index b8bc3717..923824c0 100644 --- a/src/de.rs +++ b/src/de.rs @@ -38,74 +38,26 @@ impl IValueDeserSeed { } } -// A lexical sizing pass only: serde_json still performs all validation. -// One count per object, in preorder; delimiters inside strings are ignored. -fn object_counts(json: &str) -> Vec { - let mut counts = Vec::new(); - // Objects store their index in counts; arrays only mark a nesting level. - let mut stack = Vec::new(); - let mut bytes = json.as_bytes(); - while let Some((&byte, rest)) = bytes.split_first() { - bytes = rest; - // Bound the sizing pass even for malformed or deeply nested input. - // Returning no hints leaves depth handling to the deserializer. - if matches!(byte, b'{' | b'[') && stack.len() == 128 { - return Vec::new(); - } - match byte { - b'"' => loop { - // Search string contents in chunks. A backslash escapes exactly - // the next byte for quote detection, including another backslash. - let Some(index) = memchr::memchr2(b'"', b'\\', bytes) else { - return counts; // The deserializer rejects unterminated strings. - }; - let delimiter = bytes[index]; - bytes = &bytes[index + 1..]; - if delimiter == b'"' { - break; - } - // A trailing backslash is invalid JSON, but must not panic here. - bytes = bytes.get(1..).unwrap_or_default(); - }, - b'{' => { - stack.push(Some(counts.len())); - counts.push(0); - } - b'[' => stack.push(None), - b'}' | b']' => { - stack.pop(); - } - b':' => { - if let Some(Some(index)) = stack.last() { - counts[*index] += 1; - } - } - _ => {} - } - } - counts +// Reused across completed sibling objects during one deserialization. The vector +// preserves insertion order; decoded keys in the index enforce last-value wins. +#[derive(Default)] +struct ObjectBuffer { + fields: Vec<(IString, IValue)>, + indices: hashbrown::HashMap, } impl IValueDeserSeed { - /// Deserializes JSON with object capacities counted before parsing. - /// - /// `deserializer` must traverse the same JSON text in input order, starting - /// at its beginning. Counts only affect allocation; validation and recursion - /// limits remain the deserializer's responsibility. The caller must check for - /// trailing input (for example, with `serde_json::Deserializer::end`). - /// Arrays retain their existing allocation behavior. - pub fn deserialize_with_object_hints<'de, D>( - self, - deserializer: D, - json: &str, - ) -> Result + /// Deserializes using temporary object buffers, then moves each object's + /// unique fields into exactly sized storage. Arrays keep normal growth. + /// The caller must still check for trailing input with the deserializer. + pub fn deserialize_compact_objects<'de, D>(self, deserializer: D) -> Result where D: Deserializer<'de>, { - let mut counts = object_counts(json).into_iter(); + let mut buffers = Vec::new(); deserializer.deserialize_any(ValueVisitor { fpha_config: self.fpha_config, - counts: Some(&mut counts), + buffers: Some(&mut buffers), }) } } @@ -156,7 +108,7 @@ impl<'de> Deserialize<'de> for IArray { { deserializer.deserialize_seq(ArrayVisitor { fpha_config: None, - counts: None, + buffers: None, }) } } @@ -168,26 +120,26 @@ impl<'de> Deserialize<'de> for IObject { { deserializer.deserialize_map(ObjectVisitor { fpha_config: None, - counts: None, + buffers: None, }) } } struct ValueVisitor<'a> { fpha_config: Option, - counts: Option<&'a mut std::vec::IntoIter>, + buffers: Option<&'a mut Vec>, } impl ValueVisitor<'_> { fn new(fpha_config: Option) -> Self { ValueVisitor { fpha_config, - counts: None, + buffers: None, } } } -// Reuse the visitor as a seed so nested values borrow the same count iterator. +// Nested visitors share the pool of reusable object buffers. impl<'de> DeserializeSeed<'de> for ValueVisitor<'_> { type Value = IValue; @@ -261,7 +213,7 @@ impl<'de> Visitor<'de> for ValueVisitor<'_> { { ArrayVisitor { fpha_config: self.fpha_config, - counts: self.counts, + buffers: self.buffers, } .visit_seq(visitor) .map(Into::into) @@ -273,7 +225,7 @@ impl<'de> Visitor<'de> for ValueVisitor<'_> { { ObjectVisitor { fpha_config: self.fpha_config, - counts: self.counts, + buffers: self.buffers, } .visit_map(visitor) .map(Into::into) @@ -346,7 +298,7 @@ impl<'de> Visitor<'de> for StringVisitor { struct ArrayVisitor<'a> { fpha_config: Option, - counts: Option<&'a mut std::vec::IntoIter>, + buffers: Option<&'a mut Vec>, } impl<'de> Visitor<'de> for ArrayVisitor<'_> { @@ -365,7 +317,7 @@ impl<'de> Visitor<'de> for ArrayVisitor<'_> { .map_err(|_| SError::custom("Failed to allocate array"))?; while let Some(v) = visitor.next_element_seed(ValueVisitor { fpha_config: self.fpha_config, - counts: self.counts.as_deref_mut(), + buffers: self.buffers.as_deref_mut(), })? { match self.fpha_config { Some(FPHAConfig { fpha_type }) => arr.push_with_fp_type(v, fpha_type), @@ -379,7 +331,7 @@ impl<'de> Visitor<'de> for ArrayVisitor<'_> { struct ObjectVisitor<'a> { fpha_config: Option, - counts: Option<&'a mut std::vec::IntoIter>, + buffers: Option<&'a mut Vec>, } impl<'de> Visitor<'de> for ObjectVisitor<'_> { @@ -389,27 +341,51 @@ impl<'de> Visitor<'de> for ObjectVisitor<'_> { formatter.write_str("JSON object") } - fn visit_map(mut self, mut visitor: V) -> Result + fn visit_map(self, mut visitor: V) -> Result where V: MapAccess<'de>, { - // Consume the parent's count before visiting any of its children. - let capacity = self.counts.as_deref_mut().and_then(Iterator::next); - let mut obj = IObject::with_capacity(capacity.or_else(|| visitor.size_hint()).unwrap_or(0)) + if let Some(buffers) = self.buffers { + let mut buffer = buffers.pop().unwrap_or_default(); + while let Some(key) = visitor.next_key::()? { + let value = visitor.next_value_seed(ValueVisitor { + fpha_config: self.fpha_config, + buffers: Some(&mut *buffers), + })?; + if let Some(&index) = buffer.indices.get(&key) { + buffer.fields[index].1 = value; + } else { + buffer.fields.try_reserve(1).map_err(SError::custom)?; + buffer + .indices + .try_reserve(1) + .map_err(|_| SError::custom("Failed to allocate object buffer"))?; + buffer.indices.insert(key.clone(), buffer.fields.len()); + buffer.fields.push((key, value)); + } + } + // Allocate after parsing the fields, using the number of unique keys. + buffer.indices.clear(); + let mut obj = IObject::with_capacity(buffer.fields.len()) + .map_err(|_| SError::custom("Failed to allocate object"))?; + for (key, value) in buffer.fields.drain(..) { + obj.insert(key, value).map_err(SError::custom)?; + } + buffers.try_reserve(1).map_err(SError::custom)?; + buffers.push(buffer); + return Ok(obj); + } + + let mut obj = IObject::with_capacity(visitor.size_hint().unwrap_or(0)) .map_err(|_| SError::custom("Failed to allocate object"))?; while let Some(k) = visitor.next_key::()? { let v = visitor.next_value_seed(ValueVisitor { fpha_config: self.fpha_config, - counts: self.counts.as_deref_mut(), + buffers: None, })?; obj.insert(k, v) .map_err(|e| SError::custom(e.to_string()))?; } - // Repeated keys overwrite earlier values, but the sizing pass counts - // every occurrence. Release those slots only on the hinted path. - if capacity.is_some() && obj.capacity() > obj.len() { - obj.shrink_to_fit(); - } Ok(obj) } } @@ -1184,11 +1160,30 @@ mod tests { use serde::de::DeserializeSeed; #[test] - fn object_counts_bounds_nesting() { - assert_eq!(object_counts(r#"{"a":[{"b":1}],"c":{}}"#), vec![2, 1, 0]); - assert!(object_counts(&"[".repeat(129)).is_empty()); - // Discard earlier hints too: partial counts would misalign later objects. - assert!(object_counts(&format!("{{\"a\":{{}},\"b\":{}", "[".repeat(128))).is_empty()); + #[cfg(not(miri))] + fn compact_objects_do_not_reserve_for_unvalidated_or_duplicate_fields() { + for (input, valid) in [ + (format!("{{{}", ":".repeat(100_000)), false), + (r#"{"a":{"x":1},"b":[{"y":2},"#.to_owned(), false), + ( + format!("{{{}\"same\":2}}", "\"same\":1,".repeat(100_000)), + true, + ), + ] { + // Build the input outside the measured scope. Repetition must not + // translate into a proportional temporary or final object allocation. + let allocations = mockalloc::record_allocs(|| { + let mut de = serde_json::Deserializer::from_str(&input); + let result = IValueDeserSeed::new(None).deserialize_compact_objects(&mut de); + assert_eq!(result.is_ok(), valid); + if let Ok(value) = result { + assert_eq!(value.as_object().unwrap().capacity(), 1); + de.end().unwrap(); + } + }); + assert!(allocations.peak_mem() < 64 * 1024); + allocations.result().unwrap(); + } } #[test] diff --git a/tests/precount.rs b/tests/precount.rs index bdc4cdc5..389119e3 100644 --- a/tests/precount.rs +++ b/tests/precount.rs @@ -4,7 +4,7 @@ use serde::de::DeserializeSeed; fn parse(s: &str, counted: bool) -> Result { let mut de = serde_json::Deserializer::from_str(s); let value = if counted { - IValueDeserSeed::new(None).deserialize_with_object_hints(&mut de, s)? + IValueDeserSeed::new(None).deserialize_compact_objects(&mut de)? } else { IValueDeserSeed::new(None).deserialize(&mut de)? }; @@ -13,7 +13,7 @@ fn parse(s: &str, counted: bool) -> Result { } #[test] -fn counted_objects_preserve_values_errors_and_capacity() { +fn buffered_objects_preserve_values_errors_and_capacity() { for input in [ r#"{"a":1,"b":2,"c":3,"d":4,"e":5}"#, r#"{"x":[{}, {"nested":{"a":null,"b":true}}],"str":"{[colon:]}"}"#, @@ -68,12 +68,16 @@ fn counted_objects_preserve_values_errors_and_capacity() { } #[test] -fn repeated_keys_release_unused_slots_and_preserve_last_value() { +fn repeated_keys_use_exact_storage_and_preserve_last_value() { // Escaped names compare equal after decoding. Replaced values may themselves - // contain objects, whose counts must still be consumed before the next field. + // contain objects, whose buffers must not interfere with the next field. let input = r#"{"a":{"x":1},"\u0061":{"y":2,"z":3},"next":{"p":4}}"#; let value = parse(input, true).unwrap(); assert_eq!(value, parse(input, false).unwrap()); + assert_eq!( + serde_json::to_string(&value).unwrap(), + serde_json::to_string(&parse(input, false).unwrap()).unwrap() + ); assert_eq!(value.as_object().unwrap().capacity(), 2); assert_eq!(value["a"].as_object().unwrap().capacity(), 2); assert_eq!(value["next"].as_object().unwrap().capacity(), 1); @@ -85,7 +89,7 @@ fn repeated_keys_release_unused_slots_and_preserve_last_value() { } #[test] -fn counted_objects_preserve_typed_arrays() { +fn buffered_objects_preserve_typed_arrays() { let input = r#"{"values":[0.5,1.0,1.5],"nested":[{"x":1,"y":2}]}"#; for fp_type in [ FloatType::F16, @@ -95,9 +99,7 @@ fn counted_objects_preserve_typed_arrays() { ] { let seed = || IValueDeserSeed::new(Some(FPHAConfig::new_with_type(fp_type))); let mut de = serde_json::Deserializer::from_str(input); - let value = seed() - .deserialize_with_object_hints(&mut de, input) - .unwrap(); + let value = seed().deserialize_compact_objects(&mut de).unwrap(); de.end().unwrap(); let ordinary = seed() .deserialize(&mut serde_json::Deserializer::from_str(input)) @@ -116,8 +118,8 @@ fn counted_objects_preserve_typed_arrays() { } #[test] -fn string_skipping_preserves_escape_boundaries() { - // Exercise short strings and positions around SIMD chunk boundaries. +fn buffered_objects_preserve_escaped_strings() { + // Exercise escaped keys and values at varied lengths. for padding in [0, 1, 15, 16, 31, 32, 63, 64, 65, 256, 4096] { for slashes in 0..8 { for suffix in ["", "\"", "\" :{}[] שלום"] { From a7e2eccbff8b394529d1d4c0ac3fd3da9aa00ce3 Mon Sep 17 00:00:00 2001 From: TalBarYakar Date: Mon, 5 Oct 2026 14:59:33 +0300 Subject: [PATCH 04/11] Optimize compact object construction and cover edge cases Keep up to 16 unique fields inline and reuse larger object buffers. Share the final insertion logic without repeating duplicate-key checks. Add coverage for buffer reuse, malformed input, escaped keys, hash collisions, and growth across allocation thresholds. --- Cargo.toml | 2 + src/de.rs | 77 +++++++++++++++----------- src/object.rs | 135 ++++++++++++++++++++++++++++++++++++++++++++-- tests/precount.rs | 89 ++++++++++++++++++++++++++++-- 4 files changed, 264 insertions(+), 39 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ef2183a8..c332a29c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,8 @@ serde_json = "1.0.59" [dependencies] +indexmap = "~2.13" +smallvec = "1" hashbrown = "0.13.2" dashmap = { version = "5.4", features = ["raw-api"] } lazy_static = "1.4.0" diff --git a/src/de.rs b/src/de.rs index 923824c0..8462fda7 100644 --- a/src/de.rs +++ b/src/de.rs @@ -1,5 +1,6 @@ use std::convert::TryFrom; use std::fmt::{self, Formatter}; +use std::marker::PhantomData; use serde::de::{ DeserializeSeed, EnumAccess, Error as SError, Expected, IntoDeserializer, MapAccess, SeqAccess, @@ -38,13 +39,9 @@ impl IValueDeserSeed { } } -// Reused across completed sibling objects during one deserialization. The vector -// preserves insertion order; decoded keys in the index enforce last-value wins. -#[derive(Default)] -struct ObjectBuffer { - fields: Vec<(IString, IValue)>, - indices: hashbrown::HashMap, -} +// IndexMap preserves insertion order and replaces duplicate values. Reuse the +// existing hash builder and retain drained maps for later sibling objects. +type ObjectBuffer = indexmap::IndexMap; impl IValueDeserSeed { /// Deserializes using temporary object buffers, then moves each object's @@ -346,43 +343,59 @@ impl<'de> Visitor<'de> for ObjectVisitor<'_> { V: MapAccess<'de>, { if let Some(buffers) = self.buffers { - let mut buffer = buffers.pop().unwrap_or_default(); - while let Some(key) = visitor.next_key::()? { - let value = visitor.next_value_seed(ValueVisitor { + const INLINE_FIELDS: usize = crate::object::OBJECT_BUFFER_INLINE_CAPACITY; + // Keep up to 16 unique fields inline while parsing. Final objects + // still use their own eight-field threshold for hash tables. + let mut small = smallvec::SmallVec::<[(IString, IValue); INLINE_FIELDS]>::new(); + let mut large: Option = None; + while let Some((key, value)) = visitor.next_entry_seed( + PhantomData::, + ValueVisitor { fpha_config: self.fpha_config, buffers: Some(&mut *buffers), - })?; - if let Some(&index) = buffer.indices.get(&key) { - buffer.fields[index].1 = value; - } else { - buffer.fields.try_reserve(1).map_err(SError::custom)?; + }, + )? { + if large.is_none() { + if let Some((_, previous)) = small.iter_mut().find(|(k, _)| *k == key) { + *previous = value; + continue; + } + if small.len() < INLINE_FIELDS { + small.push((key, value)); + continue; + } + let mut buffer = buffers.pop().unwrap_or_default(); buffer - .indices - .try_reserve(1) - .map_err(|_| SError::custom("Failed to allocate object buffer"))?; - buffer.indices.insert(key.clone(), buffer.fields.len()); - buffer.fields.push((key, value)); + .try_reserve(small.len() + 1) + .map_err(SError::custom)?; + buffer.extend(small.drain(..)); + large = Some(buffer); } + let buffer = large.as_mut().unwrap(); + if buffer.len() == buffer.capacity() && !buffer.contains_key(&key) { + buffer.try_reserve(1).map_err(SError::custom)?; + } + buffer.insert(key, value); } - // Allocate after parsing the fields, using the number of unique keys. - buffer.indices.clear(); - let mut obj = IObject::with_capacity(buffer.fields.len()) - .map_err(|_| SError::custom("Failed to allocate object"))?; - for (key, value) in buffer.fields.drain(..) { - obj.insert(key, value).map_err(SError::custom)?; + // Allocate only for validated, unique fields; move their values. + if let Some(mut buffer) = large { + let obj = IObject::from_unique_entries(&mut buffer).map_err(SError::custom)?; + buffers.try_reserve(1).map_err(SError::custom)?; + buffers.push(buffer); + return Ok(obj); } - buffers.try_reserve(1).map_err(SError::custom)?; - buffers.push(buffer); - return Ok(obj); + return IObject::from_unique_inline_entries(small).map_err(SError::custom); } let mut obj = IObject::with_capacity(visitor.size_hint().unwrap_or(0)) .map_err(|_| SError::custom("Failed to allocate object"))?; - while let Some(k) = visitor.next_key::()? { - let v = visitor.next_value_seed(ValueVisitor { + while let Some((k, v)) = visitor.next_entry_seed( + PhantomData::, + ValueVisitor { fpha_config: self.fpha_config, buffers: None, - })?; + }, + )? { obj.insert(k, v) .map_err(|e| SError::custom(e.to_string()))?; } diff --git a/src/object.rs b/src/object.rs index 403266bd..b8bc1384 100644 --- a/src/object.rs +++ b/src/object.rs @@ -67,6 +67,9 @@ struct KeyValuePair { /// Objects with capacity at or below this threshold store no hash table. const SMALL_OBJECT_THRESHOLD: u32 = 8; +/// Temporary parser storage; independent of the final object's table threshold. +pub(crate) const OBJECT_BUFFER_INLINE_CAPACITY: usize = 16; + /// Whether an object of this capacity carries a hash table. fn has_table(cap: u32) -> bool { cap > SMALL_OBJECT_THRESHOLD @@ -96,7 +99,15 @@ struct SplitHeader<'a> { impl<'a> SplitHeader<'a> { // Returns the *item index* (Ok) in no-table mode, or the *table bucket* (Ok) in table mode. fn find_bucket(&self, key: &IString) -> Result { + self.find_bucket_impl::(key) + } + + // Unique entries still need collision handling, but no equality checks. + fn find_bucket_impl(&self, key: &IString) -> Result { if !has_table(self.cap) { + if !CHECK_DUPLICATES { + return Err(u32::MAX); + } // Small object: linear scan the items array. // No table bucket to report; insertion just appends. return self @@ -121,7 +132,7 @@ impl<'a> SplitHeader<'a> { // If the bucket contains our key, we found the bucket let k = &self.items.get_unchecked(index as usize).key; - if k == key { + if CHECK_DUPLICATES && k == key { return Ok(bucket as u32); } @@ -332,6 +343,16 @@ trait HeaderMut<'a>: ThinMutExt<'a, Header> { self.set_len(res.checked_add(1).expect("object length overflow")); res } + + // SAFETY: there must be capacity for one more entry, and key must be absent. + unsafe fn push_unique(&mut self, key: IString, value: IValue) { + let bucket = self + .split() + .find_bucket_impl::(&key) + .expect_err("unique insertion only finds vacant buckets"); + let index = self.push(key, value); + self.reborrow().split_mut().shift(bucket, index); + } fn clear(&mut self) { // Clear the table for item in self.reborrow().split_mut().table { @@ -683,6 +704,45 @@ impl IObject { } } + /// Moves already-deduplicated inline fields into their final storage. + pub(crate) fn from_unique_inline_entries( + entries: smallvec::SmallVec<[(IString, IValue); OBJECT_BUFFER_INLINE_CAPACITY]>, + ) -> Result { + assert!(entries.len() <= OBJECT_BUFFER_INLINE_CAPACITY); + let mut obj = Self::with_capacity(entries.len())?; + if !entries.is_empty() { + // SAFETY: the object is non-static and has space for every entry. + // The parser has already removed duplicate keys. Objects above + // the storage threshold still need their hash table initialized. + unsafe { + let mut header = obj.header_mut(); + for (key, value) in entries { + header.push_unique(key, value); + } + } + } + Ok(obj) + } + + /// Moves fields whose uniqueness is guaranteed by the source map. + pub(crate) fn from_unique_entries( + entries: &mut indexmap::IndexMap, + ) -> Result { + let mut obj = Self::with_capacity(entries.len())?; + if !entries.is_empty() { + // SAFETY: allocation covers every drained entry; the source map + // guarantees unique keys. Existing probing and shifting preserve + // the final table's collision and lookup invariants. + unsafe { + let mut header = obj.header_mut(); + for (key, value) in entries.drain(..) { + header.push_unique(key, value); + } + } + } + Ok(obj) + } + fn header(&self) -> ThinRef<'_, Header> { unsafe { ThinRef::new(self.0.ptr().cast()) } } @@ -740,10 +800,17 @@ impl IObject { if current_capacity >= desired_capacity { return Ok(()); } - self.resize_internal(cmp::max( + let grown = cmp::max( current_capacity.checked_mul(2).ok_or(AllocError)?, desired_capacity.max(4), - )) + ); + // Exact-size parsed objects can have capacities such as 5 or 7. Keep + // growth table-free while all requested fields still fit a small object. + self.resize_internal(if desired_capacity <= SMALL_OBJECT_THRESHOLD { + grown.min(SMALL_OBJECT_THRESHOLD) + } else { + grown + }) } /// Returns a view of an entry within this object. @@ -1277,6 +1344,68 @@ mod tests { use super::*; use crate::convert::TryCollect; + #[test] + fn unique_entries_preserve_colliding_keys() { + let mut entries = indexmap::IndexMap::default(); + let cap = hash_capacity(20); + // Force collisions across the end of the table to exercise wrapping. + for i in 0.. { + let key = IString::intern(&format!("collision_field_{i}")); + if hash_bucket(&key, cap) >= cap - 2 { + entries.insert(key, IValue::NULL); + } + if entries.len() == 20 { + break; + } + } + let keys: Vec<_> = entries.keys().cloned().collect(); + entries.insert(keys[0].clone(), true.into()); + let mut object = IObject::from_unique_entries(&mut entries).unwrap(); + assert!(entries.is_empty()); + assert_eq!(object.capacity(), 20); + assert_eq!(object.keys().cloned().collect::>(), keys); + for (i, key) in keys.iter().enumerate() { + let expected = if i == 0 { true.into() } else { IValue::NULL }; + assert_eq!(object.get(key.as_str()), Some(&expected)); + } + for key in keys { + assert!(object.remove(key.as_str()).is_some()); + } + assert!(object.is_empty()); + object.insert("another_long_field", IValue::NULL).unwrap(); + assert_eq!(object.get("another_long_field"), Some(&IValue::NULL)); + } + + #[test] + fn unique_inline_entries_move_and_grow() { + // Heap keys exercise string allocation and ownership under Miri. + for len in 0..=OBJECT_BUFFER_INLINE_CAPACITY { + let entries = (0..len) + .map(|i| { + ( + IString::intern(&format!("long_field_name_{i}")), + IValue::NULL, + ) + }) + .collect(); + let mut object = IObject::from_unique_inline_entries(entries).unwrap(); + assert_eq!(object.len() as usize, len); + assert_eq!(object.capacity() as usize, len); + for i in len..17 { + object + .insert(format!("long_field_name_{i}"), IValue::NULL) + .unwrap(); + } + assert_eq!(object.len(), 17); + for i in 0..17 { + assert_eq!( + object.get(format!("long_field_name_{i}").as_str()), + Some(&IValue::NULL) + ); + } + } + } + #[mockalloc::test] fn can_create() { let x = IObject::new(); diff --git a/tests/precount.rs b/tests/precount.rs index 389119e3..2c037899 100644 --- a/tests/precount.rs +++ b/tests/precount.rs @@ -1,9 +1,9 @@ use ijson::{array::ArraySliceRef, FPHAConfig, FloatType, IValue, IValueDeserSeed}; use serde::de::DeserializeSeed; -fn parse(s: &str, counted: bool) -> Result { +fn parse(s: &str, compact: bool) -> Result { let mut de = serde_json::Deserializer::from_str(s); - let value = if counted { + let value = if compact { IValueDeserSeed::new(None).deserialize_compact_objects(&mut de)? } else { IValueDeserSeed::new(None).deserialize(&mut de)? @@ -25,10 +25,10 @@ fn buffered_objects_preserve_values_errors_and_capacity() { "123", "\"hello\"", ] { - let counted = parse(input, true).unwrap(); + let compact = parse(input, true).unwrap(); let ordinary = parse(input, false).unwrap(); assert_eq!( - serde_json::to_value(&counted).unwrap(), + serde_json::to_value(&compact).unwrap(), serde_json::to_value(&ordinary).unwrap() ); } @@ -138,3 +138,84 @@ fn buffered_objects_preserve_escaped_strings() { assert!(parse(input, false).is_err(), "{input}"); } } + +#[test] +fn small_objects_promote_without_changing_order_or_duplicate_values() { + let widths = [0, 1, 8, 9, 12, 16, 17, 20, 31, 32, 33, 8, 16, 17]; + let inputs: Vec<_> = widths + .into_iter() + .map(|n| { + let mut fields: Vec<_> = (0..n).map(|i| format!("\"k{i}\":{i}")).collect(); + if n > 0 { + fields.push(r#""\u006b0":{"replacement":true}"#.to_owned()); + } + format!("{{{}}}", fields.join(",")) + }) + .collect(); + let input = format!("[{}]", inputs.join(",")); + let value = parse(&input, true).unwrap(); + assert_eq!( + serde_json::to_string(&value).unwrap(), + serde_json::to_string(&parse(&input, false).unwrap()).unwrap() + ); + for (i, n) in widths.into_iter().enumerate() { + assert_eq!(value[i].as_object().unwrap().capacity(), n); + } +} + +#[test] +fn exact_small_objects_grow_without_a_premature_hash_table() { + let mut value = parse(r#"{"a":1,"b":2,"c":3,"d":4,"e":5}"#, true).unwrap(); + let object = value.as_object_mut().unwrap(); + for key in ["f", "g", "h"] { + object.insert(key, IValue::NULL).unwrap(); + assert_eq!(object.capacity(), 8); + } + object.insert("i", IValue::NULL).unwrap(); + assert_eq!(object.capacity(), 16); + assert_eq!(object.len(), 9); + assert_eq!(object["a"], IValue::from(1)); + assert!(object["i"].is_null()); +} + +#[test] +fn compact_objects_reuse_large_buffers_and_preserve_errors() { + let fields = (0..33) + .map(|i| format!("\"long_field_{i}\":null")) + .collect::>() + .join(","); + let wide = format!("{{{fields}}}"); + // Replacing a nested value drops its object; its drained buffer can still + // be reused by siblings without retaining any of the old fields. + let input = format!(r#"[{wide},{{"child":{wide},"\u0063hild":{{}},"tail":{wide}}},{wide}]"#); + let value = parse(&input, true).unwrap(); + assert_eq!( + serde_json::to_string(&value).unwrap(), + serde_json::to_string(&parse(&input, false).unwrap()).unwrap() + ); + assert_eq!(value[0].as_object().unwrap().capacity(), 33); + assert_eq!(value[1].as_object().unwrap().capacity(), 2); + assert!(value[1]["child"].as_object().unwrap().is_empty()); + assert_eq!(value[1]["tail"].as_object().unwrap().capacity(), 33); + assert_eq!(value[2].as_object().unwrap().capacity(), 33); + + for suffix in ["", ",}", ",\"bad\":]", r#", "bad":"\uD800"}"#] { + let malformed = format!("[{wide},{{{fields}{suffix}"); + let compact = parse(&malformed, true).unwrap_err(); + let ordinary = parse(&malformed, false).unwrap_err(); + assert_eq!(compact.classify(), ordinary.classify()); + assert_eq!( + (compact.line(), compact.column()), + (ordinary.line(), ordinary.column()) + ); + } + + let input = r#"{"":0,"\u0000":1,"":2,"😀":3,"\ud83d\ude00":4}"#; + let mut de = serde_json::Deserializer::from_reader(input.as_bytes()); + let value = IValueDeserSeed::new(None) + .deserialize_compact_objects(&mut de) + .unwrap(); + de.end().unwrap(); + assert_eq!(value, parse(input, false).unwrap()); + assert_eq!(value.as_object().unwrap().capacity(), 3); +} From a30b821ec38360942f023b315656df4f0f752680 Mon Sep 17 00:00:00 2001 From: TalBarYakar Date: Mon, 5 Oct 2026 14:59:33 +0300 Subject: [PATCH 05/11] Fix optional object-buffer pooling failure handling Do not fail a completed parse when reserving buffer-pool space fails. Move the inline-capacity constant to de.rs and remove the redundant length assertion, preserving the specialized object construction path. --- src/de.rs | 57 ++++++++++++++++++++++++++++++--------------------- src/object.rs | 10 +++------ 2 files changed, 37 insertions(+), 30 deletions(-) diff --git a/src/de.rs b/src/de.rs index 8462fda7..fc93bf42 100644 --- a/src/de.rs +++ b/src/de.rs @@ -39,9 +39,13 @@ impl IValueDeserSeed { } } +/// Temporary parser storage; independent of the final object's table threshold. +pub(crate) const OBJECT_BUFFER_INLINE_CAPACITY: usize = 16; + // IndexMap preserves insertion order and replaces duplicate values. Reuse the // existing hash builder and retain drained maps for later sibling objects. -type ObjectBuffer = indexmap::IndexMap; +pub(crate) type ObjectBuffer = + indexmap::IndexMap; impl IValueDeserSeed { /// Deserializes using temporary object buffers, then moves each object's @@ -343,10 +347,10 @@ impl<'de> Visitor<'de> for ObjectVisitor<'_> { V: MapAccess<'de>, { if let Some(buffers) = self.buffers { - const INLINE_FIELDS: usize = crate::object::OBJECT_BUFFER_INLINE_CAPACITY; // Keep up to 16 unique fields inline while parsing. Final objects // still use their own eight-field threshold for hash tables. - let mut small = smallvec::SmallVec::<[(IString, IValue); INLINE_FIELDS]>::new(); + let mut small = + smallvec::SmallVec::<[(IString, IValue); OBJECT_BUFFER_INLINE_CAPACITY]>::new(); let mut large: Option = None; while let Some((key, value)) = visitor.next_entry_seed( PhantomData::, @@ -355,23 +359,25 @@ impl<'de> Visitor<'de> for ObjectVisitor<'_> { buffers: Some(&mut *buffers), }, )? { - if large.is_none() { - if let Some((_, previous)) = small.iter_mut().find(|(k, _)| *k == key) { - *previous = value; - continue; - } - if small.len() < INLINE_FIELDS { - small.push((key, value)); - continue; + let buffer = match &mut large { + Some(buffer) => buffer, + None => { + if let Some((_, previous)) = small.iter_mut().find(|(k, _)| *k == key) { + *previous = value; + continue; + } + if small.len() < OBJECT_BUFFER_INLINE_CAPACITY { + small.push((key, value)); + continue; + } + let mut buffer = buffers.pop().unwrap_or_default(); + buffer + .try_reserve(small.len() + 1) + .map_err(SError::custom)?; + buffer.extend(small.drain(..)); + large.insert(buffer) } - let mut buffer = buffers.pop().unwrap_or_default(); - buffer - .try_reserve(small.len() + 1) - .map_err(SError::custom)?; - buffer.extend(small.drain(..)); - large = Some(buffer); - } - let buffer = large.as_mut().unwrap(); + }; if buffer.len() == buffer.capacity() && !buffer.contains_key(&key) { buffer.try_reserve(1).map_err(SError::custom)?; } @@ -379,12 +385,17 @@ impl<'de> Visitor<'de> for ObjectVisitor<'_> { } // Allocate only for validated, unique fields; move their values. if let Some(mut buffer) = large { - let obj = IObject::from_unique_entries(&mut buffer).map_err(SError::custom)?; - buffers.try_reserve(1).map_err(SError::custom)?; - buffers.push(buffer); + let obj = IObject::from_unique_entries(&mut buffer) + .map_err(|_| SError::custom("Failed to allocate object"))?; + // Pooling is optional: a failed reservation must not reject + // an object that was already parsed successfully. + if buffers.try_reserve(1).is_ok() { + buffers.push(buffer); + } return Ok(obj); } - return IObject::from_unique_inline_entries(small).map_err(SError::custom); + return IObject::from_unique_inline_entries(small) + .map_err(|_| SError::custom("Failed to allocate object")); } let mut obj = IObject::with_capacity(visitor.size_hint().unwrap_or(0)) diff --git a/src/object.rs b/src/object.rs index b8bc1384..b359c686 100644 --- a/src/object.rs +++ b/src/object.rs @@ -67,9 +67,6 @@ struct KeyValuePair { /// Objects with capacity at or below this threshold store no hash table. const SMALL_OBJECT_THRESHOLD: u32 = 8; -/// Temporary parser storage; independent of the final object's table threshold. -pub(crate) const OBJECT_BUFFER_INLINE_CAPACITY: usize = 16; - /// Whether an object of this capacity carries a hash table. fn has_table(cap: u32) -> bool { cap > SMALL_OBJECT_THRESHOLD @@ -706,9 +703,8 @@ impl IObject { /// Moves already-deduplicated inline fields into their final storage. pub(crate) fn from_unique_inline_entries( - entries: smallvec::SmallVec<[(IString, IValue); OBJECT_BUFFER_INLINE_CAPACITY]>, + entries: smallvec::SmallVec<[(IString, IValue); crate::de::OBJECT_BUFFER_INLINE_CAPACITY]>, ) -> Result { - assert!(entries.len() <= OBJECT_BUFFER_INLINE_CAPACITY); let mut obj = Self::with_capacity(entries.len())?; if !entries.is_empty() { // SAFETY: the object is non-static and has space for every entry. @@ -726,7 +722,7 @@ impl IObject { /// Moves fields whose uniqueness is guaranteed by the source map. pub(crate) fn from_unique_entries( - entries: &mut indexmap::IndexMap, + entries: &mut crate::de::ObjectBuffer, ) -> Result { let mut obj = Self::with_capacity(entries.len())?; if !entries.is_empty() { @@ -1379,7 +1375,7 @@ mod tests { #[test] fn unique_inline_entries_move_and_grow() { // Heap keys exercise string allocation and ownership under Miri. - for len in 0..=OBJECT_BUFFER_INLINE_CAPACITY { + for len in 0..=crate::de::OBJECT_BUFFER_INLINE_CAPACITY { let entries = (0..len) .map(|i| { ( From 5c2016ccff8f078400c4877c7954aa4f1bd380ec Mon Sep 17 00:00:00 2001 From: TalBarYakar Date: Mon, 5 Oct 2026 14:59:33 +0300 Subject: [PATCH 06/11] Reduce object capacity growth from 2x to 1.25x Grow object capacity by a rounded-up quarter to reduce unused space after insertions. Preserve overflow checks, minimum required capacity, and the small-object threshold. Update capacity assertions for the new growth policy. --- src/object.rs | 4 +++- tests/precount.rs | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/object.rs b/src/object.rs index b359c686..ff2b6c88 100644 --- a/src/object.rs +++ b/src/object.rs @@ -797,7 +797,9 @@ impl IObject { return Ok(()); } let grown = cmp::max( - current_capacity.checked_mul(2).ok_or(AllocError)?, + current_capacity + .checked_add(current_capacity.div_ceil(4)) + .ok_or(AllocError)?, desired_capacity.max(4), ); // Exact-size parsed objects can have capacities such as 5 or 7. Keep diff --git a/tests/precount.rs b/tests/precount.rs index 2c037899..f9638d35 100644 --- a/tests/precount.rs +++ b/tests/precount.rs @@ -37,7 +37,7 @@ fn buffered_objects_preserve_values_errors_and_capacity() { assert_eq!(value.as_object().unwrap().capacity(), 5); assert_eq!( parse(input, false).unwrap().as_object().unwrap().capacity(), - 8 + 5 ); value .as_object_mut() @@ -167,12 +167,12 @@ fn small_objects_promote_without_changing_order_or_duplicate_values() { fn exact_small_objects_grow_without_a_premature_hash_table() { let mut value = parse(r#"{"a":1,"b":2,"c":3,"d":4,"e":5}"#, true).unwrap(); let object = value.as_object_mut().unwrap(); - for key in ["f", "g", "h"] { + for (key, capacity) in [("f", 7), ("g", 7), ("h", 8)] { object.insert(key, IValue::NULL).unwrap(); - assert_eq!(object.capacity(), 8); + assert_eq!(object.capacity(), capacity); } object.insert("i", IValue::NULL).unwrap(); - assert_eq!(object.capacity(), 16); + assert_eq!(object.capacity(), 10); assert_eq!(object.len(), 9); assert_eq!(object["a"], IValue::from(1)); assert!(object["i"].is_null()); From 7388d79b4d354d520da4fee99d30cd7d08c7868d Mon Sep 17 00:00:00 2001 From: TalBarYakar Date: Mon, 5 Oct 2026 18:00:06 +0300 Subject: [PATCH 07/11] Moved deserialize_compact_objects into the existing impl IValueDeserSeed --- src/de.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/de.rs b/src/de.rs index fc93bf42..ea638467 100644 --- a/src/de.rs +++ b/src/de.rs @@ -37,17 +37,7 @@ impl IValueDeserSeed { pub fn new(fpha_config: Option) -> Self { IValueDeserSeed { fpha_config } } -} - -/// Temporary parser storage; independent of the final object's table threshold. -pub(crate) const OBJECT_BUFFER_INLINE_CAPACITY: usize = 16; - -// IndexMap preserves insertion order and replaces duplicate values. Reuse the -// existing hash builder and retain drained maps for later sibling objects. -pub(crate) type ObjectBuffer = - indexmap::IndexMap; -impl IValueDeserSeed { /// Deserializes using temporary object buffers, then moves each object's /// unique fields into exactly sized storage. Arrays keep normal growth. /// The caller must still check for trailing input with the deserializer. @@ -63,6 +53,14 @@ impl IValueDeserSeed { } } +/// Temporary parser storage; independent of the final object's table threshold. +pub(crate) const OBJECT_BUFFER_INLINE_CAPACITY: usize = 16; + +// IndexMap preserves insertion order and replaces duplicate values. Reuse the +// existing hash builder and retain drained maps for later sibling objects. +pub(crate) type ObjectBuffer = + indexmap::IndexMap; + impl<'de> DeserializeSeed<'de> for IValueDeserSeed { type Value = IValue; From e99f9d6cf236db71b2b7fad96dc05d484e4b37fd Mon Sep 17 00:00:00 2001 From: TalBarYakar Date: Mon, 5 Oct 2026 18:02:11 +0300 Subject: [PATCH 08/11] add comment to explain the ValueVisitor --- src/de.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/de.rs b/src/de.rs index ea638467..72f8481a 100644 --- a/src/de.rs +++ b/src/de.rs @@ -126,6 +126,9 @@ impl<'de> Deserialize<'de> for IObject { struct ValueVisitor<'a> { fpha_config: Option, + // Pool of drained maps that retain capacity for reuse while parsing objects. + // Maps are popped when needed and returned after use; indices do not identify + // JSON objects or nesting levels. None selects ordinary deserialization. buffers: Option<&'a mut Vec>, } From 3df06cfd1446847a6482792225cfcf93c445e272 Mon Sep 17 00:00:00 2001 From: TalBarYakar Date: Mon, 5 Oct 2026 18:07:38 +0300 Subject: [PATCH 09/11] rename and comments things so the code become clearer --- src/de.rs | 70 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/src/de.rs b/src/de.rs index 72f8481a..15ff79ad 100644 --- a/src/de.rs +++ b/src/de.rs @@ -343,75 +343,85 @@ impl<'de> Visitor<'de> for ObjectVisitor<'_> { formatter.write_str("JSON object") } - fn visit_map(self, mut visitor: V) -> Result + fn visit_map(self, mut map_access: V) -> Result where V: MapAccess<'de>, { - if let Some(buffers) = self.buffers { + if let Some(buffer_pool) = self.buffers { // Keep up to 16 unique fields inline while parsing. Final objects // still use their own eight-field threshold for hash tables. - let mut small = + let mut inline_entries = smallvec::SmallVec::<[(IString, IValue); OBJECT_BUFFER_INLINE_CAPACITY]>::new(); - let mut large: Option = None; - while let Some((key, value)) = visitor.next_entry_seed( + let mut indexed_entries: Option = None; + // Nested values reuse the same pool, but each active object owns its entries. + while let Some((key, value)) = map_access.next_entry_seed( PhantomData::, ValueVisitor { fpha_config: self.fpha_config, - buffers: Some(&mut *buffers), + buffers: Some(&mut *buffer_pool), }, )? { - let buffer = match &mut large { - Some(buffer) => buffer, + let entries_map = match &mut indexed_entries { + Some(entries_map) => entries_map, None => { - if let Some((_, previous)) = small.iter_mut().find(|(k, _)| *k == key) { - *previous = value; + // Duplicate keys replace their value without consuming another slot. + if let Some((_, existing_value)) = inline_entries + .iter_mut() + .find(|(existing_key, _)| *existing_key == key) + { + *existing_value = value; continue; } - if small.len() < OBJECT_BUFFER_INLINE_CAPACITY { - small.push((key, value)); + if inline_entries.len() < OBJECT_BUFFER_INLINE_CAPACITY { + inline_entries.push((key, value)); continue; } - let mut buffer = buffers.pop().unwrap_or_default(); - buffer - .try_reserve(small.len() + 1) + // The next unique field exceeds inline storage: move to a pooled map. + let mut entries_map = buffer_pool.pop().unwrap_or_default(); + entries_map + .try_reserve(inline_entries.len() + 1) .map_err(SError::custom)?; - buffer.extend(small.drain(..)); - large.insert(buffer) + entries_map.extend(inline_entries.drain(..)); + indexed_entries.insert(entries_map) } }; - if buffer.len() == buffer.capacity() && !buffer.contains_key(&key) { - buffer.try_reserve(1).map_err(SError::custom)?; + // Reserve only for new keys; inserting a duplicate replaces its value. + if entries_map.len() == entries_map.capacity() && !entries_map.contains_key(&key) { + entries_map.try_reserve(1).map_err(SError::custom)?; } - buffer.insert(key, value); + entries_map.insert(key, value); } // Allocate only for validated, unique fields; move their values. - if let Some(mut buffer) = large { - let obj = IObject::from_unique_entries(&mut buffer) + if let Some(mut entries_map) = indexed_entries { + let object = IObject::from_unique_entries(&mut entries_map) .map_err(|_| SError::custom("Failed to allocate object"))?; + // Construction drains the map; retain its capacity for another object. // Pooling is optional: a failed reservation must not reject // an object that was already parsed successfully. - if buffers.try_reserve(1).is_ok() { - buffers.push(buffer); + if buffer_pool.try_reserve(1).is_ok() { + buffer_pool.push(entries_map); } - return Ok(obj); + return Ok(object); } - return IObject::from_unique_inline_entries(small) + return IObject::from_unique_inline_entries(inline_entries) .map_err(|_| SError::custom("Failed to allocate object")); } - let mut obj = IObject::with_capacity(visitor.size_hint().unwrap_or(0)) + // Ordinary deserialization inserts directly into a growing object. + let mut object = IObject::with_capacity(map_access.size_hint().unwrap_or(0)) .map_err(|_| SError::custom("Failed to allocate object"))?; - while let Some((k, v)) = visitor.next_entry_seed( + while let Some((key, value)) = map_access.next_entry_seed( PhantomData::, ValueVisitor { fpha_config: self.fpha_config, buffers: None, }, )? { - obj.insert(k, v) + object + .insert(key, value) .map_err(|e| SError::custom(e.to_string()))?; } - Ok(obj) + Ok(object) } } From 1ba5244afbed80a76336cd5b0b5009c9f5a92c53 Mon Sep 17 00:00:00 2001 From: TalBarYakar Date: Mon, 5 Oct 2026 18:11:55 +0300 Subject: [PATCH 10/11] Added the InlineObjectEntries type alias and reused it in both places. --- src/de.rs | 6 ++++-- src/object.rs | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/de.rs b/src/de.rs index 15ff79ad..86acc10f 100644 --- a/src/de.rs +++ b/src/de.rs @@ -56,6 +56,9 @@ impl IValueDeserSeed { /// Temporary parser storage; independent of the final object's table threshold. pub(crate) const OBJECT_BUFFER_INLINE_CAPACITY: usize = 16; +pub(crate) type InlineObjectEntries = + smallvec::SmallVec<[(IString, IValue); OBJECT_BUFFER_INLINE_CAPACITY]>; + // IndexMap preserves insertion order and replaces duplicate values. Reuse the // existing hash builder and retain drained maps for later sibling objects. pub(crate) type ObjectBuffer = @@ -350,8 +353,7 @@ impl<'de> Visitor<'de> for ObjectVisitor<'_> { if let Some(buffer_pool) = self.buffers { // Keep up to 16 unique fields inline while parsing. Final objects // still use their own eight-field threshold for hash tables. - let mut inline_entries = - smallvec::SmallVec::<[(IString, IValue); OBJECT_BUFFER_INLINE_CAPACITY]>::new(); + let mut inline_entries = InlineObjectEntries::new(); let mut indexed_entries: Option = None; // Nested values reuse the same pool, but each active object owns its entries. while let Some((key, value)) = map_access.next_entry_seed( diff --git a/src/object.rs b/src/object.rs index ff2b6c88..36ca261e 100644 --- a/src/object.rs +++ b/src/object.rs @@ -703,7 +703,7 @@ impl IObject { /// Moves already-deduplicated inline fields into their final storage. pub(crate) fn from_unique_inline_entries( - entries: smallvec::SmallVec<[(IString, IValue); crate::de::OBJECT_BUFFER_INLINE_CAPACITY]>, + entries: crate::de::InlineObjectEntries, ) -> Result { let mut obj = Self::with_capacity(entries.len())?; if !entries.is_empty() { From c2d9af0e0a049112f2e27555bc6cf2797cbe5633 Mon Sep 17 00:00:00 2001 From: TalBarYakar Date: Tue, 6 Oct 2026 11:49:53 +0300 Subject: [PATCH 11/11] Avoid growing full objects when replacing existing fields Check for an existing key before reserving space in entry() and entry_or_clone(). Reuse the occupied entry without reallocating; retain normal growth for new keys. Add regression coverage for replacement and insertion across small and large exact-capacity objects. --- src/object.rs | 23 ++++++++++++++++++++++- tests/precount.rs | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/object.rs b/src/object.rs index 36ca261e..1efc6155 100644 --- a/src/object.rs +++ b/src/object.rs @@ -816,9 +816,20 @@ impl IObject { /// # Errors /// Returns an `AllocError` if reserving space for the entry fails. pub fn entry(&mut self, key: impl Into) -> Result, IJsonError> { + let key = key.into(); + // A full object only needs to grow if the key is absent. + if self.len() == self.capacity() { + if let Ok(bucket) = self.header().split().find_bucket(&key) { + return Ok(Entry::Occupied(OccupiedEntry { + // SAFETY: finding an occupied entry proves the object is non-static. + header: unsafe { self.header_mut() }, + bucket, + })); + } + } self.reserve(1)?; // Safety: cannot be static after reserving space - Ok(unsafe { self.header_mut().entry(key.into()) }) + Ok(unsafe { self.header_mut().entry(key) }) } /// Returns a view of an entry within this object, whilst avoiding /// cloning the key if the entry is already occupied. @@ -826,6 +837,16 @@ impl IObject { /// # Errors /// Returns an `AllocError` if reserving space for the entry fails. pub fn entry_or_clone(&mut self, key: &IString) -> Result, IJsonError> { + // Reuse the occupied bucket instead of reserving or probing a second time. + if self.len() == self.capacity() { + if let Ok(bucket) = self.header().split().find_bucket(key) { + return Ok(Entry::Occupied(OccupiedEntry { + // SAFETY: finding an occupied entry proves the object is non-static. + header: unsafe { self.header_mut() }, + bucket, + })); + } + } self.reserve(1)?; // Safety: cannot be static after reserving space Ok(unsafe { self.header_mut().entry_or_clone(key) }) diff --git a/tests/precount.rs b/tests/precount.rs index f9638d35..0051f7f1 100644 --- a/tests/precount.rs +++ b/tests/precount.rs @@ -12,6 +12,53 @@ fn parse(s: &str, compact: bool) -> Result { Ok(value) } +#[test] +fn replacing_compact_object_fields_does_not_grow_capacity() { + for count in [0, 1, 5, 8, 9, 16, 17, 100] { + let fields = (0..count) + .map(|i| format!("\"field{i}\":{i}")) + .collect::>() + .join(","); + for borrowed_key in [false, true] { + let mut value = parse(&format!("{{{fields}}}"), true).unwrap(); + let object = value.as_object_mut().unwrap(); + assert_eq!(object.capacity(), count); + for i in 0..count { + let key = ijson::IString::from(format!("field{i}").as_str()); + let entry = if borrowed_key { + object.entry_or_clone(&key) + } else { + object.entry(key) + }; + entry + .unwrap() + .and_modify(|value| *value = IValue::from(i + 1000)) + .or_insert(IValue::NULL); + assert_eq!(object.len(), count); + assert_eq!(object.capacity(), count); + } + + // A genuinely new field must still reserve space, including on {}. + let key = ijson::IString::from("new_field"); + let entry = if borrowed_key { + object.entry_or_clone(&key) + } else { + object.entry(key) + }; + entry.unwrap().or_insert(IValue::from(true)); + assert_eq!(object.len(), count + 1); + assert!(object.capacity() > count); + assert_eq!(object.get("new_field"), Some(&IValue::from(true))); + for i in 0..count { + assert_eq!( + object.get(format!("field{i}").as_str()), + Some(&IValue::from(i + 1000)) + ); + } + } + } +} + #[test] fn buffered_objects_preserve_values_errors_and_capacity() { for input in [