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

rustc: precompute the largest Niche and store it in LayoutDetails. #62692

Merged
merged 3 commits into from
Jul 26, 2019
Merged

rustc: precompute the largest Niche and store it in LayoutDetails. #62692

merged 3 commits into from
Jul 26, 2019

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Jul 15, 2019

Since we only ever can use at most one niche, it makes sense to just store that in the layout, for the simplest caching (especially as it's almost trivial to compute).

There might be a speedup from this, but even if it's marginal now, the caching would be a more significant benefit for future optimization attempts.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(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 Jul 15, 2019
@eddyb
Copy link
Member Author

eddyb commented Jul 15, 2019

@bors try

@bors
Copy link
Contributor

bors commented Jul 15, 2019

⌛ Trying commit 88eced5 with merge 7e03358...

bors added a commit that referenced this pull request Jul 15, 2019
@bors
Auto merge of #62692 - eddyb:precompute-niches, r=<try>
7e03358 
rustc: precompute the largest Niche and store it in LayoutDetails.

Since we only ever can use at most one niche, it makes sense to just store that in the layout, for the simplest caching (especially as it's almost trivial to compute).

There might be a speedup from this, but even if it's marginal now, the caching would be a more significant benefit for future optimization attempts.
@bors
Copy link
Contributor

bors commented Jul 15, 2019

☀️ Try build successful - checks-azure
Build commit: 7e03358

@eddyb
Copy link
Member Author

eddyb commented Jul 16, 2019

@rust-timer build 7e03358

@rust-timer
Copy link
Collaborator

Success: Queued 7e03358 with parent 92b0f52, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 7e03358, comparison URL.

@eddyb
Copy link
Member Author

eddyb commented Jul 16, 2019

cc @rkruppe @nagisa @oli-obk

@eddyb
Copy link
Member Author

eddyb commented Jul 18, 2019

@oli-obk Is that a "r=me", or do you want to wait on other people?

@eddyb eddyb mentioned this pull request Jul 18, 2019
Closed
@oli-obk
Copy link
Contributor

oli-obk commented Jul 20, 2019

@bors r+

yea, sorry, I was confused

@bors
Copy link
Contributor

bors commented Jul 20, 2019

📌 Commit 88eced5 has been approved by oli-obk

@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 Jul 20, 2019
bjorn3
bjorn3 reviewed Jul 20, 2019
View reviewed changes
src/librustc/ty/layout.rs
@@ -246,13 +246,22 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
let align = a.value.align(dl).max(b_align).max(dl.aggregate_align);
let b_offset = a.value.size(dl).align_to(b_align.abi);
let size = (b_offset + b.value.size(dl)).align_to(align.abi);

// HACK(nox): We iter on `b` and then `a` because `max_by_key`
Copy link
Member

Choose a reason for hiding this comment

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

Why @nox here? According to git all comments are authored by @eddyb.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you search for "nox" in this diff you can see where the code was copied from

Copy link
Member

Choose a reason for hiding this comment

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

Missed that :)

@bors
Copy link
Contributor

bors commented Jul 21, 2019

⌛ Testing commit 88eced5 with merge 6945455ed42f049c4d5380762ef9b5bae090062a...

@Centril
Copy link
Contributor

Centril commented Jul 21, 2019

@bors retry Yielding to #62797.

Centril added a commit to Centril/rust that referenced this pull request Jul 23, 2019
@Centril
Rollup merge of rust-lang#62692 - eddyb:precompute-niches, r=oli-obk
281dfd5 
rustc: precompute the largest Niche and store it in LayoutDetails.

Since we only ever can use at most one niche, it makes sense to just store that in the layout, for the simplest caching (especially as it's almost trivial to compute).

There might be a speedup from this, but even if it's marginal now, the caching would be a more significant benefit for future optimization attempts.
@Centril Centril mentioned this pull request Jul 23, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jul 24, 2019
@Centril
Rollup merge of rust-lang#62692 - eddyb:precompute-niches, r=oli-obk
0eac668 
rustc: precompute the largest Niche and store it in LayoutDetails.

Since we only ever can use at most one niche, it makes sense to just store that in the layout, for the simplest caching (especially as it's almost trivial to compute).

There might be a speedup from this, but even if it's marginal now, the caching would be a more significant benefit for future optimization attempts.
@Centril Centril mentioned this pull request Jul 24, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jul 25, 2019
@Centril
Rollup merge of rust-lang#62692 - eddyb:precompute-niches, r=oli-obk
5c3e224 
rustc: precompute the largest Niche and store it in LayoutDetails.

Since we only ever can use at most one niche, it makes sense to just store that in the layout, for the simplest caching (especially as it's almost trivial to compute).

There might be a speedup from this, but even if it's marginal now, the caching would be a more significant benefit for future optimization attempts.
@Centril Centril mentioned this pull request Jul 25, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jul 26, 2019
@Centril
Rollup merge of rust-lang#62692 - eddyb:precompute-niches, r=oli-obk
602980d 
rustc: precompute the largest Niche and store it in LayoutDetails.

Since we only ever can use at most one niche, it makes sense to just store that in the layout, for the simplest caching (especially as it's almost trivial to compute).

There might be a speedup from this, but even if it's marginal now, the caching would be a more significant benefit for future optimization attempts.
@Centril Centril mentioned this pull request Jul 26, 2019
Closed
bors added a commit that referenced this pull request Jul 26, 2019
@bors
Auto merge of #63001 - Centril:rollup-up9baiv, r=Centril
f907629 
Rollup of 14 pull requests

Successful merges:

 - #62084 (allow clippy::unreadable_literal in unicode tables)
 - #62421 (Introduce `as_deref` to Option)
 - #62692 (rustc: precompute the largest Niche and store it in LayoutDetails.)
 - #62801 (Remove support for -Zlower-128bit-ops)
 - #62828 (Remove vector fadd/fmul reduction workarounds)
 - #62862 (code cleanup)
 - #62897 (Attempt to fix backtrace tests on i686-msvc)
 - #62904 (Disable d32 on armv6 hf targets)
 - #62907 (Initialize the MSP430 AsmParser)
 - #62956 (Implement slow-path for FirstSets::first)
 - #62963 (Allow lexer to recover from some homoglyphs)
 - #62970 (ci: gate toolstate repo pushes on the TOOLSTATE_PUBLISH envvar)
 - #62983 (Remove needless indirection through Rc)
 - #62985 (librustc_errors: Support ui-testing flag in annotate-snippet emitter)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Jul 26, 2019
@Centril
Rollup merge of rust-lang#62692 - eddyb:precompute-niches, r=oli-obk
e13ace7 
rustc: precompute the largest Niche and store it in LayoutDetails.

Since we only ever can use at most one niche, it makes sense to just store that in the layout, for the simplest caching (especially as it's almost trivial to compute).

There might be a speedup from this, but even if it's marginal now, the caching would be a more significant benefit for future optimization attempts.
@Centril Centril mentioned this pull request Jul 26, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jul 26, 2019
@Centril
Rollup merge of rust-lang#62692 - eddyb:precompute-niches, r=oli-obk
9d12267 
rustc: precompute the largest Niche and store it in LayoutDetails.

Since we only ever can use at most one niche, it makes sense to just store that in the layout, for the simplest caching (especially as it's almost trivial to compute).

There might be a speedup from this, but even if it's marginal now, the caching would be a more significant benefit for future optimization attempts.
@Centril Centril mentioned this pull request Jul 26, 2019
Merged
bors added a commit that referenced this pull request Jul 26, 2019
@bors
Auto merge of #63015 - Centril:rollup-ydhpcas, r=Centril
c43753f 
Rollup of 22 pull requests

Successful merges:

 - #62084 (allow clippy::unreadable_literal in unicode tables)
 - #62120 (Add missing type links in documentation)
 - #62310 (Add missing doc links in boxed module)
 - #62421 (Introduce `as_deref` to Option)
 - #62583 (Implement Unpin for all raw pointers)
 - #62692 (rustc: precompute the largest Niche and store it in LayoutDetails.)
 - #62801 (Remove support for -Zlower-128bit-ops)
 - #62828 (Remove vector fadd/fmul reduction workarounds)
 - #62862 (code cleanup)
 - #62904 (Disable d32 on armv6 hf targets)
 - #62907 (Initialize the MSP430 AsmParser)
 - #62956 (Implement slow-path for FirstSets::first)
 - #62963 (Allow lexer to recover from some homoglyphs)
 - #62964 (clarify and unify some type test names)
 - #62970 (ci: gate toolstate repo pushes on the TOOLSTATE_PUBLISH envvar)
 - #62980 (std: Add more accessors for `Metadata` on Windows)
 - #62983 (Remove needless indirection through Rc)
 - #62985 (librustc_errors: Support ui-testing flag in annotate-snippet emitter)
 - #63002 (error_index_generator should output stdout/stderr when it panics.)
 - #63004 (Add test for issue-54062)
 - #63007 (ci: debug network failures while downloading awscli from PyPI)
 - #63009 (Remove redundant `mut` from variable declaration.)

Failed merges:

r? @ghost
@bors bors merged commit 88eced5 into rust-lang:master Jul 26, 2019
@eddyb eddyb deleted the precompute-niches branch July 27, 2019 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjorn3 bjorn3 bjorn3 left review comments

@oli-obk oli-obk oli-obk approved these changes

Assignees

@matthewjasper matthewjasper

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@eddyb @rust-highfive @bors @rust-timer @oli-obk @Centril @RalfJung @bjorn3 @matthewjasper