diff --git a/CHANGELOG.md b/CHANGELOG.md index f5d4c469..ae91b213 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Performance + +- Optimize error formatting in some cases. + ## [0.25.1] - 2024-10-25 ### Fixed diff --git a/crates/benchmark/data/errors.json b/crates/benchmark/data/errors.json new file mode 100644 index 00000000..e8c97b8d --- /dev/null +++ b/crates/benchmark/data/errors.json @@ -0,0 +1,71 @@ +[ + { + "name": "additional_properties", + "schema": { + "type": "object", + "properties": { + "foo": { + "type": "string" + } + }, + "additionalProperties": false + }, + "instance": { + "foo": "bar", + "extra1": "value1", + "extra2": "value2", + "extra3": "value3" + } + }, + { + "name": "type_multiple", + "schema": { + "type": [ + "string", + "number", + "boolean", + "null", + "array" + ] + }, + "instance": { + "type": "object" + } + }, + { + "name": "unevaluated_properties", + "schema": { + "type": "object", + "properties": { + "foo": { + "type": "string" + } + }, + "unevaluatedProperties": false + }, + "instance": { + "foo": "bar", + "extra1": "value1", + "extra2": "value2" + } + }, + { + "name": "unevaluated_items", + "schema": { + "type": "array", + "prefixItems": [ + { + "type": "integer" + } + ], + "unevaluatedItems": false + }, + "instance": [ + 1, + 2, + 3, + 4, + 5 + ] + } +] diff --git a/crates/benchmark/src/lib.rs b/crates/benchmark/src/lib.rs index 96d4f82d..82a50fe0 100644 --- a/crates/benchmark/src/lib.rs +++ b/crates/benchmark/src/lib.rs @@ -147,3 +147,20 @@ pub fn run_keyword_benchmarks(bench: &mut BenchFunc) { } } } + +#[derive(serde::Deserialize)] +pub struct ErrorBenchmark { + pub name: String, + pub schema: Value, + pub instance: Value, +} + +static ERROR_CASES: &[u8] = include_bytes!("../data/errors.json"); +static ERROR_BENCHMARKS: LazyLock> = + LazyLock::new(|| serde_json::from_slice(ERROR_CASES).expect("Invalid JSON")); + +pub fn run_error_formatting_benchmarks(bench_fn: &mut dyn FnMut(&str, &Value, &Value)) { + for case in ERROR_BENCHMARKS.iter() { + bench_fn(&case.name, &case.schema, &case.instance); + } +} diff --git a/crates/jsonschema-py/CHANGELOG.md b/crates/jsonschema-py/CHANGELOG.md index 302a9e89..92f88785 100644 --- a/crates/jsonschema-py/CHANGELOG.md +++ b/crates/jsonschema-py/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Performance + +- Optimize error formatting in some cases. + ## [0.25.1] - 2024-10-25 ### Changed diff --git a/crates/jsonschema/Cargo.toml b/crates/jsonschema/Cargo.toml index 69ac7915..c4efd88c 100644 --- a/crates/jsonschema/Cargo.toml +++ b/crates/jsonschema/Cargo.toml @@ -59,3 +59,8 @@ name = "jsonschema" [[bench]] harness = false name = "keywords" + +[[bench]] +harness = false +name = "errors" + diff --git a/crates/jsonschema/benches/errors.rs b/crates/jsonschema/benches/errors.rs new file mode 100644 index 00000000..5096a168 --- /dev/null +++ b/crates/jsonschema/benches/errors.rs @@ -0,0 +1,27 @@ +use benchmark::run_error_formatting_benchmarks; +use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion}; +use serde_json::Value; + +fn bench_error_formatting(c: &mut Criterion, name: &str, schema: &Value, instance: &Value) { + let validator = jsonschema::validator_for(schema).expect("Valid schema"); + let error = validator + .validate(instance) + .unwrap_err() + .next() + .expect("Should have at least one error"); + + c.bench_with_input( + BenchmarkId::new("error_formatting", name), + &error, + |b, error| b.iter(|| error.to_string()), + ); +} + +fn run_benchmarks(c: &mut Criterion) { + run_error_formatting_benchmarks(&mut |name, schema, instance| { + bench_error_formatting(c, name, schema, instance); + }); +} + +criterion_group!(error_formatting, run_benchmarks); +criterion_main!(error_formatting); diff --git a/crates/jsonschema/src/error.rs b/crates/jsonschema/src/error.rs index a527a274..d472774a 100644 --- a/crates/jsonschema/src/error.rs +++ b/crates/jsonschema/src/error.rs @@ -7,7 +7,7 @@ use serde_json::{Map, Number, Value}; use std::{ borrow::Cow, error, - fmt::{self, Formatter}, + fmt::{self, Formatter, Write}, io, iter::{empty, once}, str::Utf8Error, @@ -776,7 +776,6 @@ impl From for ValidationError<'_> { /// Textual representation of various validation errors. impl fmt::Display for ValidationError<'_> { #[allow(clippy::too_many_lines)] // The function is long but it does formatting only - #[inline] fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match &self.kind { ValidationErrorKind::Schema => f.write_str("Schema error"), @@ -788,51 +787,44 @@ impl fmt::Display for ValidationError<'_> { write!(f, r#"{} is not a "{}""#, self.instance, format) } ValidationErrorKind::AdditionalItems { limit } => { - // It's safe to unwrap here as ValidationErrorKind::AdditionalItems is reported only in - // case of arrays with more items than expected - let extras: Vec<&Value> = self - .instance - .as_array() - .expect("Always valid") - .iter() - .skip(*limit) - .collect(); - let verb = { - if extras.len() == 1 { - "was" - } else { - "were" - } - }; - write!( - f, - "Additional items are not allowed ({} {} unexpected)", - extras - .iter() - .map(ToString::to_string) - .collect::>() - .join(", "), - verb - ) + f.write_str("Additional items are not allowed (")?; + let array = self.instance.as_array().expect("Always valid"); + let mut iter = array.iter().skip(*limit); + + if let Some(item) = iter.next() { + write!(f, "{}", item)?; + } + for item in iter { + f.write_str(", ")?; + write!(f, "{}", item)?; + } + + let items_count = array.len() - limit; + f.write_str(if items_count == 1 { + " was unexpected)" + } else { + " were unexpected)" + }) } ValidationErrorKind::AdditionalProperties { unexpected } => { - let verb = { - if unexpected.len() == 1 { - "was" - } else { - "were" - } - }; - write!( - f, - "Additional properties are not allowed ({} {} unexpected)", - unexpected - .iter() - .map(|x| format!("'{}'", x)) - .collect::>() - .join(", "), - verb - ) + f.write_str("Additional properties are not allowed (")?; + let mut iter = unexpected.iter(); + if let Some(prop) = iter.next() { + f.write_char('\'')?; + write!(f, "{}", prop)?; + f.write_char('\'')?; + } + for prop in iter { + f.write_str(", ")?; + f.write_char('\'')?; + write!(f, "{}", prop)?; + f.write_char('\'')?; + } + f.write_str(if unexpected.len() == 1 { + " was unexpected)" + } else { + " were unexpected)" + }) } ValidationErrorKind::AnyOf => write!( f, @@ -956,42 +948,44 @@ impl fmt::Display for ValidationError<'_> { write!(f, "{} is not a multiple of {}", self.instance, multiple_of) } ValidationErrorKind::UnevaluatedItems { unexpected } => { - let verb = { - if unexpected.len() == 1 { - "was" - } else { - "were" - } - }; - write!( - f, - "Unevaluated items are not allowed ({} {} unexpected)", - unexpected - .iter() - .map(|x| format!("'{}'", x)) - .collect::>() - .join(", "), - verb - ) + f.write_str("Unevaluated items are not allowed (")?; + let mut iter = unexpected.iter(); + if let Some(item) = iter.next() { + f.write_char('\'')?; + write!(f, "{}", item)?; + f.write_char('\'')?; + } + for item in iter { + f.write_str(", ")?; + f.write_char('\'')?; + write!(f, "{}", item)?; + f.write_char('\'')?; + } + f.write_str(if unexpected.len() == 1 { + " was unexpected)" + } else { + " were unexpected)" + }) } ValidationErrorKind::UnevaluatedProperties { unexpected } => { - let verb = { - if unexpected.len() == 1 { - "was" - } else { - "were" - } - }; - write!( - f, - "Unevaluated properties are not allowed ({} {} unexpected)", - unexpected - .iter() - .map(|x| format!("'{}'", x)) - .collect::>() - .join(", "), - verb - ) + f.write_str("Unevaluated properties are not allowed (")?; + let mut iter = unexpected.iter(); + if let Some(prop) = iter.next() { + f.write_char('\'')?; + write!(f, "{}", prop)?; + f.write_char('\'')?; + } + for prop in iter { + f.write_str(", ")?; + f.write_char('\'')?; + write!(f, "{}", prop)?; + f.write_char('\'')?; + } + f.write_str(if unexpected.len() == 1 { + " was unexpected)" + } else { + " were unexpected)" + }) } ValidationErrorKind::UniqueItems => { write!(f, "{} has non-unique elements", self.instance) @@ -1001,16 +995,22 @@ impl fmt::Display for ValidationError<'_> { } => write!(f, r#"{} is not of type "{}""#, self.instance, type_), ValidationErrorKind::Type { kind: TypeKind::Multiple(types), - } => write!( - f, - "{} is not of types {}", - self.instance, - types - .into_iter() - .map(|t| format!(r#""{}""#, t)) - .collect::>() - .join(", ") - ), + } => { + write!(f, "{} is not of types ", self.instance)?; + let mut iter = types.into_iter(); + if let Some(t) = iter.next() { + f.write_char('"')?; + write!(f, "{}", t)?; + f.write_char('"')?; + } + for t in iter { + f.write_str(", ")?; + f.write_char('"')?; + write!(f, "{}", t)?; + f.write_char('"')?; + } + Ok(()) + } ValidationErrorKind::Custom { message } => f.write_str(message), } }