Skip to content

Commit 1ce45a0

Browse files
committed
Auto merge of #157179 - nnethercote:compare_names, r=GuillaumeGomez
rustdoc: optimize impl sorting Currently rustdoc sorts impls in a couple of places using long HTML string representations of the impls. Using plain text representations instead speeds things up. Details in individual commits.
2 parents 9e293ae + f7c8bc2 commit 1ce45a0

3 files changed

Lines changed: 20 additions & 26 deletions

File tree

src/librustdoc/html/format.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44
//! various types in `rustdoc::clean`.
55
//!
66
//! These implementations all emit HTML. As an internal implementation detail,
7-
//! some of them support an alternate format that emits text, but that should
8-
//! not be used external to this module.
7+
//! some of them support an alternate format that emits plain text.
98
109
use std::cmp::Ordering;
1110
use std::fmt::{self, Display, Write};
@@ -184,9 +183,9 @@ pub(crate) fn print_where_clause(
184183

185184
let clause = if f.alternate() {
186185
if ending == Ending::Newline {
187-
format!(" where{where_preds},")
186+
format!(" where{where_preds:#},")
188187
} else {
189-
format!(" where{where_preds}")
188+
format!(" where{where_preds:#}")
190189
}
191190
} else {
192191
let mut br_with_padding = String::with_capacity(6 * indent + 28);

src/librustdoc/html/render/print_item.rs

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2324,17 +2324,15 @@ where
23242324

23252325
#[derive(PartialEq, Eq)]
23262326
struct ImplString {
2327-
rendered: String,
2328-
is_negative: bool,
2327+
// Plain text (not HTML text) because this is only used for sorting purposes, and the plain
2328+
// text is much shorter and thus faster to compare.
2329+
cmp_text: String,
23292330
}
23302331

23312332
impl ImplString {
23322333
fn new(i: &Impl, cx: &Context<'_>) -> ImplString {
23332334
let impl_ = i.inner_impl();
2334-
ImplString {
2335-
is_negative: impl_.is_negative_trait_impl(),
2336-
rendered: format!("{}", print_impl(impl_, false, cx)),
2337-
}
2335+
ImplString { cmp_text: format!("{:#}", print_impl(impl_, false, cx)) }
23382336
}
23392337
}
23402338

@@ -2346,12 +2344,9 @@ impl PartialOrd for ImplString {
23462344

23472345
impl Ord for ImplString {
23482346
fn cmp(&self, other: &Self) -> Ordering {
2349-
// We sort negative impls first.
2350-
match (self.is_negative, other.is_negative) {
2351-
(false, true) => Ordering::Greater,
2352-
(true, false) => Ordering::Less,
2353-
_ => compare_names(&self.rendered, &other.rendered),
2354-
}
2347+
// Negative impls are naturally sorted first, because `impl !A` is less than `impl B` for
2348+
// any value of `B`, because `!` is less than any identifier-starting char.
2349+
compare_names(&self.cmp_text, &other.cmp_text)
23552350
}
23562351
}
23572352

src/librustdoc/html/render/write_shared.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
//! or contains "invocation-specific".
1515
1616
use std::cell::RefCell;
17-
use std::cmp::Ordering;
1817
use std::ffi::{OsStr, OsString};
1918
use std::fs::File;
2019
use std::io::{self, Write as _};
@@ -730,8 +729,10 @@ impl TraitAliasPart {
730729
None
731730
} else {
732731
let impl_ = imp.inner_impl();
732+
let print = print_impl(impl_, false, cx);
733733
Some(Implementor {
734-
text: print_impl(impl_, false, cx).to_string(),
734+
text: format!("{}", print),
735+
cmp_text: format!("{:#}", print),
735736
synthetic: imp.inner_impl().kind.is_auto(),
736737
types: collect_paths_for_type(&imp.inner_impl().for_, cache),
737738
is_negative: impl_.is_negative_trait_impl(),
@@ -754,14 +755,9 @@ impl TraitAliasPart {
754755
path.push(format!("{remote_item_type}.{}.js", remote_path[remote_path.len() - 1]));
755756

756757
let mut implementors = implementors.collect::<Vec<_>>();
757-
implementors.sort_unstable_by(|a, b| {
758-
// We sort negative impls first.
759-
match (a.is_negative, b.is_negative) {
760-
(false, true) => Ordering::Greater,
761-
(true, false) => Ordering::Less,
762-
_ => compare_names(&a.text, &b.text),
763-
}
764-
});
758+
// Negative impls are naturally sorted first, because `impl !A` is less than `impl B`
759+
// for any value of `B`, because `!` is less than any identifier-starting char.
760+
implementors.sort_unstable_by(|a, b| compare_names(&a.cmp_text, &b.cmp_text));
765761

766762
let part = OrderedJson::array_unsorted(
767763
implementors
@@ -777,7 +773,11 @@ impl TraitAliasPart {
777773
}
778774

779775
struct Implementor {
776+
// HTML text used in generated output.
780777
text: String,
778+
// Plain text used just for sorting output. This is a performance win, because this plain text
779+
// is much shorter than the HTML output and sorting is hot.
780+
cmp_text: String,
781781
synthetic: bool,
782782
types: Vec<String>,
783783
is_negative: bool,

0 commit comments

Comments
 (0)