From 015573b5afa7f8bbf3626247820023ec6cec637b Mon Sep 17 00:00:00 2001 From: Matej Ferencevic Date: Mon, 27 Jan 2020 17:03:17 +0100 Subject: [PATCH] Clean-up utils/hashing Reviewers: teon.banek Reviewed By: teon.banek Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D2645 --- src/query/frontend/stripped.cpp | 4 +-- src/query/frontend/stripped.hpp | 6 ++-- src/query/interpreter.cpp | 2 +- src/query/interpreter.hpp | 12 ++++---- src/query/plan/operator.cpp | 2 +- src/query/plan/operator.lcp | 2 +- src/query/plan/vertex_count_cache.hpp | 2 +- src/query/typed_value.cpp | 2 +- src/utils/{hashing => }/fnv.hpp | 42 ++++++++------------------- src/utils/hashing/fnv32.hpp | 28 ------------------ src/utils/hashing/fnv64.hpp | 28 ------------------ src/utils/platform.hpp | 12 -------- 12 files changed, 28 insertions(+), 114 deletions(-) rename src/utils/{hashing => }/fnv.hpp (78%) delete mode 100644 src/utils/hashing/fnv32.hpp delete mode 100644 src/utils/hashing/fnv64.hpp delete mode 100644 src/utils/platform.hpp diff --git a/src/query/frontend/stripped.cpp b/src/query/frontend/stripped.cpp index bf271a8e5..c297089f9 100644 --- a/src/query/frontend/stripped.cpp +++ b/src/query/frontend/stripped.cpp @@ -14,7 +14,7 @@ #include "query/frontend/opencypher/generated/MemgraphCypherLexer.h" #include "query/frontend/parsing.hpp" #include "query/frontend/stripped_lexer_constants.hpp" -#include "utils/hashing/fnv.hpp" +#include "utils/fnv.hpp" #include "utils/string.hpp" namespace query::frontend { @@ -130,7 +130,7 @@ StrippedQuery::StrippedQuery(const std::string &query) : original_(query) { } query_ = utils::Join(token_strings, " "); - hash_ = fnv(query_); + hash_ = utils::Fnv(query_); auto it = tokens.begin(); while (it != tokens.end()) { diff --git a/src/query/frontend/stripped.hpp b/src/query/frontend/stripped.hpp index ff1138423..6d1c7e9f9 100644 --- a/src/query/frontend/stripped.hpp +++ b/src/query/frontend/stripped.hpp @@ -4,7 +4,7 @@ #include #include "query/parameters.hpp" -#include "utils/hashing/fnv.hpp" +#include "utils/fnv.hpp" namespace query::frontend { @@ -50,7 +50,7 @@ class StrippedQuery { const auto &literals() const { return literals_; } const auto &named_expressions() const { return named_exprs_; } const auto ¶meters() const { return parameters_; } - HashType hash() const { return hash_; } + uint64_t hash() const { return hash_; } private: // Return len of matched keyword if something is matched, otherwise 0. @@ -86,7 +86,7 @@ class StrippedQuery { std::unordered_map named_exprs_; // Hash based on the stripped query. - HashType hash_; + uint64_t hash_; }; } // namespace query::frontend diff --git a/src/query/interpreter.cpp b/src/query/interpreter.cpp index 4ca02a44e..1caadc526 100644 --- a/src/query/interpreter.cpp +++ b/src/query/interpreter.cpp @@ -422,7 +422,7 @@ std::unique_ptr MakeLogicalPlan(AstStorage ast_storage, * cache a fresh one if it doesn't yet exist. */ std::shared_ptr CypherQueryToPlan( - HashType hash, AstStorage ast_storage, CypherQuery *query, + uint64_t hash, AstStorage ast_storage, CypherQuery *query, const Parameters ¶meters, utils::SkipList *plan_cache, DbAccessor *db_accessor) { auto plan_cache_access = plan_cache->access(); diff --git a/src/query/interpreter.hpp b/src/query/interpreter.hpp index 3eb6dda86..f183c5dbb 100644 --- a/src/query/interpreter.hpp +++ b/src/query/interpreter.hpp @@ -151,10 +151,10 @@ struct QueryCacheEntry { bool operator<(const QueryCacheEntry &other) const { return first < other.first; } - bool operator==(const HashType &other) const { return first == other; } - bool operator<(const HashType &other) const { return first < other; } + bool operator==(const uint64_t &other) const { return first == other; } + bool operator<(const uint64_t &other) const { return first < other; } - HashType first; + uint64_t first; // TODO: Maybe store the query string here and use it as a key with the hash // so that we eliminate the risk of hash collisions. CachedQuery second; @@ -167,10 +167,10 @@ struct PlanCacheEntry { bool operator<(const PlanCacheEntry &other) const { return first < other.first; } - bool operator==(const HashType &other) const { return first == other; } - bool operator<(const HashType &other) const { return first < other; } + bool operator==(const uint64_t &other) const { return first == other; } + bool operator<(const uint64_t &other) const { return first < other; } - HashType first; + uint64_t first; // TODO: Maybe store the query string here and use it as a key with the hash // so that we eliminate the risk of hash collisions. std::shared_ptr second; diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 3f8d0d83f..0e60d8aaf 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -27,7 +27,7 @@ #include "query/procedure/module.hpp" #include "utils/algorithm.hpp" #include "utils/exceptions.hpp" -#include "utils/hashing/fnv.hpp" +#include "utils/fnv.hpp" #include "utils/pmr/unordered_map.hpp" #include "utils/pmr/unordered_set.hpp" #include "utils/pmr/vector.hpp" diff --git a/src/query/plan/operator.lcp b/src/query/plan/operator.lcp index 05fc54b52..ad83154f5 100644 --- a/src/query/plan/operator.lcp +++ b/src/query/plan/operator.lcp @@ -16,7 +16,7 @@ #include "query/typed_value.hpp" #include "storage/v2/id_types.hpp" #include "utils/bound.hpp" -#include "utils/hashing/fnv.hpp" +#include "utils/fnv.hpp" #include "utils/memory.hpp" #include "utils/visitor.hpp" cpp<# diff --git a/src/query/plan/vertex_count_cache.hpp b/src/query/plan/vertex_count_cache.hpp index ad5a7729f..440804298 100644 --- a/src/query/plan/vertex_count_cache.hpp +++ b/src/query/plan/vertex_count_cache.hpp @@ -7,7 +7,7 @@ #include "storage/v2/id_types.hpp" #include "storage/v2/property_value.hpp" #include "utils/bound.hpp" -#include "utils/hashing/fnv.hpp" +#include "utils/fnv.hpp" namespace query::plan { diff --git a/src/query/typed_value.cpp b/src/query/typed_value.cpp index df4a2c90a..456896694 100644 --- a/src/query/typed_value.cpp +++ b/src/query/typed_value.cpp @@ -10,7 +10,7 @@ #include "glog/logging.h" #include "utils/exceptions.hpp" -#include "utils/hashing/fnv.hpp" +#include "utils/fnv.hpp" namespace query { diff --git a/src/utils/hashing/fnv.hpp b/src/utils/fnv.hpp similarity index 78% rename from src/utils/hashing/fnv.hpp rename to src/utils/fnv.hpp index ce36ebd02..cd9a5d76a 100644 --- a/src/utils/hashing/fnv.hpp +++ b/src/utils/fnv.hpp @@ -1,39 +1,21 @@ #pragma once #include -#include - -#include "utils/platform.hpp" - -#include "fnv32.hpp" -#include "fnv64.hpp" - -// fnv1a is recommended so use it as a default implementation. also use the -// platform specific version of the function - -namespace { - -#ifdef MEMGRAPH64 - -__attribute__((unused)) uint64_t fnv(const std::string &s) { - return fnv1a64(s); -} - -using HashType = uint64_t; - -#else - -__attribute__((unused)) uint32_t fnv(const std::string &s) { - return fnv1a32(s); -} - -using HashType = uint32_t; - -#endif -} +#include namespace utils { +inline uint64_t Fnv(const std::string_view &s) { + // fnv1a is recommended so use it as the default implementation. + uint64_t hash = 14695981039346656037UL; + + for (const auto &ch : s) { + hash = (hash ^ (uint64_t)ch) * 1099511628211UL; + } + + return hash; +} + /** * Does FNV-like hashing on a collection. Not truly FNV * because it operates on 8-bit elements, while this diff --git a/src/utils/hashing/fnv32.hpp b/src/utils/hashing/fnv32.hpp deleted file mode 100644 index 2dfc5d75e..000000000 --- a/src/utils/hashing/fnv32.hpp +++ /dev/null @@ -1,28 +0,0 @@ -#pragma once - -#include -#include - -namespace { - -#define OFFSET_BASIS32 2166136261u -#define FNV_PRIME32 16777619u - -__attribute__((unused)) uint32_t fnv32(const std::string &s) { - uint32_t hash = OFFSET_BASIS32; - - for (size_t i = 0; i < s.size(); ++i) - hash = (hash * FNV_PRIME32) xor (uint32_t) s[i]; - - return hash; -} - -__attribute__((unused)) uint32_t fnv1a32(const std::string &s) { - uint32_t hash = OFFSET_BASIS32; - - for (size_t i = 0; i < s.size(); ++i) - hash = (hash xor (uint32_t) s[i]) * FNV_PRIME32; - - return hash; -} -} diff --git a/src/utils/hashing/fnv64.hpp b/src/utils/hashing/fnv64.hpp deleted file mode 100644 index a93eb3147..000000000 --- a/src/utils/hashing/fnv64.hpp +++ /dev/null @@ -1,28 +0,0 @@ -#pragma once - -#include -#include - -namespace { - -#define OFFSET_BASIS64 14695981039346656037u -#define FNV_PRIME64 1099511628211u - -__attribute__((unused)) uint64_t fnv64(const std::string &s) { - uint64_t hash = OFFSET_BASIS64; - - for (size_t i = 0; i < s.size(); ++i) - hash = (hash * FNV_PRIME64) xor (uint64_t) s[i]; - - return hash; -} - -__attribute__((unused)) uint64_t fnv1a64(const std::string &s) { - uint64_t hash = OFFSET_BASIS64; - - for (size_t i = 0; i < s.size(); ++i) - hash = (hash xor (uint64_t) s[i]) * FNV_PRIME64; - - return hash; -} -} diff --git a/src/utils/platform.hpp b/src/utils/platform.hpp deleted file mode 100644 index 3f7747ea0..000000000 --- a/src/utils/platform.hpp +++ /dev/null @@ -1,12 +0,0 @@ -#pragma once - -#include - -// is there a better way? -#if UINTPTR_MAX == 0xffffffff -#define MEMGRAPH32 -#elif UINTPTR_MAX == 0xffffffffffffffff -#define MEMGRAPH64 -#else -#error Unrecognized platform (neither 32 or 64) -#endif