perf: Optimize error formatting in some cases

Signed-off-by: Dmitry Dygalo <[email protected]>

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Performance
+
+- Optimize error formatting in some cases.
+
 ## [0.25.1] - 2024-10-25
 
 ### Fixed

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
+    ]
+  }
+]

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<Vec<ErrorBenchmark>> =
+    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);
+    }
+}

crates/jsonschema-py/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Performance
+
+- Optimize error formatting in some cases.
+
 ## [0.25.1] - 2024-10-25
 
 ### Changed

crates/jsonschema/Cargo.toml

@@ -59,3 +59,8 @@ name = "jsonschema"
 [[bench]]
 harness = false
 name = "keywords"
+
+[[bench]]
+harness = false
+name = "errors"
+

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);

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<Utf8Error> 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::<Vec<String>>()
-                        .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::<Vec<String>>()
-                        .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::<Vec<String>>()
-                        .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::<Vec<String>>()
-                        .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::<Vec<String>>()
-                    .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),
         }
     }