Skip to content

oid: use memcmp in git_oid__hashcmp#4328

Merged
ethomson merged 1 commit intomasterfrom
peff/hashcmp-is-memcmp
Aug 14, 2017
Merged

oid: use memcmp in git_oid__hashcmp#4328
ethomson merged 1 commit intomasterfrom
peff/hashcmp-is-memcmp

Conversation

@peff
Copy link
Copy Markdown
Member

@peff peff commented Aug 9, 2017

The open-coded version was inherited from git.git. But it turns out it was based on an older version of glibc, whose memcmp was not very optimized.

Modern glibc does much better, and some compilers (like gcc 7) can even inline the memcmp into a series of multi-byte xors.

Upstream is switching to using memcmp in git/git@0b00601.

Note that this is definitively faster on modern Linux systems, but there's been no benchmark on other platforms. It's possible that a bad libc/compiler combination may perform worse. In my opinion it's best to trust in memcmp by default, though. The current open-coded version was certainly did not come from benchmarks on non-Linux systems.

The open-coded version was inherited from git.git. But it
turns out it was based on an older version of glibc, whose
memcmp was not very optimized.

Modern glibc does much better, and some compilers (like gcc
7) can even inline the memcmp into a series of multi-byte
xors.

Upstream is switching to using memcmp in
git/git@0b00601.
Copy link
Copy Markdown
Member

@ethomson ethomson left a comment

Choose a reason for hiding this comment

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

Agreed; thanks!

@ethomson ethomson merged commit 577aeef into master Aug 14, 2017
@peff peff deleted the peff/hashcmp-is-memcmp branch August 14, 2017 21:03
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.

2 participants