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

Rebased refactorings for Chalk #47861

Merged
merged 22 commits into from
Mar 2, 2018

Conversation

sgrif
Copy link
Contributor

@sgrif sgrif commented Jan 29, 2018

The code is Niko's, I just handled the rebase.

r? @nikomatsakis

@bors
Copy link
Contributor

bors commented Jan 29, 2018

☔ The latest upstream changes (presumably #47837) made this pull request unmergeable. Please resolve the merge conflicts.

@sgrif
Copy link
Contributor Author

sgrif commented Jan 29, 2018

... Well that didn't last for long, did it?

@kennytm kennytm added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 30, 2018
@sgrif sgrif force-pushed the sg-rebase-chalkify-universe-refactorings branch from 37c4b69 to cd8bc39 Compare January 31, 2018 22:13
@sgrif sgrif added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 31, 2018
@nikomatsakis
Copy link
Contributor

@bors try

I'd like do some perf measurements on this.

@bors
Copy link
Contributor

bors commented Jan 31, 2018

⌛ Trying commit cd8bc39 with merge 761166b...

bors added a commit that referenced this pull request Jan 31, 2018
…, r=<try>

Rebased refactorings for Chalk

The code is Niko's, I just handled the rebase.

r? @nikomatsakis
@bors
Copy link
Contributor

bors commented Feb 1, 2018

☀️ Test successful - status-travis
State: approved= try=True

@kennytm
Copy link
Member

kennytm commented Feb 1, 2018

@Mark-Simulacrum Perf needed, try#761166b6b9a66fcb560459e3c394daf1c125d060. (master#560a2f4faf4828ba5f48fe2bd7709265c2f5354d already measured).

@Mark-Simulacrum
Copy link
Member

Perf started. ETA around 45-50 minutes.

@nikomatsakis
Copy link
Contributor

@Mark-Simulacrum ping :)

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Feb 1, 2018

@kennytm thanks! =)

@pietroalbini
Copy link
Member

@nikomatsakis @sgrif what's the status on this?

@sgrif
Copy link
Contributor Author

sgrif commented Feb 5, 2018

I'm going to look into the perf today

@sgrif
Copy link
Contributor Author

sgrif commented Feb 5, 2018

0082951 is not the problem

@kennytm kennytm 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 5, 2018
@sgrif
Copy link
Contributor Author

sgrif commented Feb 5, 2018

Should be good to go now

@sgrif sgrif added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 5, 2018
@nikomatsakis
Copy link
Contributor

@bors try

Should we repeat the perf run? seems like a reasonable thing to do.

@kennytm
Copy link
Member

kennytm commented Feb 6, 2018

@bors clean retry try

@bors
Copy link
Contributor

bors commented Feb 6, 2018

🔒 Merge conflict

@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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 6, 2018
@bors
Copy link
Contributor

bors commented Feb 6, 2018

🔒 Merge conflict

nikomatsakis and others added 9 commits March 1, 2018 08:04
These changes were meant to be in
2b18d8fe9dc05415a8e6b7cadf879c7f7ebe020a (rebased from
12a2305), but I messed up the rebase a
bit as the file had been moved.
These modules were replaced with re-exports from ena
Additional uses of this item were added to these files in rust-lang#45701 and rust-lang#46479
@sgrif sgrif force-pushed the sg-rebase-chalkify-universe-refactorings branch from c16caff to fec4d3b Compare March 1, 2018 15:06
@sgrif
Copy link
Contributor Author

sgrif commented Mar 1, 2018

I rebased again, though there were no merge conflicts... :/

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 1, 2018
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Mar 1, 2018

📌 Commit fec4d3b has been approved by nikomatsakis

@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 Mar 1, 2018
@bors
Copy link
Contributor

bors commented Mar 1, 2018

⌛ Testing commit fec4d3b with merge ddfbf2b...

bors added a commit that referenced this pull request Mar 1, 2018
…, r=nikomatsakis

Rebased refactorings for Chalk

The code is Niko's, I just handled the rebase.

r? @nikomatsakis
@bors
Copy link
Contributor

bors commented Mar 2, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing ddfbf2b to master...

@andjo403
Copy link
Contributor

andjo403 commented Mar 2, 2018

large perf regression when this landed http://perf.rust-lang.org/compare.html?start=3eeb5a665e313c5b281820099e04d4c6c8188b46&end=ddfbf2b0f4726f11bc8e857274ae1a0f8343a73f&stat=instructions:u can it be the bump of ena that changed the times after the last perf run?

@nikomatsakis
Copy link
Contributor

@ishitatsuyuki opened #48660 to track the perf regression, but I think we ought to roll it back till we understand the cause. Could be the bump of ena, thought that would be quite surprising.

sgrif added a commit to sgrif/rust that referenced this pull request Mar 2, 2018
This reverts commit ccd92c2.

This commit is the source of a major perf regression, and was not
intended to be included in rust-lang#47861. At some point I must have
accidentally re-added the commit.
bors added a commit that referenced this pull request Mar 3, 2018
Revert "correct subtle bug in the type variable code"

This reverts commit ccd92c2.

This commit is the source of a major perf regression, and was not
intended to be included in #47861. At some point I must have
accidentally re-added the commit.

Fixes #48660.

r? @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants