Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Remove the last use of memmove. NFC.
Much clearer!
- Loading branch information
1 parent
fabffba
commit cc10788
Showing
1 changed file
with
2 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cc10788
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably, performance degradation
cc10788
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
cc10788
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember such performance issue when profiling. Evil-Spirit@44eb1a4#diff-b7a4b1c7a3c5a4ea4f00130583590ff4R336
cc10788
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I turn back memmove and probably get some speedup.
cc10788
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, that's good to know. Maybe we need to define our own
std::move
for POD types that will have the same interface but is faster.Though I do not know why they couldn't just specialize
std::move
itself for POD types, you can use SFINAE for it, I think...cc10788
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I was pretty sure that std::move/std::copy is generally implemented in terms of memmove/memcpy for trivially-copyable stuff, etc. In godbolt a simple test of std::move in a vector of pod types actually turned into a bunch of MOVUPS if the range was of constant size, or a call to memmove if it wasn't: https://godbolt.org/z/t81plU
cc10788
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://cppinsights.io/lnk?code=I2luY2x1ZGUgPGFsZ29yaXRobT4KI2luY2x1ZGUgPHZlY3Rvcj4KCnN0cnVjdCBCbGEgewogIGludCB4OwogIGZsb2F0IHk7CiAgZG91YmxlIHo7Cn07CgovLyBUeXBlIHlvdXIgY29kZSBoZXJlLCBvciBsb2FkIGFuIGV4YW1wbGUuCnZvaWQgdHJ5aXQoc3RkOjp2ZWN0b3I8QmxhPiAmc3R1ZmYsIHNpemVfdCBuLCBzaXplX3Qgb2Zmc2V0KSB7CiAgLy9zdGQ6OnZlY3RvcjxCbGE+IHN0dWZmKDgsIEJsYXsxLCAxLCAxfSk7CiAgc3RkOjptb3ZlKHN0dWZmLmJlZ2luKCksIHN0dWZmLmJlZ2luKCkgKyBuLCBzdHVmZi5iZWdpbigpICsgb2Zmc2V0KTsKfQ==&std=cpp2a&rev=1.0