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

Prefer memcpy to cast of unaligned pointer #51

Merged
merged 1 commit into from
Sep 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions include/orc/memory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ decltype(auto) make_leaky(Args&&... args) {
#endif
}

/**************************************************************************************************/
// Useful for reading integer values (that otherwise need to be aligned) from unaligned memory. The
// typical solution is to use memcpy and let the compiler optimize it further.
template <class Integer>
Integer unaligned_read(const void* p) {
Integer result{Integer()};
std::memcpy(&result, p, sizeof(result));
return result;
}

/**************************************************************************************************/

} // namespace orc
Expand Down
7 changes: 6 additions & 1 deletion src/hash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
// identity
#include "orc/hash.hpp"

// application
#include "orc/memory.hpp"

/**************************************************************************************************/

namespace {
Expand Down Expand Up @@ -37,7 +40,9 @@ inline uint64_t rotl64(uint64_t x, int8_t r) { return (x << r) | (x >> (64 - r))

/**************************************************************************************************/

FORCE_INLINE uint64_t getblock64(const uint64_t* p, int i) { return p[i]; }
FORCE_INLINE std::uint64_t getblock64(const uint64_t* p, int i) {
return orc::unaligned_read<std::uint64_t>(p + i);
}

/**************************************************************************************************/

Expand Down
18 changes: 7 additions & 11 deletions src/string_pool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ namespace {

/**************************************************************************************************/

std::size_t string_view_hash(std::string_view s) {
return orc::murmur3_64(s.data(), s.length());
}
std::size_t string_view_hash(std::string_view s) { return orc::murmur3_64(s.data(), s.length()); }

/**************************************************************************************************/

Expand Down Expand Up @@ -85,19 +83,16 @@ struct pool {
std::size_t pool_string::get_size(const char* d) {
assert(d);
const void* bytes = d - sizeof(std::uint32_t) - sizeof(std::size_t);
std::uint32_t s;
std::memcpy(&s, bytes, sizeof(s)); // not aligned - need to use memcpy
assert(s > 0); // required, else should have been _data == nullptr
assert(s < 100000); // sanity check
std::uint32_t s = orc::unaligned_read<std::uint32_t>(bytes);
assert(s > 0); // required, else should have been _data == nullptr
assert(s < 100000); // sanity check
return s;
}

std::size_t pool_string::get_hash(const char* d) {
assert(d);
const void* bytes = d - sizeof(std::size_t);
std::size_t h;
std::memcpy(&h, bytes, sizeof(h)); // not aligned -- need to use memcpy
return h;
return orc::unaligned_read<std::size_t>(bytes);
}

pool_string empool(std::string_view src) {
Expand All @@ -106,7 +101,8 @@ pool_string empool(std::string_view src) {
// default_view would be returned.)
if (src.empty()) return pool_string(nullptr);

static decltype(auto) keys = orc::make_leaky<tbb::concurrent_unordered_map<size_t, const char*>>();
static decltype(auto) keys =
orc::make_leaky<tbb::concurrent_unordered_map<size_t, const char*>>();

const std::size_t h = string_view_hash(src);

Expand Down