Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Register new snapshots #30352

Merged
merged 1 commit into from
Dec 21, 2015
Merged

Register new snapshots #30352

merged 1 commit into from
Dec 21, 2015

Conversation

alexcrichton
Copy link
Member

Lots of cruft to remove!

@alexcrichton
Copy link
Member Author

r? @nikomatsakis

@rust-highfive
Copy link
Contributor

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive assigned nikomatsakis and unassigned aturon Dec 12, 2015
@alexcrichton
Copy link
Member Author

(or anyone else)

@arielb1
Copy link
Contributor

arielb1 commented Dec 12, 2015

This version has a performance regression DO NOT SNAPSHOT UNTIL IT IS FIXED!

vtable: vtable
})
}
pub fn cstore_to_cratestore(a: Rc<CStore>) -> Rc<for<'s> CrateStore<'s>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

just kill the function. it has no reason for existing.

@alexcrichton
Copy link
Member Author

@arielb1 oh hm, is there an issue for that? Depending on how much the snapshot is desired it may be worth making a snapshot before the regression (or just going ahead if the regression isn't that bad anyway)

@dotdash
Copy link
Contributor

dotdash commented Dec 12, 2015

@alexcrichton I guess this might be about #30242 which apparently introduced a perf regression. There's no issue for that yet AFAICT.

@@ -197,7 +195,6 @@ impl<T: ?Sized> !marker::Send for Rc<T> {}
impl<T: ?Sized> !marker::Sync for Rc<T> {}

// remove cfg after new snapshot
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment should be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, it indeed can

@pnkfelix
Copy link
Member

I think that #30368 is @arielb1 's PR to address the performance regression.

@bors
Copy link
Collaborator

bors commented Dec 18, 2015

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

@alexcrichton
Copy link
Member Author

Rebased and updated with new snapshots that include #30368

@nikomatsakis
Copy link
Contributor

@bors r+ p=1

Travis has been kind of flaky lately, eh? Is there a reason for that?

@bors
Copy link
Collaborator

bors commented Dec 20, 2015

📌 Commit afa656a has been approved by nikomatsakis

@bors
Copy link
Collaborator

bors commented Dec 20, 2015

⌛ Testing commit afa656a with merge 9bffee0...

@bors
Copy link
Collaborator

bors commented Dec 20, 2015

💔 Test failed - auto-win-gnu-32-opt

@alexcrichton
Copy link
Member Author

@bors: retry

@alexcrichton
Copy link
Member Author

@bors: r=nikomatsakis 34a75ea7abc6472498e91f7e7f2d7484429b7aa9

Actually travis seems to be pretty reliable for me at least as that's a test I needed to fix!

@bors
Copy link
Collaborator

bors commented Dec 20, 2015

⌛ Testing commit 34a75ea with merge 14cea44...

@bors
Copy link
Collaborator

bors commented Dec 20, 2015

💔 Test failed - auto-mac-64-nopt-t

Lots of cruft to remove!
@alexcrichton
Copy link
Member Author

@bors: r=nikomatsakis cd1848a

@bors
Copy link
Collaborator

bors commented Dec 21, 2015

⌛ Testing commit cd1848a with merge 0c158e7...

@bors
Copy link
Collaborator

bors commented Dec 21, 2015

💔 Test failed - auto-mac-32-opt

@alexcrichton
Copy link
Member Author

@bors: retry

On Mon, Dec 21, 2015 at 11:11 AM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-mac-32-opt
http://buildbot.rust-lang.org/builders/auto-mac-32-opt/builds/7529


Reply to this email directly or view it on GitHub
#30352 (comment).

bors added a commit that referenced this pull request Dec 21, 2015
@bors
Copy link
Collaborator

bors commented Dec 21, 2015

⌛ Testing commit cd1848a with merge 2343a92...

@bors bors merged commit cd1848a into rust-lang:master Dec 21, 2015
@alexcrichton alexcrichton deleted the new-snashots branch December 22, 2015 05:09
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants