From 1961436b933e460ba717f011470b5cf9a563bb2d Mon Sep 17 00:00:00 2001
From: Craig Tiller <ctiller@google.com>
Date: Thu, 17 Nov 2016 16:05:27 -0800
Subject: [PATCH] Fix static string interning
---
src/core/lib/slice/slice_intern.c | 58 ++++++++++++++++++++++-
src/core/lib/slice/slice_internal.h | 1 +
src/core/lib/transport/static_metadata.c | 4 +-
src/core/lib/transport/static_metadata.h | 1 +
test/core/slice/slice_test.c | 38 +++++++++++++++
tools/codegen/core/gen_static_metadata.py | 5 +-
6 files changed, 103 insertions(+), 4 deletions(-)
diff --git a/src/core/lib/slice/slice_intern.c b/src/core/lib/slice/slice_intern.c
index b5e00a38d7..b574ef5f76 100644
--- a/src/core/lib/slice/slice_intern.c
+++ b/src/core/lib/slice/slice_intern.c
@@ -72,6 +72,16 @@ static int g_forced_hash_seed = 0;
static slice_shard g_shards[SHARD_COUNT];
+typedef struct {
+ uint32_t hash;
+ uint32_t idx;
+} static_metadata_hash_ent;
+
+static static_metadata_hash_ent
+ static_metadata_hash[2 * GRPC_STATIC_MDSTR_COUNT];
+static uint32_t max_static_metadata_hash_probe;
+static uint32_t static_metadata_hash_values[GRPC_STATIC_MDSTR_COUNT];
+
static void interned_slice_ref(void *p) {
interned_slice_refcount *s = p;
GPR_ASSERT(gpr_atm_no_barrier_fetch_add(&s->refcnt, 1) > 0);
@@ -152,15 +162,35 @@ uint32_t grpc_slice_default_hash_impl(void *unused_refcnt, grpc_slice s) {
g_hash_seed);
}
+uint32_t grpc_static_slice_hash(void *unused_refcnt, grpc_slice s) {
+ int id = grpc_static_metadata_index(s);
+ if (id == -1) {
+ return grpc_slice_default_hash_impl(unused_refcnt, s);
+ }
+ return static_metadata_hash_values[id];
+}
+
uint32_t grpc_slice_hash(grpc_slice s) {
return s.refcount == NULL ? grpc_slice_default_hash_impl(NULL, s)
: s.refcount->vtable->hash(s.refcount, s);
}
grpc_slice grpc_slice_intern(grpc_slice slice) {
+ if (grpc_is_static_metadata_string(slice)) {
+ return slice;
+ }
+
+ uint32_t hash = grpc_slice_hash(slice);
+ 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)];
+ if (ent.hash == hash && ent.idx < GRPC_STATIC_MDSTR_COUNT &&
+ 0 == grpc_slice_cmp(grpc_static_slice_table[ent.idx], slice)) {
+ return grpc_static_slice_table[ent.idx];
+ }
+ }
+
interned_slice_refcount *s;
- uint32_t hash = gpr_murmur_hash3(GRPC_SLICE_START_PTR(slice),
- GRPC_SLICE_LENGTH(slice), g_hash_seed);
slice_shard *shard = &g_shards[SHARD_IDX(hash)];
gpr_mu_lock(&shard->mu);
@@ -212,6 +242,9 @@ void grpc_test_only_set_slice_hash_seed(uint32_t seed) {
}
void grpc_slice_intern_init(void) {
+ if (!g_forced_hash_seed) {
+ g_hash_seed = (uint32_t)gpr_now(GPR_CLOCK_REALTIME).tv_nsec;
+ }
for (size_t i = 0; i < SHARD_COUNT; i++) {
slice_shard *shard = &g_shards[i];
gpr_mu_init(&shard->mu);
@@ -220,6 +253,27 @@ void grpc_slice_intern_init(void) {
shard->strs = gpr_malloc(sizeof(*shard->strs) * shard->capacity);
memset(shard->strs, 0, sizeof(*shard->strs) * shard->capacity);
}
+ for (size_t i = 0; i < GPR_ARRAY_SIZE(static_metadata_hash); i++) {
+ static_metadata_hash[i].hash = 0;
+ static_metadata_hash[i].idx = GRPC_STATIC_MDSTR_COUNT;
+ }
+ max_static_metadata_hash_probe = 0;
+ for (size_t i = 0; i < GRPC_STATIC_MDSTR_COUNT; i++) {
+ static_metadata_hash_values[i] =
+ grpc_slice_default_hash_impl(NULL, grpc_static_slice_table[i]);
+ for (size_t j = 0; j < GPR_ARRAY_SIZE(static_metadata_hash); j++) {
+ size_t slot = (static_metadata_hash_values[i] + j) %
+ GPR_ARRAY_SIZE(static_metadata_hash);
+ if (static_metadata_hash[slot].idx == GRPC_STATIC_MDSTR_COUNT) {
+ static_metadata_hash[slot].hash = static_metadata_hash_values[i];
+ static_metadata_hash[slot].idx = (uint32_t)i;
+ if (j > max_static_metadata_hash_probe) {
+ max_static_metadata_hash_probe = (uint32_t)j;
+ }
+ break;
+ }
+ }
+ }
}
void grpc_slice_intern_shutdown(void) {
diff --git a/src/core/lib/slice/slice_internal.h b/src/core/lib/slice/slice_internal.h
index bf9117c74e..c02a34b9fc 100644
--- a/src/core/lib/slice/slice_internal.h
+++ b/src/core/lib/slice/slice_internal.h
@@ -49,5 +49,6 @@ void grpc_slice_buffer_destroy_internal(grpc_exec_ctx *exec_ctx,
void grpc_slice_intern_init(void);
void grpc_slice_intern_shutdown(void);
void grpc_test_only_set_slice_hash_seed(uint32_t key);
+uint32_t grpc_static_slice_hash(void *refcnt, grpc_slice s);
#endif /* GRPC_CORE_LIB_SLICE_SLICE_INTERNAL_H */
diff --git a/src/core/lib/transport/static_metadata.c b/src/core/lib/transport/static_metadata.c
index 7702b7cdfc..b065af73f9 100644
--- a/src/core/lib/transport/static_metadata.c
+++ b/src/core/lib/transport/static_metadata.c
@@ -41,6 +41,8 @@
#include "src/core/lib/transport/static_metadata.h"
+#include "src/core/lib/slice/slice_internal.h"
+
static uint8_t g_raw_bytes[] = {
48, 49, 50, 50, 48, 48, 50, 48, 52, 50, 48, 54, 51, 48, 52,
52, 48, 48, 52, 48, 52, 53, 48, 48, 97, 99, 99, 101, 112, 116,
@@ -115,7 +117,7 @@ static uint8_t g_raw_bytes[] = {
static void static_ref(void *unused) {}
static void static_unref(grpc_exec_ctx *exec_ctx, void *unused) {}
static const grpc_slice_refcount_vtable static_vtable = {
- static_ref, static_unref, grpc_slice_default_hash_impl};
+ static_ref, static_unref, grpc_static_slice_hash};
static grpc_slice_refcount g_refcnt = {&static_vtable};
bool grpc_is_static_metadata_string(grpc_slice slice) {
diff --git a/src/core/lib/transport/static_metadata.h b/src/core/lib/transport/static_metadata.h
index 65cb37d444..6f1441f2e8 100644
--- a/src/core/lib/transport/static_metadata.h
+++ b/src/core/lib/transport/static_metadata.h
@@ -249,6 +249,7 @@ extern const grpc_slice grpc_static_slice_table[GRPC_STATIC_MDSTR_COUNT];
bool grpc_is_static_metadata_string(grpc_slice slice);
+int grpc_static_metadata_index(grpc_slice slice);
#define GRPC_STATIC_MDELEM_COUNT 81
extern grpc_mdelem grpc_static_mdelem_table[GRPC_STATIC_MDELEM_COUNT];
extern uintptr_t grpc_static_mdelem_user_data[GRPC_STATIC_MDELEM_COUNT];
diff --git a/test/core/slice/slice_test.c b/test/core/slice/slice_test.c
index ddb66f9dea..ddce1d29b0 100644
--- a/test/core/slice/slice_test.c
+++ b/test/core/slice/slice_test.c
@@ -38,6 +38,9 @@
#include <grpc/grpc.h>
#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
+
+#include "src/core/lib/slice/slice_internal.h"
+#include "src/core/lib/transport/static_metadata.h"
#include "test/core/util/test_config.h"
#define LOG_TEST_NAME(x) gpr_log(GPR_INFO, "%s", x);
@@ -272,9 +275,42 @@ static void test_slice_interning(void) {
grpc_shutdown();
}
+static void test_static_slice_interning(void) {
+ LOG_TEST_NAME("test_static_slice_interning");
+
+ // grpc_init/grpc_shutdown deliberately omitted: they should not be necessary
+ // to intern a static slice
+
+ for (size_t i = 0; i < GRPC_STATIC_MDSTR_COUNT; i++) {
+ GPR_ASSERT(grpc_slice_is_equivalent(
+ grpc_static_slice_table[i],
+ grpc_slice_intern(grpc_static_slice_table[i])));
+ }
+}
+
+static void test_static_slice_copy_interning(void) {
+ LOG_TEST_NAME("test_static_slice_copy_interning");
+
+ grpc_init();
+
+ for (size_t i = 0; i < GRPC_STATIC_MDSTR_COUNT; i++) {
+ grpc_slice copy =
+ grpc_slice_malloc(GRPC_SLICE_LENGTH(grpc_static_slice_table[i]));
+ memcpy(GRPC_SLICE_START_PTR(copy),
+ GRPC_SLICE_START_PTR(grpc_static_slice_table[i]),
+ GRPC_SLICE_LENGTH(grpc_static_slice_table[i]));
+ GPR_ASSERT(grpc_slice_is_equivalent(grpc_static_slice_table[i],
+ grpc_slice_intern(copy)));
+ grpc_slice_unref(copy);
+ }
+
+ grpc_shutdown();
+}
+
int main(int argc, char **argv) {
unsigned length;
grpc_test_init(argc, argv);
+ grpc_test_only_set_slice_hash_seed(0);
test_slice_malloc_returns_something_sensible();
test_slice_new_returns_something_sensible();
test_slice_new_with_user_data();
@@ -286,5 +322,7 @@ int main(int argc, char **argv) {
}
test_slice_from_copied_string_works();
test_slice_interning();
+ test_static_slice_interning();
+ test_static_slice_copy_interning();
return 0;
}
diff --git a/tools/codegen/core/gen_static_metadata.py b/tools/codegen/core/gen_static_metadata.py
index 614b0efaa6..c5519c44ad 100755
--- a/tools/codegen/core/gen_static_metadata.py
+++ b/tools/codegen/core/gen_static_metadata.py
@@ -297,6 +297,8 @@ print >>H
print >>C, '#include "src/core/lib/transport/static_metadata.h"'
print >>C
+print >>C, '#include "src/core/lib/slice/slice_internal.h"'
+print >>C
print >>H, '#define GRPC_STATIC_MDSTR_COUNT %d' % len(all_strs)
print >>H, 'extern const grpc_slice grpc_static_slice_table[GRPC_STATIC_MDSTR_COUNT];'
@@ -310,7 +312,7 @@ print >>C, 'static uint8_t g_raw_bytes[] = {%s};' % (','.join('%d' % ord(c) for
print >>C
print >>C, 'static void static_ref(void *unused) {}'
print >>C, 'static void static_unref(grpc_exec_ctx *exec_ctx, void *unused) {}'
-print >>C, 'static const grpc_slice_refcount_vtable static_vtable = {static_ref, static_unref, grpc_slice_default_hash_impl};';
+print >>C, 'static const grpc_slice_refcount_vtable static_vtable = {static_ref, static_unref, grpc_static_slice_hash};';
print >>C, 'static grpc_slice_refcount g_refcnt = {&static_vtable};'
print >>C
print >>C, 'bool grpc_is_static_metadata_string(grpc_slice slice) {'
@@ -334,6 +336,7 @@ print >>C, '};'
print >>C
print >>C, 'static const uint8_t g_revmap[] = {%s};' % ','.join('%d' % (revmap[i] if i in revmap else 255) for i in range(0, str_ofs))
print >>C
+print >>H, 'int grpc_static_metadata_index(grpc_slice slice);'
print >>C, 'int grpc_static_metadata_index(grpc_slice slice) {'
print >>C, ' if (GRPC_SLICE_LENGTH(slice) == 0) return %d;' % zero_length_idx
print >>C, ' size_t ofs = (size_t)(GRPC_SLICE_START_PTR(slice) - g_raw_bytes);'
--
GitLab