Skip to content

Conversation

JohnTitor
Copy link
Member

@JohnTitor JohnTitor commented Jan 7, 2021

This reverts commit be2a3f8, reversing
changes made to 4c4e8e7.

It's second attempt to find the cause of the perf regression in rollup (#80708).

r? @ghost

@JohnTitor
Copy link
Member Author

Hey @rylev sorry for the trouble caused by my rollup! Could you run a perf check for this?

@rylev
Copy link
Member

rylev commented Jan 7, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@bors
Copy link
Collaborator

bors commented Jan 7, 2021

⌛ Trying commit 9ac4831 with merge 492b373476f18fc763cfb5b95ffc8b053d50edb0...

@bors
Copy link
Collaborator

bors commented Jan 7, 2021

☀️ Try build successful - checks-actions
Build commit: 492b373476f18fc763cfb5b95ffc8b053d50edb0 (492b373476f18fc763cfb5b95ffc8b053d50edb0)

@rust-timer
Copy link
Collaborator

Queued 492b373476f18fc763cfb5b95ffc8b053d50edb0 with parent c8915ee, future comparison URL.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 7, 2021
@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (492b373476f18fc763cfb5b95ffc8b053d50edb0): 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 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 Jan 8, 2021
@JohnTitor
Copy link
Member Author

The result is:

image

So we could say this is the cause.

@rylev
Copy link
Member

rylev commented Jan 8, 2021

The performance regressions aren't serious enough to actually warrant reverting. I think they should be investigated, but I'm closing this PR so no one actually merges it.

@rylev rylev closed this Jan 8, 2021
@JohnTitor JohnTitor deleted the revert-80538 branch January 8, 2021 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants