Skip to content

Rerevert "Remove checked_add in Layout::repeat"#69743

Closed
ecstatic-morse wants to merge 1 commit intorust-lang:masterfrom
ecstatic-morse:rerevert-67174
Closed

Rerevert "Remove checked_add in Layout::repeat"#69743
ecstatic-morse wants to merge 1 commit intorust-lang:masterfrom
ecstatic-morse:rerevert-67174

Conversation

@ecstatic-morse
Copy link
Contributor

@ecstatic-morse ecstatic-morse commented Mar 5, 2020

This change, which originated in #67174 and was reapplied in #69544, seems to have caused a noticeable slowdown in patched/clean incremental builds (see #69710). Revert it for now while we investigate the underlying issue.

r? @nnethercote

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 5, 2020
@ecstatic-morse
Copy link
Contributor Author

ecstatic-morse commented Mar 5, 2020

@bors try
@rust-timer queue

@rust-timer
Copy link
Collaborator

rust-timer commented Mar 5, 2020

Awaiting bors try build completion

@bors
Copy link
Collaborator

bors commented Mar 5, 2020

⌛ Trying commit 04e9877 with merge 4bfc61c90b9066ee396739e074d6d05abcfda04e...

@ecstatic-morse ecstatic-morse added S-waiting-on-perf Status: Waiting on a perf run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 5, 2020
@nnethercote
Copy link
Contributor

nnethercote commented Mar 5, 2020

@bors r+
@bors rollup=never due to perf effects.

@bors
Copy link
Collaborator

bors commented Mar 5, 2020

📌 Commit 04e9877 has been approved by nnethercote

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 5, 2020
@TimDiekmann TimDiekmann mentioned this pull request Mar 9, 2020
Closed
@ecstatic-morse
Copy link
Contributor Author

ecstatic-morse commented Mar 10, 2020

#69879 resulted in very similar perf changes to this PR. That rollup includes #69799, which makes changes to RawVec. Since the perf queue is empty:

@bors try
@rust-timer queue

@bors
Copy link
Collaborator

bors commented Mar 10, 2020

🙅 Please do not try after a pull request has been r+ed. If you need to try, unapprove (r-) it first.

@rust-timer
Copy link
Collaborator

rust-timer commented Mar 10, 2020

Awaiting bors try build completion

@ecstatic-morse
Copy link
Contributor Author

ecstatic-morse commented Mar 10, 2020

@bors r-
@bors try

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 10, 2020
@bors
Copy link
Collaborator

bors commented Mar 10, 2020

⌛ Trying commit 04e9877 with merge 0f376b8...

bors added a commit that referenced this pull request Mar 10, 2020
@bors
Auto merge of #69743 - ecstatic-morse:rerevert-67174, r=<try>
0f376b8 
Rerevert "Remove checked_add in Layout::repeat"

This change, which originated in #67174 and was reapplied in #69544, seems to have caused a noticeable slowdown in patched/clean incremental builds (see #69710). Revert it for now while we investigate the underlying issue.

r? @nnethercote
@bors
Copy link
Collaborator

bors commented Mar 10, 2020

☀️ Try build successful - checks-azure
Build commit: 0f376b8 (0f376b8045f869ed7313d20e2362ed1417e11e9d)

@rust-timer
Copy link
Collaborator

rust-timer commented Mar 10, 2020

Queued 0f376b8 with parent dd155df, future comparison URL.

@rust-timer
Copy link
Collaborator

rust-timer commented Mar 11, 2020

Finished benchmarking try commit 0f376b8, comparison URL.

@ecstatic-morse
Copy link
Contributor Author

ecstatic-morse commented Mar 11, 2020

It seems that whatever optimization was made possible by this revert was also enabled by #69799. Closing.

@ecstatic-morse ecstatic-morse deleted the rerevert-67174 branch March 11, 2020 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nnethercote nnethercote

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-perf Status: Waiting on a perf run to be completed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ecstatic-morse @rust-timer @bors @nnethercote @rust-highfive