From 19d4adb39c4033672b7365e460c5952a9ca8c5bc Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 11:38:09 +0100 Subject: [PATCH] Make SVG export def ids linear instead of quadratic Writer::fresh_id probed prefix-1, prefix-2, ... from 1 on every call, formatting and hashing each candidate, so writing N gradients, clip paths or masks cost O(N^2). Ids are never released, so every number below the last one handed out for a prefix is taken: remember where the search for each prefix stopped and resume there. The ids written are the same as before. examples/still-life.vectorcraft (12k gradients, 6.3 MB SVG) exported in about 8.6 s (median); it now takes about 0.15 s. Output is byte-identical for every document in examples/. Add crates/engine/examples/export_bench.rs to time document.export on the example documents, and a test pinning the generated id sequence when object names already take some of the numbers. Co-Authored-By: Claude Opus 5.5 --- crates/engine/examples/export_bench.rs | 47 ++++++++++++++++++++++++++ crates/svg/src/export.rs | 3 ++ crates/svg/src/export/output.rs | 6 ++-- crates/svg/tests/it/ids.rs | 38 ++++++++++++++++++++- 4 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 crates/engine/examples/export_bench.rs diff --git a/crates/engine/examples/export_bench.rs b/crates/engine/examples/export_bench.rs new file mode 100644 index 000000000..62438b2be --- /dev/null +++ b/crates/engine/examples/export_bench.rs @@ -0,0 +1,47 @@ +//! `cargo run --release -p vectorcraft-engine --example export_bench -- [FORMAT] [FILE…]` — wall +//! time of `document.export` (default SVG) for each file (default: the documents in `examples/`). +//! `ITERS` sets the runs per file (default 7); prints the median and the fastest run. +// Example (dev tool): unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md › Robustness). +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use std::path::PathBuf; +use std::time::Instant; + +use serde_json::json; +use vectorcraft_engine::Session; + +fn main() { + let mut args = std::env::args().skip(1); + let format = args.next().unwrap_or_else(|| "svg".into()); + let mut files: Vec = args.map(PathBuf::from).collect(); + if files.is_empty() { + let dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../examples"); + files = std::fs::read_dir(&dir).unwrap().map(|e| e.unwrap().path()).filter(|p| p.extension().is_some_and(|e| e == "vectorcraft")).collect(); + files.sort(); + } + let iters: usize = std::env::var("ITERS").ok().and_then(|v| v.parse().ok()).unwrap_or(7).max(1); + let out_dir = std::env::temp_dir().join(format!("vectorcraft-export-bench-{}", std::process::id())); + std::fs::create_dir_all(&out_dir).unwrap(); + let mut total = 0.0; + for file in &files { + let mut s = Session::new(); + s.execute("document.open", &json!({ "path": file.to_string_lossy() })).unwrap(); + let name = file.file_stem().unwrap().to_string_lossy().into_owned(); + let out = out_dir.join(format!("{name}.{format}")); + let p = json!({ "path": out.to_string_lossy(), "format": format }); + let mut ms: Vec = (0..iters) + .map(|_| { + let t = Instant::now(); + s.execute("document.export", &p).unwrap(); + t.elapsed().as_secs_f64() * 1000.0 + }) + .collect(); + ms.sort_by(f64::total_cmp); + let median = ms[ms.len() / 2]; + total += median; + let bytes = std::fs::metadata(&out).map(|m| m.len()).unwrap_or(0); + println!("{name}.{format}: median {median:.1} ms, min {:.1} ms ({bytes} bytes)", ms[0]); + } + println!("total of medians: {total:.1} ms"); + std::fs::remove_dir_all(&out_dir).ok(); +} diff --git a/crates/svg/src/export.rs b/crates/svg/src/export.rs index 9b64f97dd..fb376a9ba 100644 --- a/crates/svg/src/export.rs +++ b/crates/svg/src/export.rs @@ -66,6 +66,7 @@ fn write(doc: &Document, opts: &ExportOptions, native: Option<&[u8]>, id_prefix: class_ids: HashMap::new(), used_ids: HashSet::new(), next_suffix: HashMap::new(), + next_fresh: HashMap::new(), linked_names: HashSet::new(), names: HashMap::new(), depth: 1, @@ -207,6 +208,8 @@ struct Writer<'a> { used_ids: HashSet, /// The next suffix to try for a base name, so a repeated name doesn't probe the ones taken again. next_suffix: HashMap, + /// The next number [`Writer::fresh_id`] tries for a prefix (every lower one is taken). + next_fresh: HashMap, /// The names in `linked`. linked_names: HashSet, names: HashMap, diff --git a/crates/svg/src/export/output.rs b/crates/svg/src/export/output.rs index 2197b2682..bcbfe4715 100644 --- a/crates/svg/src/export/output.rs +++ b/crates/svg/src/export/output.rs @@ -131,13 +131,13 @@ impl Writer<'_> { } pub(super) fn fresh_id(&mut self, prefix: &str) -> String { - let mut i = 1; + let next = self.next_fresh.entry(prefix.to_owned()).or_insert(1); loop { - let id = format!("{}{prefix}-{i}", self.id_prefix); + let id = format!("{}{prefix}-{next}", self.id_prefix); + *next += 1; if self.used_ids.insert(id.clone()) { return id; } - i += 1; } } diff --git a/crates/svg/tests/it/ids.rs b/crates/svg/tests/it/ids.rs index cbd2e834d..07dc55617 100644 --- a/crates/svg/tests/it/ids.rs +++ b/crates/svg/tests/it/ids.rs @@ -3,7 +3,7 @@ use std::collections::HashSet; -use vectorcraft_color::{Color, Gradient, GradientPaint, Paint}; +use vectorcraft_color::{Color, Gradient, GradientKind, GradientPaint, GradientStop, Paint}; use vectorcraft_doc::{Appearance, Document, Node}; use vectorcraft_geom::{Rect, shapes}; use vectorcraft_svg::{ExportOptions, ObjectIds, Styling, export, import}; @@ -86,6 +86,42 @@ fn unique_ids_never_collide() { assert!(names(&back).contains(&"Logo mark (v2)".to_string())); } +/// Generated def ids count up from 1 and step over numbers an object name already took, also +/// when thousands of them are written. +#[test] +fn generated_ids_count_up_past_taken_names() { + let mut d = Document::new(200.0, 200.0); + let l = d.layers[0].id; + for name in ["linear-gradient-2", "linear-gradient-4"] { + let mut n = Node::path(d.alloc_id(), shapes::rectangle(Rect::new(0.0, 0.0, 5.0, 5.0)), Appearance::default_art()); + n.name = Some(name.into()); + d.insert(Some(l), usize::MAX, n).unwrap(); + } + let count = 3000; + for i in 0..count { + let stops = vec![ + GradientStop::new(0.0, Color::rgb((i % 50) as f32 / 255.0, (i / 50) as f32 / 255.0, 0.0)), + GradientStop::new(1.0, Color::rgb(0.0, 0.0, 1.0)), + ]; + let g = Paint::Gradient(Box::new(GradientPaint::new(Gradient::new(GradientKind::Linear, stops)))); + let n = Node::path(d.alloc_id(), shapes::rectangle(Rect::new(10.0, 10.0, 60.0, 60.0)), Appearance::basic(g, Paint::None, 0.0)); + d.insert(Some(l), usize::MAX, n).unwrap(); + } + for object_ids in [ObjectIds::LayerNames, ObjectIds::Unique] { + let s = export(&d, &ExportOptions { object_ids, ..Default::default() }); + let all = ids(&s); + assert_eq!(all.len(), all.iter().collect::>().len(), "unique ids"); + let prefix = all.iter().find_map(|i| i.find("Layer_1").map(|at| i[..at].to_string())).unwrap(); + let numbers: Vec = + all.iter().filter_map(|i| i.strip_prefix(&format!("{prefix}linear-gradient-"))).map(|n| n.parse().unwrap()).collect(); + assert_eq!(numbers.len(), count + 2, "the two named objects and every gradient"); + let mut gradients: Vec = numbers.into_iter().filter(|n| *n != 2 && *n != 4).collect(); + gradients.sort_unstable(); + let want: Vec = [1, 3].into_iter().chain(5..count + 3).collect(); + assert_eq!(gradients, want); + } +} + #[test] fn label_attributes_name_objects() { let svg = r##"