From 90b5aa53284604b7a592f49a7e8424051e681da0 Mon Sep 17 00:00:00 2001 From: John Naylor Date: Sun, 10 Dec 2023 15:14:24 +0700 Subject: [PATCH v8 4/5] Assert that the fasthash variants give (or can give) the same answer as the original Test that incremental hashing gives the right answer for strings Use the initial length only for the init step. Test that we can ignore the length afterwards, and only use the presence of the NUL terminator to stop iterating. Assert that this results in the same hash. Based on "Use new hash APIs for search path cache" by Jeff Davis, rebased over v7. --- src/backend/catalog/namespace.c | 56 ++++++++++++++++++++++++++-- src/include/common/hashfn_unstable.h | 18 ++++++++- 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 5027efc91d..afbfaf5dd4 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -42,6 +42,7 @@ #include "catalog/pg_type.h" #include "commands/dbcommands.h" #include "common/hashfn.h" +#include "common/hashfn_unstable.h" #include "funcapi.h" #include "mb/pg_wchar.h" #include "miscadmin.h" @@ -247,11 +248,60 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames, static inline uint32 spcachekey_hash(SearchPathCacheKey key) { - const unsigned char *bytes = (const unsigned char *) key.searchPath; + const char *buf = key.searchPath; + fasthash_state hs; + + // XXX not for commit +#ifdef USE_ASSERT_CHECKING int blen = strlen(key.searchPath); - return hash_combine(hash_bytes(bytes, blen), - hash_uint32(key.roleid)); + uint64 h_orig = fasthash64_orig(buf, blen, key.roleid); + + // check full function that calls incremental interface + Assert(fasthash64((const unsigned char *) buf, blen, key.roleid) == h_orig); + + // Test that bytewise interface can give the same answer, + // if we have length up front. We would typically use it + // for cases where we don't know, but let's try to make + // it as similar as conveniently possible. + fasthash_init(&hs, blen, key.roleid); + while (*buf) + { + fasthash_accum_byte(&hs, *buf); + buf++; + } + Assert(fasthash_final64(&hs) == h_orig); + buf = key.searchPath; /* reset */ + + // Now compare chunked incremental interface + fasthash_init(&hs, blen, key.roleid); + while (*buf) + { + int chunk_len = 0; + + while(chunk_len < FH_SIZEOF_ACCUM && buf[chunk_len] != '\0') + chunk_len++; + + fasthash_accum(&hs, (const unsigned char *) buf, chunk_len); + buf += chunk_len; + } + Assert(fasthash_final64(&hs) == h_orig); + buf = key.searchPath; /* reset */ +#endif + + // TODO: Test if chunked accum has better performance + + // WIP: maybe roleid should be mixed in normally + // WIP: For now fake the length to preserve the internal seed + fasthash_init(&hs, 1, key.roleid); + while (*buf) + { + fasthash_accum_byte(&hs, *buf); + buf++; + } + + // WIP: consider returning lower 32 bits, rather than mixing the high bits with the lower + return fasthash_final32(&hs); } static inline bool diff --git a/src/include/common/hashfn_unstable.h b/src/include/common/hashfn_unstable.h index 80aec98dc9..13d6d70910 100644 --- a/src/include/common/hashfn_unstable.h +++ b/src/include/common/hashfn_unstable.h @@ -13,6 +13,8 @@ the same hashes between versions. #ifndef HASHFN_UNSTABLE_H #define HASHFN_UNSTABLE_H +#include "port/pg_bswap.h" + /* * fasthash is a modification of code taken from * https://code.google.com/archive/p/fast-hash/source/default/source @@ -91,9 +93,14 @@ fasthash_accum_byte(fasthash_state *hs, const unsigned char ch) hs->accum |= ch; hs->accum_len++; - // wip: is there a better way to get sizeof struct member? - if (hs->accum_len == sizeof(((fasthash_state *) 0)->accum)) + if (hs->accum_len == FH_SIZEOF_ACCUM) + { +#ifdef USE_ASSERT_CHECKING + // XXX not for commit + hs->accum = pg_bswap64(hs->accum); +#endif fasthash_combine(hs); + } } static inline void @@ -134,7 +141,14 @@ fasthash_final64(fasthash_state *hs) // check for remaining bytes to combine into hash // should only be used by the bytewise interface if (hs->accum_len > 0) + { +#ifdef USE_ASSERT_CHECKING + // XXX not for commit + hs->accum <<= ((FH_SIZEOF_ACCUM - hs->accum_len) * BITS_PER_BYTE); + hs->accum = pg_bswap64(hs->accum); +#endif fasthash_combine(hs); + } return fasthash_mix(hs->hash); } -- 2.43.0