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

libcore: Inline mem::forget(). #33357

Merged
merged 1 commit into from
May 5, 2016
Merged

Conversation

pcwalton
Copy link
Contributor

@pcwalton pcwalton commented May 2, 2016

Was causing severe performance problems in WebRender.

r? @brson

Was causing severe performance problems in WebRender.
@brson
Copy link
Contributor

brson commented May 2, 2016

@bors r+

@bors
Copy link
Contributor

bors commented May 2, 2016

📌 Commit 237eb72 has been approved by brson

@Aatch
Copy link
Contributor

Aatch commented May 3, 2016

Given that mem::forget is generic, I'm surprised that it wasn't already inlined. We're not reusing monomorphisations from other crates yet, are we?

@dotdash
Copy link
Contributor

dotdash commented May 3, 2016

It should be available for inlining, the only exception I know of would be when using codegen units. In that case only one codegen unit will actually monomorphize a given instance of a generic function. This is a somewhat recent change made in #32469

cc @nikomatsakis

Manishearth added a commit to Manishearth/rust that referenced this pull request May 3, 2016
libcore: Inline `mem::forget()`.

Was causing severe performance problems in WebRender.

r? @brson
bors added a commit that referenced this pull request May 3, 2016
Rollup of 14 pull requests

- Successful merges: #33277, #33294, #33314, #33322, #33333, #33338, #33339, #33340, #33343, #33357, #33363, #33365, #33371, #33372
- Failed merges:
@nikomatsakis
Copy link
Contributor

I think that if something is tagged as #[inline], as opposed to being merely generic, then it is made universally available, no?

bors added a commit that referenced this pull request May 3, 2016
Rollup of 14 pull requests

- Successful merges: #33277, #33294, #33314, #33322, #33333, #33338, #33339, #33340, #33343, #33357, #33363, #33365, #33371, #33372
- Failed merges:
@dotdash
Copy link
Contributor

dotdash commented May 3, 2016

@nikomatsakis right. The change only affected generic functions that are not tagged #[inline]

bors added a commit that referenced this pull request May 4, 2016
Rollup of 14 pull requests

- Successful merges: #33277, #33294, #33314, #33322, #33333, #33338, #33339, #33340, #33343, #33357, #33363, #33365, #33371, #33372
- Failed merges:
bors added a commit that referenced this pull request May 4, 2016
Rollup of 14 pull requests

- Successful merges: #33277, #33294, #33314, #33322, #33333, #33338, #33339, #33340, #33343, #33357, #33363, #33365, #33371, #33372
- Failed merges:
@bors bors merged commit 237eb72 into rust-lang:master May 5, 2016
@mbrubeck mbrubeck mentioned this pull request May 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants