From cb7a80266b4fb867481807dc439d3c002f74d466 Mon Sep 17 00:00:00 2001 From: yang-g Date: Wed, 30 Mar 2016 14:58:53 -0700 Subject: Use base64 encoded length for decoder table size --- src/core/lib/transport/metadata.c | 29 +++++++++++++++++++++++++++++ src/core/lib/transport/metadata.h | 2 ++ 2 files changed, 31 insertions(+) (limited to 'src/core/lib/transport') diff --git a/src/core/lib/transport/metadata.c b/src/core/lib/transport/metadata.c index 451c8d1cd3..b1005f317a 100644 --- a/src/core/lib/transport/metadata.c +++ b/src/core/lib/transport/metadata.c @@ -38,6 +38,7 @@ #include #include +#include #include #include #include @@ -93,6 +94,9 @@ typedef struct internal_string { gpr_slice base64_and_huffman; + uint8_t has_size_in_decoder_table; + size_t size_in_decoder_table; + struct internal_string *bucket_next; } internal_string; @@ -407,6 +411,8 @@ grpc_mdstr *grpc_mdstr_from_buffer(const uint8_t *buf, size_t length) { } s->has_base64_and_huffman_encoded = 0; s->hash = hash; + s->has_size_in_decoder_table = 0; + s->size_in_decoder_table = 0; s->bucket_next = shard->strs[idx]; shard->strs[idx] = s; @@ -576,6 +582,29 @@ grpc_mdelem *grpc_mdelem_from_string_and_buffer(const char *key, grpc_mdstr_from_string(key), grpc_mdstr_from_buffer(value, value_length)); } +size_t grpc_mdelem_get_size_in_hpack_table(grpc_mdelem *elem) { + size_t overhead_and_key = 32 + GPR_SLICE_LENGTH(elem->key->slice); + if (is_mdstr_static(elem->value)) { + return overhead_and_key + GPR_SLICE_LENGTH(elem->value->slice); + } else { + internal_string *is = (internal_string *)elem->value; + size_t value_len = GPR_SLICE_LENGTH(is->slice); + static const uint8_t tail_xtra[3] = {0, 2, 3}; + if (is->has_size_in_decoder_table == 0) { + is->has_size_in_decoder_table = 1; + if (grpc_is_binary_header( + (const char *)GPR_SLICE_START_PTR(elem->key->slice), + GPR_SLICE_LENGTH(elem->key->slice))) { + is->size_in_decoder_table = + value_len / 3 * 4 + tail_xtra[value_len % 3]; + } else { + is->size_in_decoder_table = value_len; + } + } + return overhead_and_key + is->size_in_decoder_table; + } +} + grpc_mdelem *grpc_mdelem_ref(grpc_mdelem *gmd DEBUG_ARGS) { internal_metadata *md = (internal_metadata *)gmd; if (is_mdelem_static(gmd)) return gmd; diff --git a/src/core/lib/transport/metadata.h b/src/core/lib/transport/metadata.h index d72ec9accc..9705a7f25e 100644 --- a/src/core/lib/transport/metadata.h +++ b/src/core/lib/transport/metadata.h @@ -110,6 +110,8 @@ grpc_mdelem *grpc_mdelem_from_string_and_buffer(const char *key, const uint8_t *value, size_t value_length); +size_t grpc_mdelem_get_size_in_hpack_table(grpc_mdelem *elem); + /* Mutator and accessor for grpc_mdelem user data. The destructor function is used as a type tag and is checked during user_data fetch. */ void *grpc_mdelem_get_user_data(grpc_mdelem *md, -- cgit v1.2.3 From 24ba7c12e2e63345d28c9464d135f9efa82eef22 Mon Sep 17 00:00:00 2001 From: yang-g Date: Wed, 30 Mar 2016 16:19:43 -0700 Subject: Handle staic metatada string --- src/core/lib/transport/metadata.c | 19 ++++++++++++++----- test/core/transport/metadata_test.c | 5 +++++ 2 files changed, 19 insertions(+), 5 deletions(-) (limited to 'src/core/lib/transport') diff --git a/src/core/lib/transport/metadata.c b/src/core/lib/transport/metadata.c index b1005f317a..a0c295275f 100644 --- a/src/core/lib/transport/metadata.c +++ b/src/core/lib/transport/metadata.c @@ -582,21 +582,30 @@ grpc_mdelem *grpc_mdelem_from_string_and_buffer(const char *key, grpc_mdstr_from_string(key), grpc_mdstr_from_buffer(value, value_length)); } +static size_t get_base64_encoded_size(size_t raw_length) { + static const uint8_t tail_xtra[3] = {0, 2, 3}; + return raw_length / 3 * 4 + tail_xtra[raw_length % 3]; +} + size_t grpc_mdelem_get_size_in_hpack_table(grpc_mdelem *elem) { size_t overhead_and_key = 32 + GPR_SLICE_LENGTH(elem->key->slice); + size_t value_len = GPR_SLICE_LENGTH(elem->value->slice); if (is_mdstr_static(elem->value)) { - return overhead_and_key + GPR_SLICE_LENGTH(elem->value->slice); + if (grpc_is_binary_header( + (const char *)GPR_SLICE_START_PTR(elem->key->slice), + GPR_SLICE_LENGTH(elem->key->slice))) { + return overhead_and_key + get_base64_encoded_size(value_len); + } else { + return overhead_and_key + value_len; + } } else { internal_string *is = (internal_string *)elem->value; - size_t value_len = GPR_SLICE_LENGTH(is->slice); - static const uint8_t tail_xtra[3] = {0, 2, 3}; if (is->has_size_in_decoder_table == 0) { is->has_size_in_decoder_table = 1; if (grpc_is_binary_header( (const char *)GPR_SLICE_START_PTR(elem->key->slice), GPR_SLICE_LENGTH(elem->key->slice))) { - is->size_in_decoder_table = - value_len / 3 * 4 + tail_xtra[value_len % 3]; + is->size_in_decoder_table = get_base64_encoded_size(value_len); } else { is->size_in_decoder_table = value_len; } diff --git a/test/core/transport/metadata_test.c b/test/core/transport/metadata_test.c index 6037f9b896..7787b32308 100644 --- a/test/core/transport/metadata_test.c +++ b/test/core/transport/metadata_test.c @@ -43,6 +43,7 @@ #include "src/core/ext/transport/chttp2/transport/bin_encoder.h" #include "src/core/lib/support/string.h" +#include "src/core/lib/transport/static_metadata.h" #include "test/core/util/test_config.h" #define LOG_TEST(x) gpr_log(GPR_INFO, "%s", x) @@ -302,6 +303,10 @@ static void test_mdelem_sizes_in_hpack(void) { verify_binary_header_size("hello-bin", binary_value, i); } + const char *static_metadata = grpc_static_metadata_strings[0]; + memcpy(binary_value, static_metadata, strlen(static_metadata)); + verify_binary_header_size("hello-bin", binary_value, strlen(static_metadata)); + grpc_shutdown(); } -- cgit v1.2.3 From 1a6293f0969bb940b7db0ec5f763cf75c2d28f56 Mon Sep 17 00:00:00 2001 From: yang-g Date: Wed, 30 Mar 2016 16:57:49 -0700 Subject: Use bool --- src/core/lib/transport/metadata.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/core/lib/transport') diff --git a/src/core/lib/transport/metadata.c b/src/core/lib/transport/metadata.c index a0c295275f..6c1c87e035 100644 --- a/src/core/lib/transport/metadata.c +++ b/src/core/lib/transport/metadata.c @@ -94,7 +94,7 @@ typedef struct internal_string { gpr_slice base64_and_huffman; - uint8_t has_size_in_decoder_table; + bool has_size_in_decoder_table; size_t size_in_decoder_table; struct internal_string *bucket_next; @@ -411,7 +411,7 @@ grpc_mdstr *grpc_mdstr_from_buffer(const uint8_t *buf, size_t length) { } s->has_base64_and_huffman_encoded = 0; s->hash = hash; - s->has_size_in_decoder_table = 0; + s->has_size_in_decoder_table = false; s->size_in_decoder_table = 0; s->bucket_next = shard->strs[idx]; shard->strs[idx] = s; @@ -600,8 +600,8 @@ size_t grpc_mdelem_get_size_in_hpack_table(grpc_mdelem *elem) { } } else { internal_string *is = (internal_string *)elem->value; - if (is->has_size_in_decoder_table == 0) { - is->has_size_in_decoder_table = 1; + if (is->has_size_in_decoder_table == false) { + is->has_size_in_decoder_table = true; if (grpc_is_binary_header( (const char *)GPR_SLICE_START_PTR(elem->key->slice), GPR_SLICE_LENGTH(elem->key->slice))) { -- cgit v1.2.3 From 4a530b08c2d25067abfa9e71206b8625864c273c Mon Sep 17 00:00:00 2001 From: yang-g Date: Thu, 31 Mar 2016 11:44:44 -0700 Subject: fix leak and race --- src/core/lib/transport/metadata.c | 18 +++++++++--------- test/core/transport/chttp2/hpack_encoder_test.c | 1 + 2 files changed, 10 insertions(+), 9 deletions(-) (limited to 'src/core/lib/transport') diff --git a/src/core/lib/transport/metadata.c b/src/core/lib/transport/metadata.c index 6c1c87e035..4ec5bed89e 100644 --- a/src/core/lib/transport/metadata.c +++ b/src/core/lib/transport/metadata.c @@ -80,6 +80,7 @@ typedef void (*destroy_user_data_func)(void *user_data); +#define SIZE_IN_DECODER_TABLE_NOT_SET -1 /* Shadow structure for grpc_mdstr for non-static values */ typedef struct internal_string { /* must be byte compatible with grpc_mdstr */ @@ -94,8 +95,7 @@ typedef struct internal_string { gpr_slice base64_and_huffman; - bool has_size_in_decoder_table; - size_t size_in_decoder_table; + gpr_atm size_in_decoder_table; struct internal_string *bucket_next; } internal_string; @@ -411,8 +411,7 @@ grpc_mdstr *grpc_mdstr_from_buffer(const uint8_t *buf, size_t length) { } s->has_base64_and_huffman_encoded = 0; s->hash = hash; - s->has_size_in_decoder_table = false; - s->size_in_decoder_table = 0; + s->size_in_decoder_table = SIZE_IN_DECODER_TABLE_NOT_SET; s->bucket_next = shard->strs[idx]; shard->strs[idx] = s; @@ -600,17 +599,18 @@ size_t grpc_mdelem_get_size_in_hpack_table(grpc_mdelem *elem) { } } else { internal_string *is = (internal_string *)elem->value; - if (is->has_size_in_decoder_table == false) { - is->has_size_in_decoder_table = true; + gpr_atm current_size = gpr_atm_no_barrier_load(&is->size_in_decoder_table); + if (current_size == SIZE_IN_DECODER_TABLE_NOT_SET) { if (grpc_is_binary_header( (const char *)GPR_SLICE_START_PTR(elem->key->slice), GPR_SLICE_LENGTH(elem->key->slice))) { - is->size_in_decoder_table = get_base64_encoded_size(value_len); + current_size = (gpr_atm)get_base64_encoded_size(value_len); } else { - is->size_in_decoder_table = value_len; + current_size = (gpr_atm)value_len; } + gpr_atm_no_barrier_store(&is->size_in_decoder_table, current_size); } - return overhead_and_key + is->size_in_decoder_table; + return overhead_and_key + (size_t)current_size; } } diff --git a/test/core/transport/chttp2/hpack_encoder_test.c b/test/core/transport/chttp2/hpack_encoder_test.c index 45b0fa41e4..262f3ff034 100644 --- a/test/core/transport/chttp2/hpack_encoder_test.c +++ b/test/core/transport/chttp2/hpack_encoder_test.c @@ -207,6 +207,7 @@ static void verify_table_size_change_match_elem_size(const char *key, grpc_metadata_batch_destroy(&b); GPR_ASSERT(g_compressor.table_size == elem_size + initial_table_size); + gpr_free(e); } static void test_encode_header_size(void) { -- cgit v1.2.3 From 9cedd3fb6efd5dd45fb35744f3450d63e3002334 Mon Sep 17 00:00:00 2001 From: yang-g Date: Fri, 1 Apr 2016 00:24:59 -0700 Subject: use acq_load and rel_store --- src/core/lib/transport/metadata.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/core/lib/transport') diff --git a/src/core/lib/transport/metadata.c b/src/core/lib/transport/metadata.c index 4ec5bed89e..cffdd9638d 100644 --- a/src/core/lib/transport/metadata.c +++ b/src/core/lib/transport/metadata.c @@ -599,7 +599,7 @@ size_t grpc_mdelem_get_size_in_hpack_table(grpc_mdelem *elem) { } } else { internal_string *is = (internal_string *)elem->value; - gpr_atm current_size = gpr_atm_no_barrier_load(&is->size_in_decoder_table); + gpr_atm current_size = gpr_atm_acq_load(&is->size_in_decoder_table); if (current_size == SIZE_IN_DECODER_TABLE_NOT_SET) { if (grpc_is_binary_header( (const char *)GPR_SLICE_START_PTR(elem->key->slice), @@ -608,7 +608,7 @@ size_t grpc_mdelem_get_size_in_hpack_table(grpc_mdelem *elem) { } else { current_size = (gpr_atm)value_len; } - gpr_atm_no_barrier_store(&is->size_in_decoder_table, current_size); + gpr_atm_rel_store(&is->size_in_decoder_table, current_size); } return overhead_and_key + (size_t)current_size; } -- cgit v1.2.3