Skip to content

Commit d012d2f

Browse files
committed
Auto merge of #109399 - petrochenkov:rendersort, r=GuillaumeGomez
rustdoc: Optimize impl sorting during rendering This should fix the perf regression on [bitmaps-3.1.0](https://github.com/rust-lang/rustc-perf/tree/master/collector/compile-benchmarks/bitmaps-3.1.0) from #107765. The bitmaps crate has a lot of impls: ```rust impl Bits for BitsImpl<1> { ... } impl Bits for BitsImpl<2> { ... } // ... impl Bits for BitsImpl<1023> { ... } impl Bits for BitsImpl<1024> { ... } ``` and the logic in `fn print_item` sorts them in natural order. Before #107765 the impls came in source order, which happened to be already sorted in the necessary way. So the comparison function was called fewer times. After #107765 the impls came in "stable" order (based on def path hash). So the comparison function was called more times to sort them. The comparison function was terribly inefficient, so it caused a large perf regression. This PR attempts to make it more efficient by using cached keys during sorting.
2 parents 8be3c2b + 4d55aff commit d012d2f

File tree

1 file changed

+20
-7
lines changed

1 file changed

+20
-7
lines changed

src/librustdoc/html/render/print_item.rs

+20-7
Original file line numberDiff line numberDiff line change
@@ -880,8 +880,8 @@ fn item_trait(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean:
880880
let (mut synthetic, mut concrete): (Vec<&&Impl>, Vec<&&Impl>) =
881881
local.iter().partition(|i| i.inner_impl().kind.is_auto());
882882

883-
synthetic.sort_by(|a, b| compare_impl(a, b, cx));
884-
concrete.sort_by(|a, b| compare_impl(a, b, cx));
883+
synthetic.sort_by_cached_key(|i| ImplString::new(i, cx));
884+
concrete.sort_by_cached_key(|i| ImplString::new(i, cx));
885885

886886
if !foreign.is_empty() {
887887
write_small_section_header(w, "foreign-impls", "Implementations on Foreign Types", "");
@@ -1597,12 +1597,25 @@ where
15971597
w.write_str("</code></pre>");
15981598
}
15991599

1600-
fn compare_impl<'a, 'b>(lhs: &'a &&Impl, rhs: &'b &&Impl, cx: &Context<'_>) -> Ordering {
1601-
let lhss = format!("{}", lhs.inner_impl().print(false, cx));
1602-
let rhss = format!("{}", rhs.inner_impl().print(false, cx));
1600+
#[derive(PartialEq, Eq)]
1601+
struct ImplString(String);
16031602

1604-
// lhs and rhs are formatted as HTML, which may be unnecessary
1605-
compare_names(&lhss, &rhss)
1603+
impl ImplString {
1604+
fn new(i: &Impl, cx: &Context<'_>) -> ImplString {
1605+
ImplString(format!("{}", i.inner_impl().print(false, cx)))
1606+
}
1607+
}
1608+
1609+
impl PartialOrd for ImplString {
1610+
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
1611+
Some(Ord::cmp(self, other))
1612+
}
1613+
}
1614+
1615+
impl Ord for ImplString {
1616+
fn cmp(&self, other: &Self) -> Ordering {
1617+
compare_names(&self.0, &other.0)
1618+
}
16061619
}
16071620

16081621
fn render_implementor(

0 commit comments

Comments
 (0)