From 1b547a941913b9294225abe295645c686aa35a07 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sun, 13 Sep 2026 23:11:12 -0400 Subject: [PATCH 1/2] perf: reduce allocations when generating and sorting package.json --- src/format_text.rs | 6 +- src/generation/generate.rs | 116 +++++++++++++++++-------------------- src/package_json.rs | 69 +++++++++++++--------- 3 files changed, 96 insertions(+), 95 deletions(-) diff --git a/src/format_text.rs b/src/format_text.rs index b9158a7..f532143 100644 --- a/src/format_text.rs +++ b/src/format_text.rs @@ -110,11 +110,7 @@ fn config_to_print_options(text: &str, config: &Configuration) -> PrintOptions { fn is_jsonc_file(path: &Path, config: &Configuration) -> bool { fn has_jsonc_extension(path: &Path) -> bool { - if let Some(ext) = path.extension() { - return ext.to_string_lossy().to_ascii_lowercase() == "jsonc"; - } - - false + path.extension().is_some_and(|ext| ext.eq_ignore_ascii_case("jsonc")) } fn is_special_json_file(path: &Path, config: &Configuration) -> bool { diff --git a/src/generation/generate.rs b/src/generation/generate.rs index 0cc6b4e..dac2beb 100644 --- a/src/generation/generate.rs +++ b/src/generation/generate.rs @@ -8,7 +8,6 @@ use dprint_core_macros::sc; use jsonc_parser::ast::*; use jsonc_parser::common::Range; use jsonc_parser::common::Ranged; -use jsonc_parser::tokens::TokenAndRange; use std::borrow::Cow; use std::collections::HashSet; use std::fmt::Write; @@ -90,12 +89,13 @@ fn gen_node_with_inner<'a>( let mut items = PrintItems::new(); // get the leading comments - if let Some(comments) = context.comments.get(&node.start()) { + let leading_comments = context.comments.get(&node.start()); + if let Some(comments) = leading_comments { items.extend(gen_comments_as_leading(&node, comments.iter(), context)); } // generate the node - if has_ignore_comment(&node, context) { + if has_ignore_comment(leading_comments, context) { items.push_force_current_line_indentation(); items.extend(inner_gen( ir_helpers::gen_from_raw_string(node.text(context.text)), @@ -120,9 +120,13 @@ fn gen_node_with_inner<'a>( fn gen_node_inner<'a>(node: &Node<'a, 'a>, context: &mut Context<'a, '_>) -> PrintItems { match node { Node::Array(node) => gen_array(node, context), - Node::BooleanLit(node) => node.value.to_string().into(), - Node::NullKeyword(_) => "null".into(), - Node::NumberLit(node) => node.value.to_string().into(), + Node::BooleanLit(node) => sc_items(if node.value { sc!("true") } else { sc!("false") }), + Node::NullKeyword(_) => sc_items(sc!("null")), + Node::NumberLit(node) => { + let mut items = PrintItems::new(); + items.push_str(node.value); + items + } Node::Object(node) => gen_object(node, context), Node::ObjectProp(node) => gen_object_prop(node, context), Node::StringLit(node) => gen_string_lit(node, context), @@ -146,7 +150,7 @@ fn gen_array<'a>(node: &'a Array<'a>, context: &mut Context<'a, '_>) -> PrintIte let mut items = PrintItems::new(); items.extend(gen_comma_separated_values( GenCommaSeparatedValuesOptions { - nodes: node.elements.iter().map(|x| Some(x.into())).collect(), + nodes: node.elements.iter().map(Node::from), prefer_hanging: false, force_use_new_lines: force_multi_lines, allow_blank_lines: true, @@ -186,7 +190,7 @@ fn gen_object<'a>(obj: &'a Object, context: &mut Context<'a, '_>) -> PrintItems let mut items = PrintItems::new(); items.extend(gen_comma_separated_values( GenCommaSeparatedValuesOptions { - nodes: obj.properties.iter().map(|x| Some(Node::ObjectProp(x))).collect(), + nodes: obj.properties.iter().map(Node::ObjectProp), prefer_hanging: false, force_use_new_lines: force_multi_lines, allow_blank_lines: true, @@ -269,6 +273,7 @@ fn gen_dangling_comments<'a: 'b, 'b>(keys: &[usize], context: &mut Context<'a, ' } const DOUBLE_QUOTE_SC: &StringContainer = sc!("\""); +const COMMA_SC: &StringContainer = sc!(","); fn gen_string_lit<'a>(node: &'a StringLit, context: &mut Context<'a, '_>) -> PrintItems { let text = node.text(context.text); @@ -277,10 +282,10 @@ fn gen_string_lit<'a>(node: &'a StringLit, context: &mut Context<'a, '_>) -> Pri let text = &text[1..text.len() - 1]; items.push_sc(DOUBLE_QUOTE_SC); if is_double_quotes { - items.push_string(escape_control_chars(text).into_owned()); + items.push_str(&escape_control_chars(text)); } else { let text = text.replace("\\'", "'").replace('"', "\\\""); - items.push_string(escape_control_chars(&text).into_owned()); + items.push_str(&escape_control_chars(&text)); } items.push_sc(DOUBLE_QUOTE_SC); items @@ -290,13 +295,13 @@ fn gen_word_lit<'a>(node: &'a WordLit<'a>, _: &mut Context<'a, '_>) -> PrintItem // this will be a property name that's not a string literal let mut items = PrintItems::new(); items.push_sc(DOUBLE_QUOTE_SC); - items.push_string(node.value.to_string()); + items.push_str(node.value); items.push_sc(DOUBLE_QUOTE_SC); items } -struct GenCommaSeparatedValuesOptions<'a> { - nodes: Vec>>, +struct GenCommaSeparatedValuesOptions { + nodes: TNodes, prefer_hanging: bool, force_use_new_lines: bool, allow_blank_lines: bool, @@ -308,7 +313,7 @@ struct GenCommaSeparatedValuesOptions<'a> { } fn gen_comma_separated_values<'a>( - opts: GenCommaSeparatedValuesOptions<'a>, + opts: GenCommaSeparatedValuesOptions>>, context: &mut Context<'a, '_>, ) -> PrintItems { let nodes = opts.nodes; @@ -316,18 +321,14 @@ fn gen_comma_separated_values<'a>( let compute_lines_span = opts.allow_blank_lines && opts.force_use_new_lines; // save time otherwise ir_helpers::gen_separated_values( |is_multi_line_or_hanging_ref| { - let mut generated_nodes = Vec::new(); let nodes_count = nodes.len(); - for (i, value) in nodes.into_iter().enumerate() { - let (allow_inline_multi_line, allow_inline_single_line) = if let Some(value) = &value { - (value.kind() == NodeKind::Object, false) - } else { - (false, false) - }; + let mut generated_nodes = Vec::with_capacity(nodes_count); + for (i, value) in nodes.enumerate() { + let allow_inline_multi_line = value.kind() == NodeKind::Object; let lines_span = if compute_lines_span { - value.as_ref().map(|x| ir_helpers::LinesSpan { - start_line: context.start_line_with_comments(x), - end_line: context.end_line_with_comments(x), + Some(ir_helpers::LinesSpan { + start_line: context.start_line_with_comments(&value), + end_line: context.end_line_with_comments(&value), }) } else { None @@ -337,18 +338,15 @@ fn gen_comma_separated_values<'a>( let use_comma_for_last = !is_final_node || match context.config.trailing_commas { TrailingCommaKind::Always => true, - TrailingCommaKind::Maintain => match &value { - Some(value) => context.token_finder.get_next_token_if_comma(&value.range()).is_some(), - None => false, - }, + TrailingCommaKind::Maintain => context.token_finder.get_next_token_if_comma(&value).is_some(), TrailingCommaKind::Jsonc => context.is_jsonc, TrailingCommaKind::Never => false, }; let maybe_comma = if !is_final_node { - ",".into() + sc_items(COMMA_SC) } else if use_comma_for_last { let is_multi_line = is_multi_line_or_hanging_ref.create_resolver(); - if_true_or("is_multi_line", is_multi_line, ",".into(), PrintItems::new()).into() + if_true_or("is_multi_line", is_multi_line, sc_items(COMMA_SC), PrintItems::new()).into() } else { PrintItems::new() }; @@ -358,7 +356,7 @@ fn gen_comma_separated_values<'a>( items, lines_span, allow_inline_multi_line, - allow_inline_single_line, + allow_inline_single_line: false, is_known_multi_line: false, }); } @@ -385,45 +383,33 @@ fn gen_comma_separated_values<'a>( } fn gen_comma_separated_value<'a>( - value: Option>, + element: Node<'a, 'a>, generated_comma: PrintItems, context: &mut Context<'a, '_>, ) -> PrintItems { let mut items = PrintItems::new(); - let comma_token = get_comma_token(&value, context); - - if let Some(element) = value { - let generated_comma = generated_comma.into_rc_path(); - let element_end = element.end(); - let has_comma = comma_token.is_some(); - items.extend(gen_node_with_inner(element, context, move |mut items, context| { - // Own-line comments between the element and its trailing comma would be dropped, so emit them - // here. Only when a comma follows; otherwise they're the closing token's leading comments. - if has_comma { - items.extend(gen_dangling_comments(&[element_end], context)); - } - // this Rc clone is necessary because we can't move the captured generated_comma out of this closure - items.push_optional_path(generated_comma); - items - })); - } else { - items.extend(generated_comma); - } + let comma_token = context.token_finder.get_next_token_if_comma(&element); + + let generated_comma = generated_comma.into_rc_path(); + let element_end = element.end(); + let has_comma = comma_token.is_some(); + items.extend(gen_node_with_inner(element, context, move |mut items, context| { + // Own-line comments between the element and its trailing comma would be dropped, so emit them + // here. Only when a comma follows; otherwise they're the closing token's leading comments. + if has_comma { + items.extend(gen_dangling_comments(&[element_end], context)); + } + // this Rc clone is necessary because we can't move the captured generated_comma out of this closure + items.push_optional_path(generated_comma); + items + })); // get the trailing comments after the comma token if let Some(comma_token) = comma_token { items.extend(gen_trailing_comments(comma_token, context)); } - return items; - - fn get_comma_token<'a, 'b>(element: &Option, context: &mut Context<'a, 'b>) -> Option<&'b TokenAndRange<'a>> { - if let Some(element) = element { - context.token_finder.get_next_token_if_comma(element) - } else { - None - } - } + items } struct GenSurroundedByTokensOptions { @@ -798,8 +784,8 @@ fn gen_comment(comment: &Comment, context: &mut Context) -> Option { }) } -fn has_ignore_comment(node: &dyn Ranged, context: &Context) -> bool { - if let Some(last_comment) = context.comments.get(&(node.start())).and_then(|c| c.last()) { +fn has_ignore_comment(leading_comments: Option<&Rc>>, context: &Context) -> bool { + if let Some(last_comment) = leading_comments.and_then(|c| c.last()) { ir_helpers::text_has_dprint_ignore(last_comment.text(), &context.config.ignore_node_comment_text) } else { false @@ -840,3 +826,9 @@ fn escape_control_chars(text: &str) -> Cow<'_, str> { } Cow::Owned(result) } + +fn sc_items(sc: &'static StringContainer) -> PrintItems { + let mut items = PrintItems::new(); + items.push_sc(sc); + items +} diff --git a/src/package_json.rs b/src/package_json.rs index 87070aa..3dd17d8 100644 --- a/src/package_json.rs +++ b/src/package_json.rs @@ -40,7 +40,7 @@ pub fn apply_conventions<'a>(text: &'a str, config: &Configuration) -> Cow<'a, s // A comment written above a top level property travels with it. A conventional order rearranges // the whole file, so a comment left behind would end up over a property it says nothing about, // and one written above `dependencies` is almost always about those. - root_object.sort_properties().by(compare_top_level_fields); + root_object.sort_properties().by_key(top_level_field_key); for prop in root_object.properties() { let Some(section) = alphabetical_section(&decoded_name(&prop)) else { continue; @@ -49,7 +49,7 @@ pub fn apply_conventions<'a>(text: &'a str, config: &Configuration) -> Cow<'a, s continue; }; match section { - Section::Plain => object.sort_properties().by(compare_properties), + Section::Plain => object.sort_properties().by_key(NpmName::of), Section::Dependencies => sort_dependencies(&object, config), Section::Overrides if !names_a_package_twice(&object) => sort_dependencies(&object, config), Section::Overrides => {} @@ -83,7 +83,7 @@ pub fn apply_conventions<'a>(text: &'a str, config: &Configuration) -> Cow<'a, s /// as a whole. fn sort_dependencies(section: &CstObject, config: &Configuration) { if config.object_prefer_single_line || !opens_on_its_own_line(section) { - section.sort_properties().by(compare_properties); + section.sort_properties().by_key(NpmName::of); return; } @@ -98,12 +98,10 @@ fn sort_dependencies(section: &CstObject, config: &Configuration) { (prop.child_index(), run) }) .collect::>(); - section.sort_properties().pin_comment_headers().by(|left, right| { - runs - .get(&left.child_index()) - .cmp(&runs.get(&right.child_index())) - .then_with(|| compare_properties(left, right)) - }); + section + .sort_properties() + .pin_comment_headers() + .by_key(|prop| (runs.get(&prop.child_index()).copied(), NpmName::of(prop))); } /// Whether the section's first property is written on a line after its open brace, which is what @@ -134,10 +132,11 @@ fn has_heading(prop: &CstObjectProp) -> bool { /// matches, so sorting them could change what gets installed. fn names_a_package_twice(overrides: &CstObject) -> bool { let mut seen = HashSet::new(); - overrides - .properties() - .iter() - .any(|prop| !seen.insert(package_name(&decoded_name(prop)).to_string())) + overrides.properties().iter().any(|prop| { + let mut name = decoded_name(prop); + name.truncate(package_name(&name).len()); + !seen.insert(name) + }) } /// The package a key names without any version written after it, so that `foo@^2` is `foo` and @@ -151,24 +150,38 @@ fn package_name(key: &str) -> &str { } } -fn compare_top_level_fields(left: &CstObjectProp, right: &CstObjectProp) -> Ordering { - let left = decoded_name(left); - let right = decoded_name(right); - match (field_index(&left), field_index(&right)) { - (Some(left), Some(right)) => left.cmp(&right), - (Some(_), None) => Ordering::Less, - (None, Some(_)) => Ordering::Greater, - // a field the conventions don't know goes below the ones they do, in alphabetical order, with - // the underscore prefixed fields npm adds to an installed package (`_id`, `_resolved`, ...) last - (None, None) => left - .starts_with('_') - .cmp(&right.starts_with('_')) - .then_with(|| compare_names(&left, &right)), +/// The sort key of a top level field. +/// +/// Keys are computed once per property rather than on every comparison, which would decode both +/// names each time. +fn top_level_field_key(prop: &CstObjectProp) -> (usize, bool, NpmName) { + let name = decoded_name(prop); + // a field the conventions don't know goes below the ones they do, in alphabetical order, with + // the underscore prefixed fields npm adds to an installed package (`_id`, `_resolved`, ...) last + let index = field_index(&name).unwrap_or(FIELD_ORDER.len()); + (index, name.starts_with('_'), NpmName(name)) +} + +/// A property name that orders the way npm orders names, as described on [`compare_names`]. +#[derive(PartialEq, Eq)] +struct NpmName(String); + +impl NpmName { + fn of(prop: &CstObjectProp) -> Self { + NpmName(decoded_name(prop)) } } -fn compare_properties(left: &CstObjectProp, right: &CstObjectProp) -> Ordering { - compare_names(&decoded_name(left), &decoded_name(right)) +impl Ord for NpmName { + fn cmp(&self, other: &Self) -> Ordering { + compare_names(&self.0, &other.0) + } +} + +impl PartialOrd for NpmName { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } } /// Compares two names the way npm does, so that formatting doesn't undo npm's own sorting. From 6c066a9be0d0613434aa12322004581da5b87ca0 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sun, 13 Sep 2026 23:26:45 -0400 Subject: [PATCH 2/2] perf: skip dangling comment line lookup when there are no comments --- src/generation/generate.rs | 11 ++++++++--- src/package_json.rs | 20 ++++++++------------ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/generation/generate.rs b/src/generation/generate.rs index dac2beb..9280fc0 100644 --- a/src/generation/generate.rs +++ b/src/generation/generate.rs @@ -95,7 +95,7 @@ fn gen_node_with_inner<'a>( } // generate the node - if has_ignore_comment(leading_comments, context) { + if has_ignore_comment(leading_comments.map(|c| c.as_slice()), context) { items.push_force_current_line_indentation(); items.extend(inner_gen( ir_helpers::gen_from_raw_string(node.text(context.text)), @@ -244,6 +244,10 @@ fn gen_dangling_comments<'a: 'b, 'b>(keys: &[usize], context: &mut Context<'a, ' let Some(&after) = keys.first() else { return items; }; + // this runs for every property and element, so avoid computing the line when there's nothing to check + if !keys.iter().any(|key| context.comments.contains_key(key)) { + return items; + } let after_line = context.text_info.line_index(after); let mut dangling: Vec<&'b Comment<'a>> = keys .iter() @@ -784,7 +788,7 @@ fn gen_comment(comment: &Comment, context: &mut Context) -> Option { }) } -fn has_ignore_comment(leading_comments: Option<&Rc>>, context: &Context) -> bool { +fn has_ignore_comment(leading_comments: Option<&[Comment]>, context: &Context) -> bool { if let Some(last_comment) = leading_comments.and_then(|c| c.last()) { ir_helpers::text_has_dprint_ignore(last_comment.text(), &context.config.ignore_node_comment_text) } else { @@ -808,7 +812,8 @@ fn should_break_up_single_line(ranged: &impl Ranged, context: &Context) -> bool /// Escapes control characters (U+0000 through U+001F), which JSON doesn't allow /// unescaped in strings. The parser accepts them and the printer can't handle raw newlines. fn escape_control_chars(text: &str) -> Cow<'_, str> { - if !text.chars().any(|c| c < '\u{20}') { + // checking bytes is enough since every byte of a multi-byte utf-8 char is at least 0x80 + if !text.bytes().any(|b| b < 0x20) { return Cow::Borrowed(text); } diff --git a/src/package_json.rs b/src/package_json.rs index 3dd17d8..9f75bcd 100644 --- a/src/package_json.rs +++ b/src/package_json.rs @@ -101,7 +101,7 @@ fn sort_dependencies(section: &CstObject, config: &Configuration) { section .sort_properties() .pin_comment_headers() - .by_key(|prop| (runs.get(&prop.child_index()).copied(), NpmName::of(prop))); + .by_key(|prop| (runs[&prop.child_index()], NpmName::of(prop))); } /// Whether the section's first property is written on a line after its open brace, which is what @@ -132,11 +132,10 @@ fn has_heading(prop: &CstObjectProp) -> bool { /// matches, so sorting them could change what gets installed. fn names_a_package_twice(overrides: &CstObject) -> bool { let mut seen = HashSet::new(); - overrides.properties().iter().any(|prop| { - let mut name = decoded_name(prop); - name.truncate(package_name(&name).len()); - !seen.insert(name) - }) + overrides + .properties() + .iter() + .any(|prop| !seen.insert(package_name(&decoded_name(prop)).to_string())) } /// The package a key names without any version written after it, so that `foo@^2` is `foo` and @@ -150,14 +149,11 @@ fn package_name(key: &str) -> &str { } } -/// The sort key of a top level field. -/// -/// Keys are computed once per property rather than on every comparison, which would decode both -/// names each time. +/// The key that sorts a top level field into place: the fields the conventions know come first, in +/// their order, and a field they don't know goes below them, in alphabetical order, with the +/// underscore prefixed fields npm adds to an installed package (`_id`, `_resolved`, ...) last. fn top_level_field_key(prop: &CstObjectProp) -> (usize, bool, NpmName) { let name = decoded_name(prop); - // a field the conventions don't know goes below the ones they do, in alphabetical order, with - // the underscore prefixed fields npm adds to an installed package (`_id`, `_resolved`, ...) last let index = field_index(&name).unwrap_or(FIELD_ORDER.len()); (index, name.starts_with('_'), NpmName(name)) }