Skip to content

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

Merged
bors merged 1 commit into
rust-lang:masterfrom
dotdash:inline_hash
May 3, 2015
Merged

Restore HashMap performance by allowing some functions to be inlined#25070
bors merged 1 commit into
rust-lang:masterfrom
dotdash:inline_hash

Conversation

@dotdash

@dotdash dotdash commented May 3, 2015

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor

r? @nikomatsakis

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

Comment thread src/libcore/hash/sip.rs

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very true.

@dotdash

dotdash commented May 3, 2015

Copy link
Copy Markdown
Contributor Author

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

huonw commented May 3, 2015

Copy link
Copy Markdown
Contributor

#6616 is relevant. :)

@alexcrichton

Copy link
Copy Markdown
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

bors commented May 3, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit f4176b5 with merge 796be61...

@bors

bors commented May 3, 2015

Copy link
Copy Markdown
Collaborator

@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