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.

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

rustdoc: Don't enter an infer_ctxt in get_blanket_impls for impls that aren't blanket impls #82864

Merged
merged 1 commit into from
Mar 30, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 7, 2021

Less broken version of #82856.

get_blanket_impls is a very hot region of rustdoc, so even small changes like this should help. Unfortunately I don't have benchmarks for this until rust-lang/rustc-perf#802 is merged.

@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. I-compiletime Issue: Problems and improvements with respect to compile times. labels Mar 7, 2021
@rust-highfive
Copy link
Collaborator

r? @GuillaumeGomez

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 7, 2021
@GuillaumeGomez
Copy link
Member

Then let's wait until the perf check is merged (otherwise the change looks good). Marking it as blocked.

@GuillaumeGomez GuillaumeGomez added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Mar 7, 2021
@jyn514
Copy link
Member Author

jyn514 commented Mar 7, 2021

Then let's wait until the perf check is merged (otherwise the change looks good). Marking it as blocked.

It's been open for several months :/ I don't know if it will be merged at all.

@GuillaumeGomez
Copy link
Member

I want to actually be able to follow the perf changes as much as possible. What do we need to do in order to make it merged faster?

@jyn514
Copy link
Member Author

jyn514 commented Mar 7, 2021

I want to actually be able to follow the perf changes as much as possible. What do we need to do in order to make it merged faster?

rust-lang/rustc-perf#802 (comment)

@GuillaumeGomez
Copy link
Member

Apparently it's "close" to getting merge. If there's still no update in a week, let's maybe ask @Mark-Simulacrum directly about it. :)

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2021
@jyn514
Copy link
Member Author

jyn514 commented Mar 29, 2021

rust-lang/rustc-perf#802 was merged.

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 29, 2021
@bors
Copy link
Contributor

bors commented Mar 29, 2021

⌛ Trying commit f8b15d8 with merge 631b9863b1cf0df0e5f955ea5332af802c96df78...

@jyn514 jyn514 removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Mar 29, 2021
@bors
Copy link
Contributor

bors commented Mar 29, 2021

☀️ Try build successful - checks-actions
Build commit: 631b9863b1cf0df0e5f955ea5332af802c96df78 (631b9863b1cf0df0e5f955ea5332af802c96df78)

@rust-timer
Copy link
Collaborator

Queued 631b9863b1cf0df0e5f955ea5332af802c96df78 with parent 3aedcf0, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (631b9863b1cf0df0e5f955ea5332af802c96df78): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

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

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 29, 2021
@jyn514
Copy link
Member Author

jyn514 commented Mar 29, 2021

Wow, that is a surprisingly large difference, I only expected it to show up for big crates.

otherwise the change looks good

@bors r=GuillaumeGomez

@bors
Copy link
Contributor

bors commented Mar 29, 2021

📌 Commit f8b15d8 has been approved by GuillaumeGomez

@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 Mar 29, 2021
@bors
Copy link
Contributor

bors commented Mar 29, 2021

⌛ Testing commit f8b15d8 with merge 50489e3...

@bors
Copy link
Contributor

bors commented Mar 30, 2021

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 50489e3 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 30, 2021
@bors bors merged commit 50489e3 into rust-lang:master Mar 30, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 30, 2021
@jyn514 jyn514 deleted the short-circuit branch March 30, 2021 15:40
@jyn514 jyn514 added the relnotes-perf Performance improvements that should be mentioned in the release notes. label Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-compiletime Issue: Problems and improvements with respect to compile times. merged-by-bors This PR was explicitly merged by bors. relnotes-perf Performance improvements that should be mentioned in the release notes. 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
Development

Successfully merging this pull request may close these issues.

7 participants