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

KeySet::push_back: Use memcpy to copy #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

jmr
Copy link
Contributor

@jmr jmr commented Jun 18, 2020

Replace for loop.

This should at least not be slower, and is probably faster. I didn't
benchmark it.

For full C++ style points, this could be std::copy_n(), but that
seems unnecessary.

jmr added 2 commits June 18, 2020 11:21
Replace for loop.

This should at least not be slower, and is probably faster. I didn't
benchmark it.

For full C++ style points, this could be std::copy_n(), but that
seems unnecessary.
I got these confused and the regions do not overlap.
@jmr jmr changed the title KeySet::push_back: Use memmove to copy KeySet::push_back: Use memcpy to copy Jun 19, 2020
@glebm
Copy link
Contributor

glebm commented Jun 20, 2020

For larger types, copy_n may be faster if the toolchain provides specialization that take advantage of type alignment (libstdc++ and libc++ don't seem to) and figure out that the ranges don't overlap.

As we're copying char here, memcpy should always be at least as fast as std::copy(_n).

Update: At least GCC seems to be aware of memcpy semantics and takes advantage of the type alignment (even though the argument is void *). Or maybe this happens at a lower level in the compiler, I didn't dig that deep.

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