Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 5, 2016
Merged

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

merged 1 commit into from
May 5, 2016

Conversation

@pcwalton
Copy link
Contributor

@pcwalton pcwalton commented May 2, 2016

Was causing severe performance problems in WebRender.

r? @brson
@pcwalton
libcore: Inline mem::forget().
237eb72 
Was causing severe performance problems in WebRender.
@brson
Copy link
Contributor

brson commented May 2, 2016

@bors r+
@bors
Copy link
Collaborator

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 •
edited
Loading

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 Manishearth mentioned this pull request May 3, 2016
Merged
Manishearth added a commit to Manishearth/rust that referenced this pull request May 3, 2016
@Manishearth
Rollup merge of rust-lang#33357 - pcwalton:inline-mem-forget, r=brson
676fd36 
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
@bors
Auto merge of #33376 - Manishearth:rollup, r=Manishearth
1fb4b5b 
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
@bors
Auto merge of #33376 - Manishearth:rollup, r=Manishearth
f7125fc 
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
@bors
Auto merge of #33376 - Manishearth:rollup, r=Manishearth
1895d08 
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
@bors
Auto merge of #33376 - Manishearth:rollup, r=Manishearth
7a0ccc4 
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
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

6 participants

@pcwalton @brson @bors @Aatch @dotdash @nikomatsakis