rustdoc: make intra-doc link pass non-quadratic for repeated links #109876

jsha · 2023-04-03T00:06:40Z

In the collect_intra_doc_links pass, links to a given item that occurred repeatedly were getting inserted into a Vec<clean::ItemLink> repeatedly. This led to n^2 behavior (where n = the number of pages generated), particularly for the intra-doc link on the Into<U> for T where U: From<T> blanket implementation, since that link appears on every single struct page.

Fixes #109851

rustbot · 2023-04-03T00:06:46Z

r? @notriddle

(rustbot has picked a reviewer for you, use r? to override)

Mark-Simulacrum · 2023-04-03T00:10:20Z

src/librustdoc/passes/collect_intra_doc_links.rs

@@ -918,7 +918,10 @@ impl LinkCollector<'_, '_> {
            for md_link in preprocessed_markdown_links(&doc) {
                let link = self.resolve_link(item, item_id, module_id, &doc, &md_link);
                if let Some(link) = link {
-                    self.cx.cache.intra_doc_links.entry(item.item_id).or_default().push(link);
+                    let entry = self.cx.cache.intra_doc_links.entry(item.item_id).or_default();
+                    if entry.iter().find(|other| **other == link).is_none() {


Based on the issue, it seems like this still means we're going to iterate over all the entries -- maybe this should be a FxHashSet or so, instead of a vec?

So, when I try this, tests/rustdoc/intra-doc/prim-precedence.rs starts failing. I haven't quite dug in yet but I suspect it's for a good reason - I think the precedence of primitive links may be relying on an ordering property of the Vec?

In any case if you want to look at the modified commit with FxHashSet it's here: https://github.com/rust-lang/rust/compare/master...jsha:rust:uniquify-intra-doc-link-2?expand=1

Swapping FxHashSet out for FxIndexSet there seems to make tests pass

Aha, perfect! Thanks for the tip.

In the collect_intra_doc_links pass, links to a given item that occurred repeatedly were getting inserted into a Vec<clean::ItemLink> repeatedly. This led to n^2 behavior (where n = the number of pages generated), particularly for the intra-doc link on the `Into<U> for T where U: From<T>` blanket implementation, since that link appears on every single struct page.

rustbot · 2023-04-03T03:46:27Z

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

jsha · 2023-04-03T04:39:22Z

@bors try @rust-timer queue

bors · 2023-04-03T04:39:33Z

⌛ Trying commit d9edb05 with merge 47695e82e5bfde35d1774a9b7a3ced4eefdb990b...

bors · 2023-04-03T06:40:03Z

☀️ Try build successful - checks-actions
Build commit: 47695e82e5bfde35d1774a9b7a3ced4eefdb990b (47695e82e5bfde35d1774a9b7a3ced4eefdb990b)

rust-timer · 2023-04-03T10:58:44Z

Finished benchmarking commit (47695e82e5bfde35d1774a9b7a3ced4eefdb990b): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-11.9%	[-77.7%, -0.7%]	8
Improvements ✅ (secondary)	-3.7%	[-3.7%, -3.7%]	1
All ❌✅ (primary)	-11.9%	[-77.7%, -0.7%]	8

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.7%	[-2.8%, -2.5%]	2
Improvements ✅ (secondary)	-3.9%	[-3.9%, -3.9%]	1
All ❌✅ (primary)	-2.7%	[-2.8%, -2.5%]	2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-15.1%	[-64.9%, -1.5%]	5
Improvements ✅ (secondary)	-4.6%	[-5.1%, -4.1%]	5
All ❌✅ (primary)	-15.1%	[-64.9%, -1.5%]	5

notriddle · 2023-04-03T15:04:49Z

@bors r+

bors · 2023-04-03T15:04:51Z

📌 Commit d9edb05 has been approved by notriddle

It is now in the queue for this repository.

notriddle · 2023-04-03T15:05:08Z

@bors rollup=never

bors · 2023-04-04T01:12:27Z

⌛ Testing commit d9edb05 with merge eb48e97...

bors · 2023-04-04T03:50:03Z

☀️ Test successful - checks-actions
Approved by: notriddle
Pushing eb48e97 to master...

rust-timer · 2023-04-04T09:19:02Z

Finished benchmarking commit (eb48e97): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-11.8%	[-77.7%, -0.6%]	8
Improvements ✅ (secondary)	-3.6%	[-3.6%, -3.6%]	1
All ❌✅ (primary)	-11.8%	[-77.7%, -0.6%]	8

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.1%	[3.1%, 3.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.1%	[-1.1%, -1.1%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-23.4%	[-64.2%, -2.6%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-23.4%	[-64.2%, -2.6%]	3

rustbot assigned notriddle Apr 3, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 3, 2023

Mark-Simulacrum reviewed Apr 3, 2023

View reviewed changes

jyn514 changed the title ~~rustdoc: uniquify intra-doc links~~ rustdoc: make intra-doc link pass non-quadratic for repeated links Apr 3, 2023

jyn514 added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 3, 2023

jsha force-pushed the uniquify-intra-doc branch from a755c57 to d9edb05 Compare April 3, 2023 03:46

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 3, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 3, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 4, 2023

bors merged commit eb48e97 into rust-lang:master Apr 4, 2023

rustbot added this to the 1.70.0 milestone Apr 4, 2023

notriddle mentioned this pull request Apr 4, 2023

rustdoc: use a template to generate Hrefs #109616

Closed

rustdoc: make intra-doc link pass non-quadratic for repeated links #109876

rustdoc: make intra-doc link pass non-quadratic for repeated links #109876

Uh oh!

Conversation

jsha commented Apr 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 3, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Apr 3, 2023

Uh oh!

jsha commented Apr 3, 2023 via email

Uh oh!

This comment has been minimized.

bors commented Apr 3, 2023

Uh oh!

bors commented Apr 3, 2023

Uh oh!

This comment has been minimized.

rust-timer commented Apr 3, 2023

Overall result: ✅ improvements - no action needed

Uh oh!

notriddle commented Apr 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented Apr 3, 2023

Uh oh!

notriddle commented Apr 3, 2023

Uh oh!

bors commented Apr 4, 2023

Uh oh!

bors commented Apr 4, 2023

Uh oh!

rust-timer commented Apr 4, 2023

Overall result: ✅ improvements - no action needed

Uh oh!

Uh oh!

jsha commented Apr 3, 2023 •

edited

Loading

notriddle commented Apr 3, 2023 •

edited

Loading