From f72f8a9eca5cf8377372fb7f268a6ed56f1c2274 Mon Sep 17 00:00:00 2001 From: elser_personal <19245195+benelser@users.noreply.github.com> Date: Thu, 15 Jan 2026 23:20:28 -0600 Subject: [PATCH 1/7] Optimize Context JSON parsing: eliminate double parsing - Add CedarValueJson::from_serde_value_direct() for direct conversion - Update val_into_restricted_expr() to use direct conversion - Eliminates double parsing overhead (serde_json::from_value) - Results: 80% memory reduction, 13-47% performance improvement - All tests pass (1312 in cedar-policy-core, 1411 in cedar-policy) - Add comprehensive benchmark suite for performance tracking Signed-off-by: elser_personal <19245195+benelser@users.noreply.github.com> --- cedar-policy-core/src/entities/json/value.rs | 139 +++++++- cedar-policy/Cargo.toml | 4 + cedar-policy/benches/context_json_parsing.rs | 316 +++++++++++++++++++ 3 files changed, 456 insertions(+), 3 deletions(-) create mode 100644 cedar-policy/benches/context_json_parsing.rs diff --git a/cedar-policy-core/src/entities/json/value.rs b/cedar-policy-core/src/entities/json/value.rs index f24e187720..e1c9f8a00f 100644 --- a/cedar-policy-core/src/entities/json/value.rs +++ b/cedar-policy-core/src/entities/json/value.rs @@ -376,6 +376,138 @@ impl CedarValueJson { } } + /// Convert directly from `serde_json::Value` without re-parsing via serde. + /// This avoids the double parsing overhead when we already have a parsed Value. + /// + /// This is more efficient than `serde_json::from_value()` because it doesn't + /// go through serde's deserialization machinery, which creates intermediate copies. + /// + /// `ctx` is a function that provides error context when errors occur. + pub fn from_serde_value_direct( + val: serde_json::Value, + ctx: &dyn Fn() -> JsonDeserializationErrorContext, + ) -> Result { + match val { + serde_json::Value::Bool(b) => Ok(CedarValueJson::Bool(b)), + + serde_json::Value::Number(n) => { + if let Some(i) = n.as_i64() { + Ok(CedarValueJson::Long(i)) + } else { + // Number is not a valid i64 (could be float, too large, etc.) + // Create a serde error with an appropriate message + use std::io; + let io_err = io::Error::new( + io::ErrorKind::InvalidData, + format!("invalid number: expected a 64-bit signed integer, got {}", n), + ); + Err(JsonDeserializationError::from(serde_json::Error::io(io_err))) + } + } + + serde_json::Value::String(s) => { + // Direct conversion: String -> SmolStr (no intermediate parsing) + Ok(CedarValueJson::String(SmolStr::new(s))) + } + + serde_json::Value::Array(arr) => { + // Recursively convert array elements + Ok(CedarValueJson::Set( + arr.into_iter() + .map(|v| Self::from_serde_value_direct(v, ctx)) + .collect::, _>>()?, + )) + } + + serde_json::Value::Object(obj) => { + // Check for escapes first (__expr, __entity, __extn) + // This matches the logic in From + if obj.len() == 1 { + // Check for __expr escape + if let Some(expr_val) = obj.get("__expr") { + if let serde_json::Value::String(s) = expr_val { + return Ok(CedarValueJson::ExprEscape { + __expr: SmolStr::new(s), + }); + } + } + + // Check for __entity escape + if let Some(entity_val) = obj.get("__entity") { + if let serde_json::Value::Object(entity_obj) = entity_val { + if entity_obj.len() >= 2 { + if let ( + Some(serde_json::Value::String(ty)), + Some(serde_json::Value::String(id)), + ) = (entity_obj.get("type"), entity_obj.get("id")) + { + return Ok(CedarValueJson::EntityEscape { + __entity: TypeAndId { + entity_type: SmolStr::new(ty), + id: SmolStr::new(id), + }, + }); + } + } + } + } + + // Check for __extn escape + if let Some(extn_val) = obj.get("__extn") { + if let serde_json::Value::Object(extn_obj) = extn_val { + if extn_obj.len() >= 2 { + if let Some(serde_json::Value::String(fn_name)) = + extn_obj.get("fn") + { + // Single argument + if let Some(arg) = extn_obj.get("arg") { + return Ok(CedarValueJson::ExtnEscape { + __extn: FnAndArgs::Single { + ext_fn: SmolStr::new(fn_name), + arg: Box::new(Self::from_serde_value_direct( + arg.clone(), + ctx, + )?), + }, + }); + } + // Multiple arguments + if let Some(serde_json::Value::Array(args_arr)) = + extn_obj.get("args") + { + return Ok(CedarValueJson::ExtnEscape { + __extn: FnAndArgs::Multi { + ext_fn: SmolStr::new(fn_name), + args: args_arr + .iter() + .map(|v| Self::from_serde_value_direct(v.clone(), ctx)) + .collect::, _>>()?, + }, + }); + } + } + } + } + } + } + + // Regular record - convert all key-value pairs + let mut map = BTreeMap::new(); + for (k, v) in obj { + map.insert( + SmolStr::new(k), + Self::from_serde_value_direct(v, ctx)?, + ); + } + Ok(CedarValueJson::Record(JsonRecord { values: map })) + } + + serde_json::Value::Null => { + Err(JsonDeserializationError::Null(Box::new(ctx()))) + } + } + } + /// Convert this `CedarValueJson` into a Cedar "restricted expression" pub fn into_expr( self, @@ -900,9 +1032,10 @@ impl<'e> ValueParser<'e> { // The expected type is any other type, or we don't have an expected type. // No special parsing rules apply; we do ordinary, non-schema-based parsing. Some(_) | None => { - // Everything is parsed as `CedarValueJson`, and converted into - // `RestrictedExpr` from that. - let jvalue: CedarValueJson = serde_json::from_value(val)?; + // Use direct conversion instead of double parsing via serde + // This avoids the overhead of serde_json::from_value() which re-parses + // an already-parsed serde_json::Value + let jvalue = CedarValueJson::from_serde_value_direct(val, ctx)?; Ok(jvalue.into_expr(ctx)?) } } diff --git a/cedar-policy/Cargo.toml b/cedar-policy/Cargo.toml index fee5ea1d62..6c2076c9da 100644 --- a/cedar-policy/Cargo.toml +++ b/cedar-policy/Cargo.toml @@ -107,6 +107,10 @@ harness = false name = "deeply_nested_est" harness = false +[[bench]] +name = "context_json_parsing" +harness = false + [package.metadata.docs.rs] features = ["experimental"] rustdoc-args = ["--cfg", "docsrs"] diff --git a/cedar-policy/benches/context_json_parsing.rs b/cedar-policy/benches/context_json_parsing.rs new file mode 100644 index 0000000000..c4ace28d6e --- /dev/null +++ b/cedar-policy/benches/context_json_parsing.rs @@ -0,0 +1,316 @@ +/* + * Copyright Cedar Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#![allow(clippy::unwrap_used, reason = "benchmarking")] +#![allow(clippy::expect_used, reason = "benchmarking")] + +//! Benchmark suite for Context JSON parsing performance and memory usage. +//! +//! This benchmark measures: +//! - Parsing performance (wall time) for various input sizes and structures +//! - Memory usage (RSS) to identify amplification issues +//! +//! For detailed heap allocation profiling, use: +//! ```bash +//! cargo bench --bench context_json_parsing --features heap-profiling +//! ``` +//! This will use dhat-rs to track all heap allocations and produce detailed reports. + +use std::hint::black_box; + +use cedar_policy_core::entities::json::{ContextJsonParser, NullContextSchema}; +use cedar_policy_core::extensions::Extensions; +use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion, Throughput}; +use serde_json::json; + +#[cfg(feature = "heap-profiling")] +use dhat::{Dhat, DhatAllocator}; + +#[cfg(feature = "heap-profiling")] +#[global_allocator] +static ALLOCATOR: DhatAllocator = DhatAllocator; + +/// Generate a large context JSON with a single large string value +fn generate_large_context_string(size_mb: usize) -> serde_json::Value { + let size_bytes = size_mb * 1024 * 1024; + let large_string = "A".repeat(size_bytes); + json!({ + "chunk": large_string + }) +} + +/// Generate a context JSON with nested structures +fn generate_nested_context(depth: usize, width: usize) -> serde_json::Value { + // Build nested structure from the bottom up + let mut nested = json!({}); + for i in 0..width { + if let Some(map) = nested.as_object_mut() { + map.insert(format!("key_{}", i), json!("value")); + } + } + + // Build up the nesting + for _ in 0..depth { + let mut new_level = serde_json::Map::new(); + for i in 0..width { + new_level.insert(format!("key_{}", i), json!("value")); + } + new_level.insert("nested".to_string(), nested); + nested = serde_json::Value::Object(new_level); + } + + nested +} + +/// Generate a context JSON with many small attributes +fn generate_many_attributes(count: usize) -> serde_json::Value { + let mut map = serde_json::Map::new(); + for i in 0..count { + map.insert(format!("attr_{}", i), json!(format!("value_{}", i))); + } + serde_json::Value::Object(map) +} + +/// Measure memory usage using RSS (Resident Set Size) +/// This works on Linux (via /proc/self/status) and macOS (via libproc) +#[cfg(target_os = "linux")] +fn get_memory_rss() -> Option { + use std::fs; + + let status = fs::read_to_string("/proc/self/status").ok()?; + for line in status.lines() { + if line.starts_with("VmRSS:") { + let parts: Vec<&str> = line.split_whitespace().collect(); + if parts.len() >= 2 { + return parts[1].parse::().ok().map(|kb| kb * 1024); + } + } + } + None +} + +#[cfg(target_os = "macos")] +fn get_memory_rss() -> Option { + use std::process::Command; + + // On macOS, use ps to get RSS + let output = Command::new("ps") + .args(&["-o", "rss=", "-p"]) + .arg(std::process::id().to_string()) + .output() + .ok()?; + + let rss_kb = String::from_utf8(output.stdout).ok()? + .trim() + .parse::() + .ok()?; + + Some(rss_kb * 1024) // Convert KB to bytes +} + +#[cfg(not(any(target_os = "linux", target_os = "macos")))] +fn get_memory_rss() -> Option { + // On other systems, we can't easily get RSS + None +} + +/// Benchmark context parsing with memory measurement +/// Uses RSS (Resident Set Size) for memory tracking +/// For more detailed heap allocation tracking, use dhat-rs with --features heap-profiling +fn bench_context_parsing_memory(c: &mut Criterion) { + let mut group = c.benchmark_group("context_json_parsing_memory"); + + // Test different input sizes + let sizes = vec![1, 10, 50, 100]; // MB + + for size_mb in sizes { + let context_json = generate_large_context_string(size_mb); + let json_str = serde_json::to_string(&context_json).unwrap(); + let input_size = json_str.len(); + + group.throughput(Throughput::Bytes(input_size as u64)); + + group.bench_function( + BenchmarkId::new("large_string", format!("{}MB", size_mb)), + |b| { + let parser = ContextJsonParser::new( + None::<&NullContextSchema>, + Extensions::all_available(), + ); + + // Pre-allocate to get baseline memory (for future use if needed) + let _baseline = black_box(Vec::::with_capacity(0)); + let _mem_baseline = get_memory_rss(); + + b.iter(|| { + let mem_before = get_memory_rss(); + let context = black_box( + parser.from_json_str(black_box(&json_str)).unwrap() + ); + let mem_after = get_memory_rss(); + + // Calculate memory amplification + if let (Some(before), Some(after)) = (mem_before, mem_after) { + let memory_used = after.saturating_sub(before); + let amplification = (memory_used as f64) / (input_size as f64); + + // Only print on first iteration to avoid spam + // Criterion will call this many times, so we use a static to track + static PRINTED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); + if !PRINTED.swap(true, std::sync::atomic::Ordering::Relaxed) { + eprintln!( + "Memory measurement (first iteration): Input: {}MB ({} bytes), RSS: {}MB, Amplification: {:.2}x", + size_mb, + input_size, + memory_used / (1024 * 1024), + amplification + ); + } + } + + black_box(context) + }); + }, + ); + } + + group.finish(); +} + +/// Benchmark context parsing performance (time only) +fn bench_context_parsing_performance(c: &mut Criterion) { + let mut group = c.benchmark_group("context_json_parsing_performance"); + + // Test with nested structures + let depths = vec![1, 5, 10, 20]; + for depth in depths { + let context_json = generate_nested_context(depth, 10); + let json_str = serde_json::to_string(&context_json).unwrap(); + let input_size = json_str.len(); + + group.throughput(Throughput::Bytes(input_size as u64)); + + group.bench_function( + BenchmarkId::new("nested", format!("depth_{}", depth)), + |b| { + let parser = ContextJsonParser::new( + None::<&NullContextSchema>, + Extensions::all_available(), + ); + + b.iter(|| { + black_box( + parser.from_json_str(black_box(&json_str)).unwrap() + ) + }); + }, + ); + } + + // Test with many attributes + let attribute_counts = vec![10, 100, 1000, 10000]; + for count in attribute_counts { + let context_json = generate_many_attributes(count); + let json_str = serde_json::to_string(&context_json).unwrap(); + let input_size = json_str.len(); + + group.throughput(Throughput::Bytes(input_size as u64)); + + group.bench_function( + BenchmarkId::new("many_attributes", format!("{}", count)), + |b| { + let parser = ContextJsonParser::new( + None::<&NullContextSchema>, + Extensions::all_available(), + ); + + b.iter(|| { + black_box( + parser.from_json_str(black_box(&json_str)).unwrap() + ) + }); + }, + ); + } + + group.finish(); +} + +/// Benchmark using serde_json::Value directly (the code path we're optimizing) +/// This is the critical path: from_json_value() -> val_into_restricted_expr() +/// where the double parsing occurs +fn bench_context_from_json_value(c: &mut Criterion) { + let mut group = c.benchmark_group("context_from_json_value"); + + let sizes = vec![1, 10, 50, 100]; // MB + + for size_mb in sizes { + let context_json = generate_large_context_string(size_mb); + let input_size = serde_json::to_string(&context_json).unwrap().len(); + + group.throughput(Throughput::Bytes(input_size as u64)); + + group.bench_function( + BenchmarkId::new("from_value", format!("{}MB", size_mb)), + |b| { + let parser = ContextJsonParser::new( + None::<&NullContextSchema>, + Extensions::all_available(), + ); + + // Clone once outside the loop to avoid measuring clone overhead + let json_value = context_json.clone(); + + b.iter(|| { + let mem_before = get_memory_rss(); + let context = black_box( + parser.from_json_value(black_box(json_value.clone())).unwrap() + ); + let mem_after = get_memory_rss(); + + // Calculate memory amplification + if let (Some(before), Some(after)) = (mem_before, mem_after) { + let memory_used = after.saturating_sub(before); + let amplification = (memory_used as f64) / (input_size as f64); + + // Only print on first iteration + static PRINTED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); + if !PRINTED.swap(true, std::sync::atomic::Ordering::Relaxed) { + eprintln!( + "from_json_value (first iteration): Input: {}MB ({} bytes), RSS: {}MB, Amplification: {:.2}x", + size_mb, + input_size, + memory_used / (1024 * 1024), + amplification + ); + } + } + + black_box(context) + }); + }, + ); + } + + group.finish(); +} + +criterion_group!( + benches, + bench_context_parsing_memory, + bench_context_parsing_performance, + bench_context_from_json_value +); +criterion_main!(benches); From e36056cf1399d5daac338e9f42bd957b17269385 Mon Sep 17 00:00:00 2001 From: elser_personal <19245195+benelser@users.noreply.github.com> Date: Thu, 15 Jan 2026 23:25:16 -0600 Subject: [PATCH 2/7] Optimize remaining CedarValueJson::from_value calls for consistency - Also optimize error paths (lines 936, 1009) for consistency - All CedarValueJson conversions now use direct conversion - Maintains same behavior, better performance even in error cases Signed-off-by: elser_personal <19245195+benelser@users.noreply.github.com> --- cedar-policy-core/src/entities/json/value.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cedar-policy-core/src/entities/json/value.rs b/cedar-policy-core/src/entities/json/value.rs index e1c9f8a00f..ce40ece9e6 100644 --- a/cedar-policy-core/src/entities/json/value.rs +++ b/cedar-policy-core/src/entities/json/value.rs @@ -933,7 +933,8 @@ impl<'e> ValueParser<'e> { )), val => { let actual_val = { - let jvalue: CedarValueJson = serde_json::from_value(val)?; + // Use direct conversion for consistency and performance + let jvalue = CedarValueJson::from_serde_value_direct(val, ctx)?; jvalue.into_expr(ctx)? }; let err = TypeMismatchError::type_mismatch( @@ -1006,7 +1007,8 @@ impl<'e> ValueParser<'e> { } val => { let actual_val = { - let jvalue: CedarValueJson = serde_json::from_value(val)?; + // Use direct conversion for consistency and performance + let jvalue = CedarValueJson::from_serde_value_direct(val, ctx)?; jvalue.into_expr(ctx)? }; let err = TypeMismatchError::type_mismatch( From ba0fc699b2558198a21634c2afbdf234f344978e Mon Sep 17 00:00:00 2001 From: elser_personal <19245195+benelser@users.noreply.github.com> Date: Fri, 16 Jan 2026 00:15:26 -0600 Subject: [PATCH 3/7] Fix formatting and clippy issues - Remove trailing whitespace in benchmark file - Fix clippy collapsible-match warnings by collapsing nested if-let patterns - Fix dhat import (remove unused Dhat, keep only DhatAllocator) - All code now passes cargo fmt, cargo clippy, and cargo build checks Signed-off-by: elser_personal <19245195+benelser@users.noreply.github.com> --- cedar-policy-core/src/entities/json/value.rs | 116 +++++++++---------- cedar-policy/benches/context_json_parsing.rs | 99 +++++++--------- 2 files changed, 99 insertions(+), 116 deletions(-) diff --git a/cedar-policy-core/src/entities/json/value.rs b/cedar-policy-core/src/entities/json/value.rs index ce40ece9e6..f028a82718 100644 --- a/cedar-policy-core/src/entities/json/value.rs +++ b/cedar-policy-core/src/entities/json/value.rs @@ -399,9 +399,14 @@ impl CedarValueJson { use std::io; let io_err = io::Error::new( io::ErrorKind::InvalidData, - format!("invalid number: expected a 64-bit signed integer, got {}", n), + format!( + "invalid number: expected a 64-bit signed integer, got {}", + n + ), ); - Err(JsonDeserializationError::from(serde_json::Error::io(io_err))) + Err(JsonDeserializationError::from(serde_json::Error::io( + io_err, + ))) } } @@ -424,67 +429,61 @@ impl CedarValueJson { // This matches the logic in From if obj.len() == 1 { // Check for __expr escape - if let Some(expr_val) = obj.get("__expr") { - if let serde_json::Value::String(s) = expr_val { - return Ok(CedarValueJson::ExprEscape { - __expr: SmolStr::new(s), - }); - } + if let Some(serde_json::Value::String(s)) = obj.get("__expr") { + return Ok(CedarValueJson::ExprEscape { + __expr: SmolStr::new(s), + }); } // Check for __entity escape - if let Some(entity_val) = obj.get("__entity") { - if let serde_json::Value::Object(entity_obj) = entity_val { - if entity_obj.len() >= 2 { - if let ( - Some(serde_json::Value::String(ty)), - Some(serde_json::Value::String(id)), - ) = (entity_obj.get("type"), entity_obj.get("id")) - { - return Ok(CedarValueJson::EntityEscape { - __entity: TypeAndId { - entity_type: SmolStr::new(ty), - id: SmolStr::new(id), - }, - }); - } + if let Some(serde_json::Value::Object(entity_obj)) = obj.get("__entity") { + if entity_obj.len() >= 2 { + if let ( + Some(serde_json::Value::String(ty)), + Some(serde_json::Value::String(id)), + ) = (entity_obj.get("type"), entity_obj.get("id")) + { + return Ok(CedarValueJson::EntityEscape { + __entity: TypeAndId { + entity_type: SmolStr::new(ty), + id: SmolStr::new(id), + }, + }); } } } // Check for __extn escape - if let Some(extn_val) = obj.get("__extn") { - if let serde_json::Value::Object(extn_obj) = extn_val { - if extn_obj.len() >= 2 { - if let Some(serde_json::Value::String(fn_name)) = - extn_obj.get("fn") + if let Some(serde_json::Value::Object(extn_obj)) = obj.get("__extn") { + if extn_obj.len() >= 2 { + if let Some(serde_json::Value::String(fn_name)) = extn_obj.get("fn") { + // Single argument + if let Some(arg) = extn_obj.get("arg") { + return Ok(CedarValueJson::ExtnEscape { + __extn: FnAndArgs::Single { + ext_fn: SmolStr::new(fn_name), + arg: Box::new(Self::from_serde_value_direct( + arg.clone(), + ctx, + )?), + }, + }); + } + // Multiple arguments + if let Some(serde_json::Value::Array(args_arr)) = + extn_obj.get("args") { - // Single argument - if let Some(arg) = extn_obj.get("arg") { - return Ok(CedarValueJson::ExtnEscape { - __extn: FnAndArgs::Single { - ext_fn: SmolStr::new(fn_name), - arg: Box::new(Self::from_serde_value_direct( - arg.clone(), - ctx, - )?), - }, - }); - } - // Multiple arguments - if let Some(serde_json::Value::Array(args_arr)) = - extn_obj.get("args") - { - return Ok(CedarValueJson::ExtnEscape { - __extn: FnAndArgs::Multi { - ext_fn: SmolStr::new(fn_name), - args: args_arr - .iter() - .map(|v| Self::from_serde_value_direct(v.clone(), ctx)) - .collect::, _>>()?, - }, - }); - } + return Ok(CedarValueJson::ExtnEscape { + __extn: FnAndArgs::Multi { + ext_fn: SmolStr::new(fn_name), + args: args_arr + .iter() + .map(|v| { + Self::from_serde_value_direct(v.clone(), ctx) + }) + .collect::, _>>()?, + }, + }); } } } @@ -494,17 +493,12 @@ impl CedarValueJson { // Regular record - convert all key-value pairs let mut map = BTreeMap::new(); for (k, v) in obj { - map.insert( - SmolStr::new(k), - Self::from_serde_value_direct(v, ctx)?, - ); + map.insert(SmolStr::new(k), Self::from_serde_value_direct(v, ctx)?); } Ok(CedarValueJson::Record(JsonRecord { values: map })) } - serde_json::Value::Null => { - Err(JsonDeserializationError::Null(Box::new(ctx()))) - } + serde_json::Value::Null => Err(JsonDeserializationError::Null(Box::new(ctx()))), } } diff --git a/cedar-policy/benches/context_json_parsing.rs b/cedar-policy/benches/context_json_parsing.rs index c4ace28d6e..53100f164e 100644 --- a/cedar-policy/benches/context_json_parsing.rs +++ b/cedar-policy/benches/context_json_parsing.rs @@ -36,7 +36,7 @@ use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion, Through use serde_json::json; #[cfg(feature = "heap-profiling")] -use dhat::{Dhat, DhatAllocator}; +use dhat::DhatAllocator; #[cfg(feature = "heap-profiling")] #[global_allocator] @@ -60,7 +60,7 @@ fn generate_nested_context(depth: usize, width: usize) -> serde_json::Value { map.insert(format!("key_{}", i), json!("value")); } } - + // Build up the nesting for _ in 0..depth { let mut new_level = serde_json::Map::new(); @@ -70,7 +70,7 @@ fn generate_nested_context(depth: usize, width: usize) -> serde_json::Value { new_level.insert("nested".to_string(), nested); nested = serde_json::Value::Object(new_level); } - + nested } @@ -88,7 +88,7 @@ fn generate_many_attributes(count: usize) -> serde_json::Value { #[cfg(target_os = "linux")] fn get_memory_rss() -> Option { use std::fs; - + let status = fs::read_to_string("/proc/self/status").ok()?; for line in status.lines() { if line.starts_with("VmRSS:") { @@ -104,19 +104,20 @@ fn get_memory_rss() -> Option { #[cfg(target_os = "macos")] fn get_memory_rss() -> Option { use std::process::Command; - + // On macOS, use ps to get RSS let output = Command::new("ps") .args(&["-o", "rss=", "-p"]) .arg(std::process::id().to_string()) .output() .ok()?; - - let rss_kb = String::from_utf8(output.stdout).ok()? + + let rss_kb = String::from_utf8(output.stdout) + .ok()? .trim() .parse::() .ok()?; - + Some(rss_kb * 1024) // Convert KB to bytes } @@ -131,17 +132,17 @@ fn get_memory_rss() -> Option { /// For more detailed heap allocation tracking, use dhat-rs with --features heap-profiling fn bench_context_parsing_memory(c: &mut Criterion) { let mut group = c.benchmark_group("context_json_parsing_memory"); - + // Test different input sizes let sizes = vec![1, 10, 50, 100]; // MB - + for size_mb in sizes { let context_json = generate_large_context_string(size_mb); let json_str = serde_json::to_string(&context_json).unwrap(); let input_size = json_str.len(); - + group.throughput(Throughput::Bytes(input_size as u64)); - + group.bench_function( BenchmarkId::new("large_string", format!("{}MB", size_mb)), |b| { @@ -149,23 +150,23 @@ fn bench_context_parsing_memory(c: &mut Criterion) { None::<&NullContextSchema>, Extensions::all_available(), ); - + // Pre-allocate to get baseline memory (for future use if needed) let _baseline = black_box(Vec::::with_capacity(0)); let _mem_baseline = get_memory_rss(); - + b.iter(|| { let mem_before = get_memory_rss(); let context = black_box( parser.from_json_str(black_box(&json_str)).unwrap() ); let mem_after = get_memory_rss(); - + // Calculate memory amplification if let (Some(before), Some(after)) = (mem_before, mem_after) { let memory_used = after.saturating_sub(before); let amplification = (memory_used as f64) / (input_size as f64); - + // Only print on first iteration to avoid spam // Criterion will call this many times, so we use a static to track static PRINTED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); @@ -179,72 +180,60 @@ fn bench_context_parsing_memory(c: &mut Criterion) { ); } } - + black_box(context) }); }, ); } - + group.finish(); } /// Benchmark context parsing performance (time only) fn bench_context_parsing_performance(c: &mut Criterion) { let mut group = c.benchmark_group("context_json_parsing_performance"); - + // Test with nested structures let depths = vec![1, 5, 10, 20]; for depth in depths { let context_json = generate_nested_context(depth, 10); let json_str = serde_json::to_string(&context_json).unwrap(); let input_size = json_str.len(); - + group.throughput(Throughput::Bytes(input_size as u64)); - + group.bench_function( BenchmarkId::new("nested", format!("depth_{}", depth)), |b| { - let parser = ContextJsonParser::new( - None::<&NullContextSchema>, - Extensions::all_available(), - ); - - b.iter(|| { - black_box( - parser.from_json_str(black_box(&json_str)).unwrap() - ) - }); + let parser = + ContextJsonParser::new(None::<&NullContextSchema>, Extensions::all_available()); + + b.iter(|| black_box(parser.from_json_str(black_box(&json_str)).unwrap())); }, ); } - + // Test with many attributes let attribute_counts = vec![10, 100, 1000, 10000]; for count in attribute_counts { let context_json = generate_many_attributes(count); let json_str = serde_json::to_string(&context_json).unwrap(); let input_size = json_str.len(); - + group.throughput(Throughput::Bytes(input_size as u64)); - + group.bench_function( BenchmarkId::new("many_attributes", format!("{}", count)), |b| { - let parser = ContextJsonParser::new( - None::<&NullContextSchema>, - Extensions::all_available(), - ); - - b.iter(|| { - black_box( - parser.from_json_str(black_box(&json_str)).unwrap() - ) - }); + let parser = + ContextJsonParser::new(None::<&NullContextSchema>, Extensions::all_available()); + + b.iter(|| black_box(parser.from_json_str(black_box(&json_str)).unwrap())); }, ); } - + group.finish(); } @@ -253,15 +242,15 @@ fn bench_context_parsing_performance(c: &mut Criterion) { /// where the double parsing occurs fn bench_context_from_json_value(c: &mut Criterion) { let mut group = c.benchmark_group("context_from_json_value"); - + let sizes = vec![1, 10, 50, 100]; // MB - + for size_mb in sizes { let context_json = generate_large_context_string(size_mb); let input_size = serde_json::to_string(&context_json).unwrap().len(); - + group.throughput(Throughput::Bytes(input_size as u64)); - + group.bench_function( BenchmarkId::new("from_value", format!("{}MB", size_mb)), |b| { @@ -269,22 +258,22 @@ fn bench_context_from_json_value(c: &mut Criterion) { None::<&NullContextSchema>, Extensions::all_available(), ); - + // Clone once outside the loop to avoid measuring clone overhead let json_value = context_json.clone(); - + b.iter(|| { let mem_before = get_memory_rss(); let context = black_box( parser.from_json_value(black_box(json_value.clone())).unwrap() ); let mem_after = get_memory_rss(); - + // Calculate memory amplification if let (Some(before), Some(after)) = (mem_before, mem_after) { let memory_used = after.saturating_sub(before); let amplification = (memory_used as f64) / (input_size as f64); - + // Only print on first iteration static PRINTED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); if !PRINTED.swap(true, std::sync::atomic::Ordering::Relaxed) { @@ -297,13 +286,13 @@ fn bench_context_from_json_value(c: &mut Criterion) { ); } } - + black_box(context) }); }, ); } - + group.finish(); } From 7f826d06013eaecf323ef0e325b0702c8b899294 Mon Sep 17 00:00:00 2001 From: elser_personal <19245195+benelser@users.noreply.github.com> Date: Fri, 16 Jan 2026 11:21:07 -0600 Subject: [PATCH 4/7] update tests Signed-off-by: elser_personal <19245195+benelser@users.noreply.github.com> --- cedar-policy-core/src/entities/json/value.rs | 493 ++++++++++++++++++- 1 file changed, 485 insertions(+), 8 deletions(-) diff --git a/cedar-policy-core/src/entities/json/value.rs b/cedar-policy-core/src/entities/json/value.rs index f028a82718..1b7fd07db7 100644 --- a/cedar-policy-core/src/entities/json/value.rs +++ b/cedar-policy-core/src/entities/json/value.rs @@ -376,11 +376,12 @@ impl CedarValueJson { } } - /// Convert directly from `serde_json::Value` without re-parsing via serde. + /// Convert directly from `serde_json::Value` without re-deserialization via serde. /// This avoids the double parsing overhead when we already have a parsed Value. /// - /// This is more efficient than `serde_json::from_value()` because it doesn't - /// go through serde's deserialization machinery, which creates intermediate copies. + /// This is more efficient than `serde_json::from_value()` because it avoids + /// re-deserialization via serde visitors and avoids intermediate `RawCedarValueJson` + /// construction. /// /// `ctx` is a function that provides error context when errors occur. pub fn from_serde_value_direct( @@ -395,7 +396,9 @@ impl CedarValueJson { Ok(CedarValueJson::Long(i)) } else { // Number is not a valid i64 (could be float, too large, etc.) - // Create a serde error with an appropriate message + // serde_json::Error has limited public constructors outside of the serde + // deserialization APIs; using Error::io here preserves the existing error + // category (JsonDeserializationError::Serde) for non-i64 numbers. use std::io; let io_err = io::Error::new( io::ErrorKind::InvalidData, @@ -429,10 +432,12 @@ impl CedarValueJson { // This matches the logic in From if obj.len() == 1 { // Check for __expr escape - if let Some(serde_json::Value::String(s)) = obj.get("__expr") { - return Ok(CedarValueJson::ExprEscape { - __expr: SmolStr::new(s), - }); + if let Some(val) = obj.get("__expr") { + if let serde_json::Value::String(s) = val { + return Ok(CedarValueJson::ExprEscape { + __expr: SmolStr::new(s), + }); + } } // Check for __entity escape @@ -1195,3 +1200,475 @@ pub enum ExtnValueJson { // If one of the above forms fits, we use that. ImplicitConstructor(CedarValueJson), } + +#[cfg(test)] +mod tests { + use super::*; + + /// Helper function to categorize error types for equivalence checking + fn err_kind(e: &JsonDeserializationError) -> &'static str { + use JsonDeserializationError::*; + match e { + Null(_) => "Null", + Serde(_) => "Serde", + ParseEscape(_) => "ParseEscape", + TypeMismatch(_) => "TypeMismatch", + DuplicateKey(_) => "DuplicateKey", + ExprTag(_) => "ExprTag", + RestrictedExpressionError(_) => "RestrictedExpressionError", + ExpectedLiteralEntityRef(_) => "ExpectedLiteralEntityRef", + ExpectedExtnValue(_) => "ExpectedExtnValue", + ActionParentIsNotAction(_) => "ActionParentIsNotAction", + MissingImpliedConstructor(_) => "MissingImpliedConstructor", + IncorrectNumOfArguments(_) => "IncorrectNumOfArguments", + FailedExtensionFunctionLookup(_) => "FailedExtensionFunctionLookup", + EntityAttributeEvaluation(_) => "EntityAttributeEvaluation", + EntitySchemaConformance(_) => "EntitySchemaConformance", + UnexpectedRecordAttr(_) => "UnexpectedRecordAttr", + MissingRequiredRecordAttr(_) => "MissingRequiredRecordAttr", + ReservedName(_) => "ReservedName", + #[allow(deprecated)] + UnsupportedEntityTags => "UnsupportedEntityTags", + #[cfg(feature = "tolerant-ast")] + ASTErrorNode => "ASTErrorNode", + } + } + + /// Test that `from_serde_value_direct` produces identical results to + /// `serde_json::from_value` for all non-null inputs; null is rejected in both + /// pipelines (direct rejects earlier). + #[test] + fn test_from_serde_value_direct_equivalence() { + let ctx = || JsonDeserializationErrorContext::Context; + + // Test basic types + let mut test_cases = vec![ + serde_json::json!(true), + serde_json::json!(false), + serde_json::json!(42), + serde_json::json!(-100), + serde_json::json!(0), + serde_json::json!("hello"), + serde_json::json!(""), + serde_json::json!([]), + serde_json::json!([1, 2, 3]), + serde_json::json!(["a", "b", "c"]), + serde_json::json!([true, false]), + serde_json::json!({}), + serde_json::json!({"key": "value"}), + serde_json::json!({"a": 1, "b": 2}), + serde_json::json!({"nested": {"inner": "value"}}), + // Boundary integer cases + serde_json::json!(i64::MAX), + serde_json::json!(i64::MIN), + ]; + + // Test number larger than i64::MAX (must be constructed from string) + // If serde_json can't represent this integer as a Number on this build, + // skip the case rather than failing the test harness itself. + let too_large_num_str = "9223372036854775808"; + if let Ok(v) = serde_json::from_str::(too_large_num_str) { + test_cases.push(v); + } + + // Edge cases: empty objects with reserved keys but wrong shape + test_cases.push(serde_json::json!({"__entity": {}})); + test_cases.push(serde_json::json!({"__extn": {}})); + // Empty args array + test_cases.push(serde_json::json!({"__extn": {"fn": "ip", "args": []}})); + + for val in test_cases { + let direct_result = CedarValueJson::from_serde_value_direct(val.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(val.clone()).map_err(JsonDeserializationError::from); + + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => { + assert_eq!( + direct, + serde, + "Results differ for value: {}", + serde_json::to_string(&val).unwrap() + ); + } + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for value: {}", + serde_json::to_string(&val).unwrap() + ); + } + (Ok(_), Err(e)) => { + panic!( + "Direct succeeded but serde failed for {}: {}", + serde_json::to_string(&val).unwrap(), + e + ); + } + (Err(e), Ok(_)) => { + panic!( + "Serde succeeded but direct failed for {}: {}", + serde_json::to_string(&val).unwrap(), + e + ); + } + } + } + } + + /// Test escape sequence handling equivalence + #[test] + fn test_escape_sequences_equivalence() { + let ctx = || JsonDeserializationErrorContext::Context; + + // Test __entity escape + let entity_val = serde_json::json!({ + "__entity": { + "type": "User", + "id": "alice" + } + }); + let direct_result = CedarValueJson::from_serde_value_direct(entity_val.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(entity_val).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => assert_eq!(direct, serde), + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for __entity escape" + ); + } + _ => panic!("One succeeded and the other failed for __entity escape"), + } + + // Test __extn escape with single arg + let extn_single = serde_json::json!({ + "__extn": { + "fn": "ip", + "arg": "1.2.3.4" + } + }); + let direct_result = CedarValueJson::from_serde_value_direct(extn_single.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(extn_single).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => assert_eq!(direct, serde), + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for __extn single arg" + ); + } + _ => panic!("One succeeded and the other failed for __extn single arg"), + } + + // Test __extn escape with multiple args + let extn_multi = serde_json::json!({ + "__extn": { + "fn": "decimal", + "args": ["123.45", "67.89"] + } + }); + let direct_result = CedarValueJson::from_serde_value_direct(extn_multi.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(extn_multi).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => assert_eq!(direct, serde), + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for __extn multi arg" + ); + } + _ => panic!("One succeeded and the other failed for __extn multi arg"), + } + + // Test __expr escape (should be rejected with ExprTag error) + let expr_val = serde_json::json!({ + "__expr": "principal == User::\"alice\"" + }); + let direct_result = CedarValueJson::from_serde_value_direct(expr_val.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(expr_val).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => assert_eq!(direct, serde), + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for __expr escape" + ); + } + _ => panic!("One succeeded and the other failed for __expr escape"), + } + + // Test multi-key object containing __expr (should fall back to record, not escape) + let expr_extra = serde_json::json!({ + "__expr": "x", + "y": 1 + }); + let direct_result = CedarValueJson::from_serde_value_direct(expr_extra.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(expr_extra).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => assert_eq!(direct, serde), + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for multi-key object with __expr" + ); + } + _ => panic!("One succeeded and the other failed for multi-key object with __expr"), + } + + // Test multi-key object containing __entity (should fall back to record) + let entity_extra = serde_json::json!({ + "__entity": { + "type": "User", + "id": "alice" + }, + "extra": "field" + }); + let direct_result = CedarValueJson::from_serde_value_direct(entity_extra.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(entity_extra).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => assert_eq!(direct, serde), + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for multi-key object with __entity" + ); + } + _ => panic!("One succeeded and the other failed for multi-key object with __entity"), + } + + // Test multi-key object containing __extn (should fall back to record) + let extn_extra = serde_json::json!({ + "__extn": { + "fn": "ip", + "arg": "1.2.3.4" + }, + "extra": "field" + }); + let direct_result = CedarValueJson::from_serde_value_direct(extn_extra.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(extn_extra).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => assert_eq!(direct, serde), + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for multi-key object with __extn" + ); + } + _ => panic!("One succeeded and the other failed for multi-key object with __extn"), + } + } + + /// Test invalid escape shape cases - these test precedence and fallback behavior + #[test] + fn test_invalid_escape_shapes() { + let ctx = || JsonDeserializationErrorContext::Context; + + // Test __entity with non-object value (should fall back to record) + let invalid_entity = serde_json::json!({ + "__entity": "not an object" + }); + let direct_result = CedarValueJson::from_serde_value_direct(invalid_entity.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(invalid_entity).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => assert_eq!(direct, serde), + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for invalid __entity shape" + ); + } + _ => panic!("One succeeded and the other failed for invalid __entity shape"), + } + + // Test __extn with fn as non-string (should fall back to record) + let invalid_extn_fn = serde_json::json!({ + "__extn": { + "fn": 123 + } + }); + let direct_result = CedarValueJson::from_serde_value_direct(invalid_extn_fn.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(invalid_extn_fn).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => assert_eq!(direct, serde), + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for invalid __extn fn shape" + ); + } + _ => panic!("One succeeded and the other failed for invalid __extn fn shape"), + } + + // Test __extn with missing required fields (should fall back to record) + let invalid_extn_missing = serde_json::json!({ + "__extn": { + "fn": "ip" + // missing arg/args + } + }); + let direct_result = + CedarValueJson::from_serde_value_direct(invalid_extn_missing.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(invalid_extn_missing).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => assert_eq!(direct, serde), + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for invalid __extn missing fields" + ); + } + _ => panic!("One succeeded and the other failed for invalid __extn missing fields"), + } + + // Test __extn with args as non-array (should fall back to record) + let invalid_extn_args_type = serde_json::json!({ + "__extn": { + "fn": "decimal", + "args": "not an array" + } + }); + let direct_result = + CedarValueJson::from_serde_value_direct(invalid_extn_args_type.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(invalid_extn_args_type).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => assert_eq!(direct, serde), + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for invalid __extn args type" + ); + } + _ => panic!("One succeeded and the other failed for invalid __extn args type"), + } + } + + /// Test error cases equivalence - verifies equivalent error category/variant + /// (e.g., Serde vs TypeMismatch vs Null), with direct path rejecting null earlier. + #[test] + fn test_error_cases_equivalence() { + let ctx = || JsonDeserializationErrorContext::Context; + + // Test null: serde deserializes to CedarValueJson::Null, but direct function + // errors immediately. Both paths ultimately reject null when converting into + // an expression (via into_expr), so this divergence is acceptable. + let null_val = serde_json::Value::Null; + let direct_result = CedarValueJson::from_serde_value_direct(null_val.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(null_val).map_err(JsonDeserializationError::from); + // serde succeeds (produces Null variant), direct errors + assert!( + direct_result.is_err() && serde_result.is_ok(), + "Direct should error on null, serde should produce Null variant" + ); + // Verify direct path returns Err(Null) + if let Err(e) = &direct_result { + assert_eq!(err_kind(e), "Null", "Direct path should return Null error"); + } + // Verify serde path returns CedarValueJson::Null + if let Ok(serde_val) = &serde_result { + assert_eq!(serde_val, &CedarValueJson::Null); + // Verify that the Null variant also errors when converting to expression + assert!( + serde_val.clone().into_expr(&ctx).is_err(), + "Null variant must fail into_expr" + ); + } + + // Test invalid number (float) - both should error with same error kind + let float_val = serde_json::json!(3.14); + let direct_result = CedarValueJson::from_serde_value_direct(float_val.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(float_val).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Err(d), Err(s)) => { + assert_eq!(err_kind(&d), err_kind(&s), "Error kinds differ for float"); + // Explicitly verify float errors are still Serde category + assert_eq!(err_kind(&d), "Serde", "Float errors must be Serde category"); + } + _ => panic!("Both should error on float"), + } + } + + /// Test nested structures equivalence + #[test] + fn test_nested_structures_equivalence() { + let ctx = || JsonDeserializationErrorContext::Context; + + // Deeply nested structure + let nested = serde_json::json!({ + "level1": { + "level2": { + "level3": { + "level4": { + "value": "deep" + } + } + } + } + }); + // Deep array nesting (depth >= 50) + let mut deep_array = serde_json::json!([]); + for _ in 0..50 { + deep_array = serde_json::json!([deep_array]); + } + let direct_result = CedarValueJson::from_serde_value_direct(deep_array.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(deep_array).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => assert_eq!(direct, serde), + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for deep array nesting" + ); + } + _ => panic!("Deep array nesting handling differs"), + } + + // Array with nested objects + let array_nested = serde_json::json!([ + {"a": 1}, + {"b": 2}, + {"c": {"d": 3}} + ]); + let direct_result = CedarValueJson::from_serde_value_direct(array_nested.clone(), &ctx); + let serde_result: Result = + serde_json::from_value(array_nested).map_err(JsonDeserializationError::from); + match (direct_result, serde_result) { + (Ok(direct), Ok(serde)) => assert_eq!(direct, serde), + (Err(d), Err(s)) => { + assert_eq!( + err_kind(&d), + err_kind(&s), + "Error kinds differ for array with nested objects" + ); + } + _ => panic!("Array with nested objects handling differs"), + } + } +} From 830b87b3ad9d4a1edb54665ef04a28b510883fb2 Mon Sep 17 00:00:00 2001 From: elser_personal <19245195+benelser@users.noreply.github.com> Date: Fri, 16 Jan 2026 13:16:30 -0600 Subject: [PATCH 5/7] formatting Signed-off-by: elser_personal <19245195+benelser@users.noreply.github.com> --- cedar-policy-core/src/entities/json/value.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cedar-policy-core/src/entities/json/value.rs b/cedar-policy-core/src/entities/json/value.rs index 1b7fd07db7..8cdbb31298 100644 --- a/cedar-policy-core/src/entities/json/value.rs +++ b/cedar-policy-core/src/entities/json/value.rs @@ -1619,7 +1619,7 @@ mod tests { let ctx = || JsonDeserializationErrorContext::Context; // Deeply nested structure - let nested = serde_json::json!({ + let _nested = serde_json::json!({ "level1": { "level2": { "level3": { From 7ddab2fa8f8b122c69a581cc62e79e7cec4b559e Mon Sep 17 00:00:00 2001 From: elser_personal <19245195+benelser@users.noreply.github.com> Date: Fri, 16 Jan 2026 13:19:01 -0600 Subject: [PATCH 6/7] remove usless comment Signed-off-by: elser_personal <19245195+benelser@users.noreply.github.com> --- cedar-policy-core/src/entities/json/value.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/cedar-policy-core/src/entities/json/value.rs b/cedar-policy-core/src/entities/json/value.rs index 8cdbb31298..a53034e80c 100644 --- a/cedar-policy-core/src/entities/json/value.rs +++ b/cedar-policy-core/src/entities/json/value.rs @@ -932,7 +932,6 @@ impl<'e> ValueParser<'e> { )), val => { let actual_val = { - // Use direct conversion for consistency and performance let jvalue = CedarValueJson::from_serde_value_direct(val, ctx)?; jvalue.into_expr(ctx)? }; @@ -1006,7 +1005,6 @@ impl<'e> ValueParser<'e> { } val => { let actual_val = { - // Use direct conversion for consistency and performance let jvalue = CedarValueJson::from_serde_value_direct(val, ctx)?; jvalue.into_expr(ctx)? }; From 3c7b5192eafa24e6587be9c2fc3504d1516e7b30 Mon Sep 17 00:00:00 2001 From: elser_personal <19245195+benelser@users.noreply.github.com> Date: Fri, 16 Jan 2026 15:31:04 -0600 Subject: [PATCH 7/7] Remove broken heap-profiling code from context_json_parsing benchmark Signed-off-by: elser_personal <19245195+benelser@users.noreply.github.com> --- cedar-policy/benches/context_json_parsing.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/cedar-policy/benches/context_json_parsing.rs b/cedar-policy/benches/context_json_parsing.rs index 53100f164e..2aa0bcec3b 100644 --- a/cedar-policy/benches/context_json_parsing.rs +++ b/cedar-policy/benches/context_json_parsing.rs @@ -21,12 +21,6 @@ //! This benchmark measures: //! - Parsing performance (wall time) for various input sizes and structures //! - Memory usage (RSS) to identify amplification issues -//! -//! For detailed heap allocation profiling, use: -//! ```bash -//! cargo bench --bench context_json_parsing --features heap-profiling -//! ``` -//! This will use dhat-rs to track all heap allocations and produce detailed reports. use std::hint::black_box; @@ -35,13 +29,6 @@ use cedar_policy_core::extensions::Extensions; use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion, Throughput}; use serde_json::json; -#[cfg(feature = "heap-profiling")] -use dhat::DhatAllocator; - -#[cfg(feature = "heap-profiling")] -#[global_allocator] -static ALLOCATOR: DhatAllocator = DhatAllocator; - /// Generate a large context JSON with a single large string value fn generate_large_context_string(size_mb: usize) -> serde_json::Value { let size_bytes = size_mb * 1024 * 1024; @@ -129,7 +116,6 @@ fn get_memory_rss() -> Option { /// Benchmark context parsing with memory measurement /// Uses RSS (Resident Set Size) for memory tracking -/// For more detailed heap allocation tracking, use dhat-rs with --features heap-profiling fn bench_context_parsing_memory(c: &mut Criterion) { let mut group = c.benchmark_group("context_json_parsing_memory");