Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rustdoc: Collect "rustdoc-reachable" items during early doc link resolution #107054

Merged
merged 3 commits into from
Jan 27, 2023
Merged

rustdoc: Collect "rustdoc-reachable" items during early doc link resolution #107054

merged 3 commits into from
Jan 27, 2023

Conversation

petrochenkov
Copy link
Contributor

This pass only needs to know about visibilities, attributes and reexports, so it can be run early, similarly to compute_effective_visibilities in rustc.
Results of this pass can be used to prune the list of extern impls early thus improving performance of #94857.

@rustbot
Copy link
Collaborator

rustbot commented Jan 18, 2023

r? @jsha

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

@rustbot 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 Jan 18, 2023
@petrochenkov
Copy link
Contributor Author

This implementation is preliminary and needs perf-tuning and cleanup.
@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 18, 2023
@bors
Copy link
Contributor

bors commented Jan 18, 2023

⌛ Trying commit dc7c5a6bdb60e23019a5fcc27cd43010b84bdfa6 with merge 5c2a19ad7168382f8bbb7a16230e73c294255bd7...

@petrochenkov petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 18, 2023
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Jan 19, 2023

☀️ Try build successful - checks-actions
Build commit: 5c2a19ad7168382f8bbb7a16230e73c294255bd7 (5c2a19ad7168382f8bbb7a16230e73c294255bd7)

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (5c2a19ad7168382f8bbb7a16230e73c294255bd7): comparison URL.

Overall result: ❌ regressions - 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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@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)		 3.4% [0.9%, 8.3%] 17
Regressions ❌
(secondary)		 5.4% [1.4%, 8.3%] 23
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 3.4% [0.9%, 8.3%] 17

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)		 -1.7% [-2.5%, -0.5%] 12
Improvements ✅
(secondary)		 -2.7% [-5.5%, -1.6%] 21
All ❌✅ (primary) -1.7% [-2.5%, -0.5%] 12

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)		 3.0% [1.0%, 5.3%] 13
Regressions ❌
(secondary)		 4.0% [1.6%, 5.2%] 21
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.3% [-2.3%, -2.3%] 1
All ❌✅ (primary) 3.0% [1.0%, 5.3%] 13

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 19, 2023
@petrochenkov petrochenkov added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 19, 2023
@petrochenkov petrochenkov mentioned this pull request Jan 20, 2023
Merged
@petrochenkov
Copy link
Contributor Author

Blocked on #107136.
@rustbot label +S-blocked

@rustbot rustbot added the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label Jan 21, 2023
@petrochenkov petrochenkov removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 21, 2023
petrochenkov added a commit to petrochenkov/rust that referenced this pull request Jan 21, 2023
@petrochenkov
rustc_metadata: Encode doc(hidden) flag to metadata
415c141 
To retrieve these flags rustdoc currently has to mass decode full attributes for items in the whole crate tree, so it's better to pre-compute it in advance.

This is especially for short-term performance of rust-lang#107054 because resolver cannot use memoization of query results yet.
@bors

This comment was marked as resolved.

Sign in to view
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 23, 2023
@bors
Auto merge of rust-lang#107136 - petrochenkov:dochidden, r=cjgillot
5bef91c 
rustc_metadata: Encode `doc(hidden)` flag to metadata

To retrieve these flags rustdoc currently has to mass decode full attributes for items in the whole crate tree, so it's better to pre-compute it in advance.

This is especially important for short-term performance of rust-lang#107054 because resolver cannot use memoization of query results yet.
@petrochenkov
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 25, 2023
@bors
Copy link
Contributor

bors commented Jan 25, 2023

⌛ Trying commit 3982258 with merge 65af9c7fb16aacb9bffc7f415bc387d8740839c2...

@petrochenkov petrochenkov removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 25, 2023
@bors
Copy link
Contributor

bors commented Jan 25, 2023

☀️ Try build successful - checks-actions
Build commit: 65af9c7fb16aacb9bffc7f415bc387d8740839c2 (65af9c7fb16aacb9bffc7f415bc387d8740839c2)

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (65af9c7fb16aacb9bffc7f415bc387d8740839c2): 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)		 -0.4% [-0.7%, -0.2%] 4
Improvements ✅
(secondary)		 -0.6% [-0.9%, -0.2%] 16
All ❌✅ (primary) -0.4% [-0.7%, -0.2%] 4

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.9% [0.9%, 0.9%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 26, 2023
@petrochenkov
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 26, 2023
@petrochenkov petrochenkov mentioned this pull request Jan 26, 2023
Merged
@petrochenkov
Copy link
Contributor Author

r? @GuillaumeGomez

@rustbot rustbot assigned GuillaumeGomez and unassigned jsha Jan 26, 2023
@GuillaumeGomez
Copy link
Member

Thanks!

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Jan 26, 2023

📌 Commit 3982258 has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors 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 Jan 26, 2023
@bors
Copy link
Contributor

bors commented Jan 27, 2023

⌛ Testing commit 3982258 with merge 6874f4e...

@bors
Copy link
Contributor

bors commented Jan 27, 2023

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 6874f4e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 27, 2023
@bors bors merged commit 6874f4e into rust-lang:master Jan 27, 2023
@rustbot rustbot added this to the 1.69.0 milestone Jan 27, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6874f4e): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.4% [0.3%, 0.4%] 2
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -0.5% [-1.0%, -0.3%] 4
Improvements ✅
(secondary)		 -0.6% [-0.9%, -0.2%] 14
All ❌✅ (primary) -0.2% [-1.0%, 0.4%] 6

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.0% [3.0%, 3.0%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.9% [-2.9%, -2.9%] 1
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

@rustbot rustbot added the perf-regression Performance regression. label Jan 27, 2023
@nnethercote
Copy link
Contributor

Perf wins greatly outweigh the regressions here.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.69.0
Development

Successfully merging this pull request may close these issues.
8 participants
@petrochenkov @rustbot @rust-timer @bors @rust-log-analyzer @GuillaumeGomez @nnethercote @jsha