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

Back more metadata using per-query tables #94129

Merged
merged 9 commits into from
Feb 24, 2022
Merged

Conversation

cjgillot
Copy link
Contributor

r? @ghost

@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Feb 18, 2022
@cjgillot
Copy link
Contributor Author

@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 Feb 18, 2022
@bors
Copy link
Contributor

bors commented Feb 18, 2022

⌛ Trying commit 4442ef44c027a775c29820c4906cd7ae295c1323 with merge 11295abb90a0b335673636cbcf583aee7b99d98b...

@bors
Copy link
Contributor

bors commented Feb 18, 2022

☀️ Try build successful - checks-actions
Build commit: 11295abb90a0b335673636cbcf583aee7b99d98b (11295abb90a0b335673636cbcf583aee7b99d98b)

@rust-timer
Copy link
Collaborator

Queued 11295abb90a0b335673636cbcf583aee7b99d98b with parent b8c56fa, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (11295abb90a0b335673636cbcf583aee7b99d98b): comparison url.

Summary: This benchmark run shows 38 relevant improvements 🎉 to instruction counts.

  • Average relevant improvement: -0.6%
  • Largest improvement in instruction counts: -0.9% on full builds of await-call-tree doc

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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 led to changes in compiler perf.

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Feb 19, 2022
@cjgillot cjgillot marked this pull request as ready for review February 19, 2022 16:34
@cjgillot
Copy link
Contributor Author

r? rust-lang/compiler

@lcnr
Copy link
Contributor

lcnr commented Feb 21, 2022

after a quick skim this lgtm

r? @petrochenkov

@petrochenkov
Copy link
Contributor

@bors r+
This slightly increases max-rss though.

@bors
Copy link
Contributor

bors commented Feb 22, 2022

📌 Commit 7afcf9f has been approved by petrochenkov

@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 Feb 22, 2022
Comment on lines 330 to +333
#[derive(Copy, Clone, MetadataEncodable, MetadataDecodable)]
enum EntryKind {
AnonConst(mir::ConstQualifs, Lazy<RenderedConst>),
Const(mir::ConstQualifs, Lazy<RenderedConst>),
AnonConst,
Const,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! I wonder if some time soon this whole enum could be subsumed by just DefKind.

@bors
Copy link
Contributor

bors commented Feb 24, 2022

⌛ Testing commit 7afcf9f with merge 7ccfe2f...

@bors
Copy link
Contributor

bors commented Feb 24, 2022

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 7ccfe2f to master...

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (7ccfe2f): comparison url.

Summary: This benchmark run shows 42 relevant improvements 🎉 but 4 relevant regressions 😿 to instruction counts.

  • Average relevant regression: 0.8%
  • Average relevant improvement: -0.6%
  • Largest improvement in instruction counts: -1.0% on full builds of wf-projection-stress-65510 doc
  • Largest regression in instruction counts: 0.9% on incr-unchanged builds of externs opt

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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

@rustbot rustbot added the perf-regression Performance regression. label Feb 24, 2022
@cjgillot cjgillot deleted the rmeta-table branch February 24, 2022 21:04
@cjgillot
Copy link
Contributor Author

The perf results are overall green for rustdoc. Regression is localized to a single benchmark externs for debug/opt configurations, but not check. This regression is real, but on a stress test: loading native_library_kind is slower.
The size of on-disk metadata has grown: the encoding is less space-efficient than it used to be (5% larger for externs).

@cjgillot cjgillot added the perf-regression-triaged The performance regression has been triaged. label Feb 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue. 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