Skip to content

Add inline annotations on Vec's Deref and DerefMut #52343

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

Closed
wants to merge 1 commit into from

Conversation

Marwes
Copy link
Contributor

@Marwes Marwes commented Jul 13, 2018

While doing profiling on https://github.com/BurntSushi/aho-corasick I noticed that Vec::deref_mut would show up as a hot function despite the fact that it should be a no-op that should be trivially inlined. (as the layout of Vec<T> is that of &[T] + a capacity after).

I'd expect that the method(s) would usually be inlined despite the lack of an inline in most cases but for the crate in question I observed a 1.2x speedup if I replaced the deref with ::std::mem::transmute_copy so there was definitely a noticeable performance regression.

Also, I recognize that it is likely that, were I to use different version of rustc, then the problem could go away. But the fact remains that (evidently) some versions of rustc misses this optimization and maybe an inline annotation can help to avoid this.

While doing profiling on https://github.com/BurntSushi/aho-corasick I noticed that `Vec::deref_mut` would show up as a hot function despite the fact that it should be a no-op that should be trivially inlined. (as the layout of `Vec<T>` is that of `&[T]` + a capacity after).

I'd expect that the method(s) would usually be inlined despite the lack of an inline in most cases but for the crate in question I observed a 1.2x speedup if I replaced the deref with `::std::mem::transmute_copy` so there was definitely a noticeable performance regression.

Also, I recognize that it is likely that, were I to use different version of rustc, then the problem could go away. But the fact remains that (evidently) some versions of rustc misses this optimization and maybe an inline annotation can help to avoid this.
@rust-highfive
Copy link
Contributor

r? @KodrAus

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 13, 2018
@Marwes
Copy link
Contributor Author

Marwes commented Jul 13, 2018

Err, this is just me being an idiot again. CARGO_INCREMENTAL=1 were set in another location (thought I had purged them all ... #48905)

@Marwes Marwes closed this Jul 13, 2018
@Marwes Marwes deleted the patch-1 branch July 13, 2018 13:19
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.

3 participants