diff --git a/src/core/lib/slice/slice_intern.cc b/src/core/lib/slice/slice_intern.cc index 369135c9eac..0feef1604ed 100644 --- a/src/core/lib/slice/slice_intern.cc +++ b/src/core/lib/slice/slice_intern.cc @@ -50,10 +50,6 @@ typedef struct slice_shard { size_t capacity; } slice_shard; -/* hash seed: decided at initialization time */ -uint32_t g_hash_seed; -static int g_forced_hash_seed = 0; - static slice_shard g_shards[SHARD_COUNT]; typedef struct { @@ -68,6 +64,10 @@ uint32_t grpc_static_metadata_hash_values[GRPC_STATIC_MDSTR_COUNT]; namespace grpc_core { +/* hash seed: decided at initialization time */ +uint32_t g_hash_seed; +static bool g_forced_hash_seed = false; + InternedSliceRefcount::~InternedSliceRefcount() { slice_shard* shard = &g_shards[SHARD_IDX(this->hash)]; MutexLock lock(&shard->mu); @@ -115,7 +115,7 @@ grpc_core::InternedSlice::InternedSlice(InternedSliceRefcount* s) { uint32_t grpc_slice_default_hash_impl(grpc_slice s) { return gpr_murmur_hash3(GRPC_SLICE_START_PTR(s), GRPC_SLICE_LENGTH(s), - g_hash_seed); + grpc_core::g_hash_seed); } uint32_t grpc_static_slice_hash(grpc_slice s) { @@ -159,20 +159,20 @@ grpc_slice grpc_slice_intern(grpc_slice slice) { } // Attempt to see if the provided slice or string matches a static slice. -// SliceArgs... is either a const grpc_slice& or a string and length. In either -// case, hash is the pre-computed hash value. +// SliceArgs is either a const grpc_slice& or const pair&. +// In either case, hash is the pre-computed hash value. // // Returns: a matching static slice, or null. -template +template static const grpc_core::StaticMetadataSlice* MatchStaticSlice( - uint32_t hash, SliceArgs&&... args) { + uint32_t hash, const SliceArgs& args) { for (uint32_t i = 0; i <= max_static_metadata_hash_probe; i++) { static_metadata_hash_ent ent = static_metadata_hash[(hash + i) % GPR_ARRAY_SIZE(static_metadata_hash)]; const grpc_core::StaticMetadataSlice* static_slice_table = grpc_static_slice_table(); if (ent.hash == hash && ent.idx < GRPC_STATIC_MDSTR_COUNT && - static_slice_table[ent.idx].Equals(std::forward(args)...)) { + static_slice_table[ent.idx] == args) { return &static_slice_table[ent.idx]; } } @@ -182,8 +182,12 @@ static const grpc_core::StaticMetadataSlice* MatchStaticSlice( // Helper methods to enable us to select appropriately overloaded slice methods // whether we're dealing with a slice, or a buffer with length, when interning // strings. Helpers for FindOrCreateInternedSlice(). -static const void* GetBuffer(const void* buf, size_t len) { return buf; } -static size_t GetLength(const void* buf, size_t len) { return len; } +static const char* GetBuffer(const std::pair& buflen) { + return buflen.first; +} +static size_t GetLength(const std::pair& buflen) { + return buflen.second; +} static const void* GetBuffer(const grpc_slice& slice) { return GRPC_SLICE_START_PTR(slice); } @@ -192,19 +196,19 @@ static size_t GetLength(const grpc_slice& slice) { } // Creates an interned slice for a string that does not currently exist in the -// intern table. SliceArgs... is either a const grpc_slice& or a string and -// length. In either case, hash is the pre-computed hash value. We must already -// hold the shard lock. Helper for FindOrCreateInternedSlice(). +// intern table. SliceArgs is either a const grpc_slice& or a const +// pair&. Hash is the pre-computed hash value. We must +// already hold the shard lock. Helper for FindOrCreateInternedSlice(). // // Returns: a newly interned slice. -template +template static InternedSliceRefcount* InternNewStringLocked(slice_shard* shard, size_t shard_idx, uint32_t hash, - SliceArgs&&... args) { + const SliceArgs& args) { /* string data goes after the internal_string header */ - size_t len = GetLength(std::forward(args)...); - const void* buffer = GetBuffer(std::forward(args)...); + size_t len = GetLength(args); + const void* buffer = GetBuffer(args); InternedSliceRefcount* s = static_cast(gpr_malloc(sizeof(*s) + len)); new (s) grpc_core::InternedSliceRefcount(len, hash, shard->strs[shard_idx]); @@ -227,16 +231,15 @@ static InternedSliceRefcount* InternNewStringLocked(slice_shard* shard, // shard lock. Helper for FindOrCreateInternedSlice(). // // Returns: a pre-existing matching static slice, or null. -template +template static InternedSliceRefcount* MatchInternedSliceLocked(uint32_t hash, size_t idx, - SliceArgs&&... args) { + const SliceArgs& args) { InternedSliceRefcount* s; slice_shard* shard = &g_shards[SHARD_IDX(hash)]; /* search for an existing string */ for (s = shard->strs[idx]; s; s = s->bucket_next) { - if (s->hash == hash && - grpc_core::InternedSlice(s).Equals(std::forward(args)...)) { + if (s->hash == hash && grpc_core::InternedSlice(s) == args) { if (s->refcnt.RefIfNonZero()) { return s; } @@ -248,22 +251,20 @@ static InternedSliceRefcount* MatchInternedSliceLocked(uint32_t hash, // Attempt to see if the provided slice or string matches an existing interned // slice, and failing that, create an interned slice with its contents. Returns // either the existing matching interned slice or the newly created one. -// SliceArgs... is either a const grpc_slice& or a string and length. In either -// case, hash is the pre-computed hash value. We do not hold the shard lock -// here, but do take it. +// SliceArgs is either a const grpc_slice& or const pair&. +// In either case, hash is the pre-computed hash value. We do not hold the +// shard lock here, but do take it. // // Returns: an interned slice, either pre-existing/matched or newly created. -template +template static InternedSliceRefcount* FindOrCreateInternedSlice(uint32_t hash, - SliceArgs&&... args) { + const SliceArgs& args) { slice_shard* shard = &g_shards[SHARD_IDX(hash)]; gpr_mu_lock(&shard->mu); const size_t idx = TABLE_IDX(hash, shard->capacity); - InternedSliceRefcount* s = - MatchInternedSliceLocked(hash, idx, std::forward(args)...); + InternedSliceRefcount* s = MatchInternedSliceLocked(hash, idx, args); if (s == nullptr) { - s = InternNewStringLocked(shard, idx, hash, - std::forward(args)...); + s = InternNewStringLocked(shard, idx, hash, args); } gpr_mu_unlock(&shard->mu); return s; @@ -277,12 +278,13 @@ grpc_core::ManagedMemorySlice::ManagedMemorySlice(const char* string, size_t len) { GPR_TIMER_SCOPE("grpc_slice_intern", 0); const uint32_t hash = gpr_murmur_hash3(string, len, g_hash_seed); - const StaticMetadataSlice* static_slice = MatchStaticSlice(hash, string, len); + const StaticMetadataSlice* static_slice = + MatchStaticSlice(hash, std::pair(string, len)); if (static_slice) { *this = *static_slice; } else { - *this = - grpc_core::InternedSlice(FindOrCreateInternedSlice(hash, string, len)); + *this = grpc_core::InternedSlice(FindOrCreateInternedSlice( + hash, std::pair(string, len))); } } @@ -303,13 +305,14 @@ grpc_core::ManagedMemorySlice::ManagedMemorySlice(const grpc_slice* slice_ptr) { } void grpc_test_only_set_slice_hash_seed(uint32_t seed) { - g_hash_seed = seed; - g_forced_hash_seed = 1; + grpc_core::g_hash_seed = seed; + grpc_core::g_forced_hash_seed = true; } void grpc_slice_intern_init(void) { - if (!g_forced_hash_seed) { - g_hash_seed = static_cast(gpr_now(GPR_CLOCK_REALTIME).tv_nsec); + if (!grpc_core::g_forced_hash_seed) { + grpc_core::g_hash_seed = + static_cast(gpr_now(GPR_CLOCK_REALTIME).tv_nsec); } for (size_t i = 0; i < SHARD_COUNT; i++) { slice_shard* shard = &g_shards[i]; diff --git a/src/core/lib/slice/slice_internal.h b/src/core/lib/slice/slice_internal.h index f1939b5ba7c..e74aba94145 100644 --- a/src/core/lib/slice/slice_internal.h +++ b/src/core/lib/slice/slice_internal.h @@ -36,7 +36,6 @@ // Interned slices have specific fast-path operations for hashing. To inline // these operations, we need to forward declare them here. extern uint32_t grpc_static_metadata_hash_values[GRPC_STATIC_MDSTR_COUNT]; -extern uint32_t g_hash_seed; // grpc_slice_refcount : A reference count for grpc_slice. // @@ -259,7 +258,8 @@ inline uint32_t grpc_slice_refcount::Hash(const grpc_slice& slice) { break; } return gpr_murmur_hash3(grpc_refcounted_slice_data(slice), - grpc_refcounted_slice_length(slice), g_hash_seed); + grpc_refcounted_slice_length(slice), + grpc_core::g_hash_seed); } inline const grpc_slice& grpc_slice_ref_internal(const grpc_slice& slice) { @@ -327,7 +327,7 @@ inline uint32_t grpc_slice_hash_refcounted(const grpc_slice& s) { inline uint32_t grpc_slice_default_hash_internal(const grpc_slice& s) { return gpr_murmur_hash3(GRPC_SLICE_START_PTR(s), GRPC_SLICE_LENGTH(s), - g_hash_seed); + grpc_core::g_hash_seed); } inline uint32_t grpc_slice_hash_internal(const grpc_slice& s) { diff --git a/src/core/lib/slice/slice_utils.h b/src/core/lib/slice/slice_utils.h index a4e19a9a61b..b203a1662a6 100644 --- a/src/core/lib/slice/slice_utils.h +++ b/src/core/lib/slice/slice_utils.h @@ -25,6 +25,12 @@ #include +#include "src/core/lib/gpr/murmur_hash.h" + +namespace grpc_core { +extern uint32_t g_hash_seed; +} // namespace grpc_core + // When we compare two slices, and we know the latter is not inlined, we can // short circuit our comparison operator. We specifically use differs() // semantics instead of equals() semantics due to more favourable code @@ -101,15 +107,16 @@ struct ManagedMemorySlice : public grpc_slice { explicit ManagedMemorySlice(const char* string); ManagedMemorySlice(const char* buf, size_t len); explicit ManagedMemorySlice(const grpc_slice* slice); - bool Equals(const grpc_slice& other) const { + bool operator==(const grpc_slice& other) const { if (refcount == other.refcount) { return true; } return !grpc_slice_differs_refcounted(other, *this); } - bool Equals(const char* buf, const size_t len) const { - return data.refcounted.length == len && buf != nullptr && - memcmp(buf, data.refcounted.bytes, len) == 0; + bool operator!=(const grpc_slice& other) const { return !(*this == other); } + bool operator==(std::pair buflen) const { + return data.refcounted.length == buflen.second && buflen.first != nullptr && + memcmp(buflen.first, data.refcounted.bytes, buflen.second) == 0; } }; struct UnmanagedMemorySlice : public grpc_slice { @@ -150,6 +157,16 @@ struct ExternallyManagedSlice : public UnmanagedMemorySlice { data.refcounted.length = length; data.refcounted.bytes = bytes; } + bool operator==(const grpc_slice& other) const { + return data.refcounted.length == GRPC_SLICE_LENGTH(other) && + memcmp(data.refcounted.bytes, GRPC_SLICE_START_PTR(other), + data.refcounted.length) == 0; + } + bool operator!=(const grpc_slice& other) const { return !(*this == other); } + uint32_t Hash() { + return gpr_murmur_hash3(data.refcounted.bytes, data.refcounted.length, + g_hash_seed); + } }; struct StaticMetadataSlice : public ManagedMemorySlice { diff --git a/src/core/lib/surface/server.cc b/src/core/lib/surface/server.cc index 4fd3b0e99f1..16186fa0377 100644 --- a/src/core/lib/surface/server.cc +++ b/src/core/lib/surface/server.cc @@ -99,8 +99,8 @@ struct channel_registered_method { registered_method* server_registered_method; uint32_t flags; bool has_host; - grpc_slice method; - grpc_slice host; + grpc_core::ExternallyManagedSlice method; + grpc_core::ExternallyManagedSlice host; }; struct channel_data { @@ -630,8 +630,8 @@ static void start_new_rpc(grpc_call_element* elem) { chand->registered_method_slots]; if (rm->server_registered_method == nullptr) break; if (!rm->has_host) continue; - if (!grpc_slice_eq(rm->host, calld->host)) continue; - if (!grpc_slice_eq(rm->method, calld->path)) continue; + if (rm->host != calld->host) continue; + if (rm->method != calld->path) continue; if ((rm->flags & GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST) && 0 == (calld->recv_initial_metadata_flags & GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST)) { @@ -648,7 +648,7 @@ static void start_new_rpc(grpc_call_element* elem) { chand->registered_method_slots]; if (rm->server_registered_method == nullptr) break; if (rm->has_host) continue; - if (!grpc_slice_eq(rm->method, calld->path)) continue; + if (rm->method != calld->path) continue; if ((rm->flags & GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST) && 0 == (calld->recv_initial_metadata_flags & GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST)) { @@ -921,8 +921,14 @@ static void server_destroy_channel_elem(grpc_channel_element* elem) { if (chand->registered_methods) { for (i = 0; i < chand->registered_method_slots; i++) { grpc_slice_unref_internal(chand->registered_methods[i].method); + GPR_DEBUG_ASSERT(chand->registered_methods[i].method.refcount == + &grpc_core::kNoopRefcount || + chand->registered_methods[i].method.refcount == nullptr); if (chand->registered_methods[i].has_host) { grpc_slice_unref_internal(chand->registered_methods[i].host); + GPR_DEBUG_ASSERT(chand->registered_methods[i].host.refcount == + &grpc_core::kNoopRefcount || + chand->registered_methods[i].host.refcount == nullptr); } } gpr_free(chand->registered_methods); @@ -1202,18 +1208,13 @@ void grpc_server_setup_transport( chand->registered_methods = static_cast(gpr_zalloc(alloc)); for (rm = s->registered_methods; rm; rm = rm->next) { - grpc_slice host; - bool has_host; - grpc_slice method; - if (rm->host != nullptr) { - host = grpc_slice_from_static_string(rm->host); - has_host = true; - } else { - has_host = false; + grpc_core::ExternallyManagedSlice host; + grpc_core::ExternallyManagedSlice method(rm->method); + const bool has_host = rm->host != nullptr; + if (has_host) { + host = grpc_core::ExternallyManagedSlice(rm->host); } - method = grpc_slice_from_static_string(rm->method); - hash = GRPC_MDSTR_KV_HASH(has_host ? grpc_slice_hash_internal(host) : 0, - grpc_slice_hash_internal(method)); + hash = GRPC_MDSTR_KV_HASH(has_host ? host.Hash() : 0, method.Hash()); for (probes = 0; chand->registered_methods[(hash + probes) % slots] .server_registered_method != nullptr; probes++)