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

Optimize string comparison by using memcmp #28338

Merged
merged 1 commit into from
Sep 11, 2015
Merged

Conversation

erickt
Copy link
Contributor

@erickt erickt commented Sep 10, 2015

llvm seems to be having some trouble optimizing the iterator-based string comparsion method into some equivalent to memcmp. This explicitly calls out to the memcmp intrinisic in order to allow llvm to generate better code. In some manual benchmarking, this memcmp-based approach is 20 times faster than the iterator approach.

@bluss
Copy link
Member

bluss commented Sep 10, 2015

Very nice find

@bors r+

@bors
Copy link
Contributor

bors commented Sep 10, 2015

📌 Commit bbad2d5 has been approved by bluss

@bluss
Copy link
Member

bluss commented Sep 10, 2015

I wonder how you slid in without being noticed by travis or the highfive bot.

@erickt
Copy link
Contributor Author

erickt commented Sep 10, 2015

How strange! https://status.github.com/ doesn't report any issues, so it's spooky.

@brson
Copy link
Contributor

brson commented Sep 10, 2015

@erickt Can you add a comment in the code about the optimization so somebody doesn't re-'fix' it later?

llvm seems to be having some trouble optimizing the iterator-based
string comparsion method into some equivalent to memcmp. This
explicitly calls out to the memcmp intrinisic in order to allow
llvm to generate better code. In some manual benchmarking, this
memcmp-based approach is 20 times faster than the iterator approach.
@erickt
Copy link
Contributor Author

erickt commented Sep 10, 2015

@brson: Done. How's it look?

@bluss
Copy link
Member

bluss commented Sep 11, 2015

@bors r+

@bors
Copy link
Contributor

bors commented Sep 11, 2015

📌 Commit fbd91a7 has been approved by bluss

@bors
Copy link
Contributor

bors commented Sep 11, 2015

⌛ Testing commit fbd91a7 with merge 1a1e6b8...

bors added a commit that referenced this pull request Sep 11, 2015
llvm seems to be having some trouble optimizing the iterator-based string comparsion method into some equivalent to memcmp. This explicitly calls out to the memcmp intrinisic in order to allow llvm to generate better code. In some manual benchmarking, this memcmp-based approach is 20 times faster than the iterator approach.
@bors bors merged commit fbd91a7 into rust-lang:master Sep 11, 2015
@bluss bluss added the relnotes Marks issues that should be documented in the release notes of the next release. label Sep 11, 2015
@erickt
Copy link
Contributor Author

erickt commented Sep 11, 2015

fyi, I had a bug in my benchmarks where llvm 3.6 was still optimizing a good chunk of code way even in light of #[inline(never)] and test::black_box. My test suite only tested for the first and last elements in a 26 item if-else chain, and when using memcmp it was clever enough to realize it could delete most of the actual comparisons. Darn you llvm for cheating!

After I refactored the tests to test all 26 items as well as factoring out the search methods into another crate to make sure llvm couldn't do that trick again, I had a much more modest 8-22% speedup with memcmp compared to the iterator comparison method.

@erickt erickt deleted the str-cmp branch September 11, 2015 16:37
@brson
Copy link
Contributor

brson commented Sep 11, 2015

Thanks @erickt. Still a fine win.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants