Skip to content

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

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

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

Conversation

@ecstatic-morse

@ecstatic-morseecstatic-morse commented Mar 5, 2020

Copy link
Copy Markdown
Contributor

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-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 5, 2020
@ecstatic-morse

ecstatic-morse commented Mar 5, 2020

Copy link
Copy Markdown
ContributorAuthor

@bors try
@rust-timer queue

@rust-timer

rust-timer commented Mar 5, 2020

Copy link
Copy Markdown
Collaborator

Awaiting bors try build completion

@bors

bors commented Mar 5, 2020

Copy link
Copy Markdown
Collaborator

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

@ecstatic-morseecstatic-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

nnethercote commented Mar 5, 2020

Copy link
Copy Markdown
Contributor

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

@bors

bors commented Mar 5, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 04e9877 has been approved by nnethercote

@borsbors 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
@TimDiekmannTimDiekmann mentioned this pull request Mar 9, 2020
Closed
@ecstatic-morse

ecstatic-morse commented Mar 10, 2020

Copy link
Copy Markdown
ContributorAuthor

#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

bors commented Mar 10, 2020

Copy link
Copy Markdown
Collaborator

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

@rust-timer

rust-timer commented Mar 10, 2020

Copy link
Copy Markdown
Collaborator

Awaiting bors try build completion

@ecstatic-morse

ecstatic-morse commented Mar 10, 2020

Copy link
Copy Markdown
ContributorAuthor

@bors r-
@bors try

@borsbors 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

bors commented Mar 10, 2020

Copy link
Copy Markdown
Collaborator

⌛ 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

bors commented Mar 10, 2020

Copy link
Copy Markdown
Collaborator

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

@rust-timer

rust-timer commented Mar 10, 2020

Copy link
Copy Markdown
Collaborator

Queued 0f376b8 with parent dd155df, future comparison URL.

@rust-timer

rust-timer commented Mar 11, 2020

Copy link
Copy Markdown
Collaborator

Finished benchmarking try commit 0f376b8, comparison URL.

@ecstatic-morse

ecstatic-morse commented Mar 11, 2020

Copy link
Copy Markdown
ContributorAuthor

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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nnethercotennethercote

Labels

S-waiting-on-authorStatus: This is awaiting some action (such as code changes or more information) from the author.S-waiting-on-perfStatus: 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