Skip to content

Restore HashMap performance by allowing some functions to be inlined #25070

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

Merged
merged 1 commit into from
May 3, 2015

Conversation

dotdash
Copy link
Contributor

@dotdash dotdash commented May 3, 2015

Since the hashmap and its hasher are implemented in different crates, we
currently can't benefit from inlining, which means that especially for
small, fixed size keys, there is a huge overhead in hash calculations,
because the compiler can't apply optimizations that only apply for these
keys.

Fixes the brainfuck benchmark in #24014.

Since the hashmap and its hasher are implemented in different crates, we
currently can't benefit from inlining, which means that especially for
small, fixed size keys, there is a huge overhead in hash calculations,
because the compiler can't apply optimizations that only apply for these
keys.

Fixes the brainfuck benchmark in rust-lang#24014.
@rust-highfive
Copy link
Contributor

r? @nikomatsakis

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

@@ -120,6 +121,7 @@ impl SipHasher {
self.ntail = 0;
}

#[inline]
fn write(&mut self, msg: &[u8]) {
Copy link
Member

Choose a reason for hiding this comment

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

Hm, it's unfortunate that this function is so large.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but that's also the most important function to have available for inlining, because most of it is thrown away when the compiler sees that, for example, it's only called with once with a single 8 byte slice. See my remark below about `#[inline(Maybe)].

Copy link
Member

Choose a reason for hiding this comment

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

Very true.

@dotdash
Copy link
Contributor Author

dotdash commented May 3, 2015

For the record, I wonder whether we might want a #[inline(Maybe)] attribute that could be applied to e.g. the write function, which would make rustc emit the AST so the function may be inlined, but without setting the inline hint.

@huonw
Copy link
Member

huonw commented May 3, 2015

#6616 is relevant. :)

@alexcrichton
Copy link
Member

@bors: r+ f4176b5

bors added a commit that referenced this pull request May 3, 2015
Since the hashmap and its hasher are implemented in different crates, we
currently can't benefit from inlining, which means that especially for
small, fixed size keys, there is a huge overhead in hash calculations,
because the compiler can't apply optimizations that only apply for these
keys.

Fixes the brainfuck benchmark in #24014.
@bors
Copy link
Collaborator

bors commented May 3, 2015

⌛ Testing commit f4176b5 with merge 796be61...

@bors bors merged commit f4176b5 into rust-lang:master May 3, 2015
@dotdash dotdash deleted the inline_hash branch July 27, 2015 08:49
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