From 6ec8aa1c0677355269c4bb0afd5fb7e38e7a8d3b Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 23 Nov 2016 09:35:25 -0800 Subject: [PATCH] Simplify batch callout index calculation --- src/core/lib/transport/metadata_batch.c | 16 +++++----- src/core/lib/transport/static_metadata.c | 34 -------------------- src/core/lib/transport/static_metadata.h | 38 +++++++++++++---------- tools/codegen/core/gen_static_metadata.py | 26 ++++------------ 4 files changed, 35 insertions(+), 79 deletions(-) diff --git a/src/core/lib/transport/metadata_batch.c b/src/core/lib/transport/metadata_batch.c index c41d28d9f7e..25cc65d5f19 100644 --- a/src/core/lib/transport/metadata_batch.c +++ b/src/core/lib/transport/metadata_batch.c @@ -70,7 +70,7 @@ static void assert_valid_callouts(grpc_metadata_batch *batch) { for (grpc_linked_mdelem *l = batch->list.head; l != NULL; l = l->next) { grpc_slice key_interned = grpc_slice_intern(GRPC_MDKEY(l->md)); grpc_metadata_batch_callouts_index callout_idx = - grpc_batch_index_of(key_interned); + GRPC_BATCH_INDEX_OF(key_interned); if (callout_idx != GRPC_BATCH_CALLOUTS_COUNT) { GPR_ASSERT(batch->idx.array[callout_idx] == l); } @@ -115,22 +115,22 @@ static grpc_error *maybe_link_callout(grpc_metadata_batch *batch, static grpc_error *maybe_link_callout(grpc_metadata_batch *batch, grpc_linked_mdelem *storage) { grpc_metadata_batch_callouts_index idx = - grpc_batch_index_of(GRPC_MDKEY(storage->md)); + GRPC_BATCH_INDEX_OF(GRPC_MDKEY(storage->md)); if (idx == GRPC_BATCH_CALLOUTS_COUNT) { return GRPC_ERROR_NONE; } - if (batch->idx.array[idx] != NULL) { - return grpc_attach_md_to_error( - GRPC_ERROR_CREATE("Unallowed duplicate metadata"), storage->md); + if (batch->idx.array[idx] == NULL) { + batch->idx.array[idx] = storage; + return GRPC_ERROR_NONE; } - batch->idx.array[idx] = storage; - return GRPC_ERROR_NONE; + return grpc_attach_md_to_error( + GRPC_ERROR_CREATE("Unallowed duplicate metadata"), storage->md); } static void maybe_unlink_callout(grpc_metadata_batch *batch, grpc_linked_mdelem *storage) { grpc_metadata_batch_callouts_index idx = - grpc_batch_index_of(GRPC_MDKEY(storage->md)); + GRPC_BATCH_INDEX_OF(GRPC_MDKEY(storage->md)); if (idx == GRPC_BATCH_CALLOUTS_COUNT) { return; } diff --git a/src/core/lib/transport/static_metadata.c b/src/core/lib/transport/static_metadata.c index 588c0986dd1..abd5a91eff3 100644 --- a/src/core/lib/transport/static_metadata.c +++ b/src/core/lib/transport/static_metadata.c @@ -807,39 +807,5 @@ grpc_mdelem_data grpc_static_mdelem_table[GRPC_STATIC_MDELEM_COUNT] = { {.refcount = &grpc_static_metadata_refcounts[97], .data.refcounted = {g_bytes + 1010, 21}}}, }; -#define BATCH_PHASHLEN 0x10 -#define BATCH_PHASHNKEYS 17 -#define BATCH_PHASHRANGE 32 -#define BATCH_PHASHSALT 0x9e3779b9 - -static const uint8_t batch_tab[] = { - 0, 13, 0, 13, 0, 13, 0, 13, 0, 13, 0, 15, 0, 13, 0, 23, -}; - -static uint32_t batch_phash(uint32_t val) { - val += (uint32_t)0; - - uint32_t a, b, rsl; - - b = (val & 0xf); - a = ((val << 27) >> 28); - rsl = (a ^ batch_tab[b]); - return rsl; -} - -static const uint8_t batch_hash_to_idx[] = { - 0, 2, 4, 6, 8, 10, 12, 14, 16, 9, 11, 13, 3, 1, 7, 5, - 15, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; - -grpc_metadata_batch_callouts_index grpc_batch_index_of(grpc_slice slice) { - if (!GRPC_IS_STATIC_METADATA_STRING(slice)) return GRPC_BATCH_CALLOUTS_COUNT; - uint32_t idx = (uint32_t)GRPC_STATIC_METADATA_INDEX(slice); - uint32_t hash = batch_phash(idx); - if (hash < GPR_ARRAY_SIZE(batch_hash_to_idx) && - batch_hash_to_idx[hash] == idx) - return (grpc_metadata_batch_callouts_index)hash; - return GRPC_BATCH_CALLOUTS_COUNT; -} - const uint8_t grpc_static_accept_encoding_metadata[8] = {0, 74, 75, 76, 77, 78, 79, 80}; diff --git a/src/core/lib/transport/static_metadata.h b/src/core/lib/transport/static_metadata.h index c9cceeb9a6f..e782a1f1c37 100644 --- a/src/core/lib/transport/static_metadata.h +++ b/src/core/lib/transport/static_metadata.h @@ -507,22 +507,22 @@ extern uintptr_t grpc_static_mdelem_user_data[GRPC_STATIC_MDELEM_COUNT]; grpc_mdelem grpc_static_mdelem_for_static_strings(int a, int b); typedef enum { GRPC_BATCH_PATH, + GRPC_BATCH_METHOD, GRPC_BATCH_STATUS, + GRPC_BATCH_AUTHORITY, GRPC_BATCH_SCHEME, + GRPC_BATCH_TE, GRPC_BATCH_GRPC_MESSAGE, + GRPC_BATCH_GRPC_STATUS, GRPC_BATCH_GRPC_PAYLOAD_BIN, - GRPC_BATCH_GRPC_ACCEPT_ENCODING, - GRPC_BATCH_GRPC_INTERNAL_ENCODING_REQUEST, - GRPC_BATCH_HOST, - GRPC_BATCH_LB_COST_BIN, GRPC_BATCH_GRPC_ENCODING, + GRPC_BATCH_GRPC_ACCEPT_ENCODING, GRPC_BATCH_CONTENT_TYPE, + GRPC_BATCH_GRPC_INTERNAL_ENCODING_REQUEST, GRPC_BATCH_USER_AGENT, - GRPC_BATCH_AUTHORITY, - GRPC_BATCH_METHOD, - GRPC_BATCH_GRPC_STATUS, - GRPC_BATCH_TE, + GRPC_BATCH_HOST, GRPC_BATCH_LB_TOKEN, + GRPC_BATCH_LB_COST_BIN, GRPC_BATCH_CALLOUTS_COUNT } grpc_metadata_batch_callouts_index; @@ -530,26 +530,30 @@ typedef union { struct grpc_linked_mdelem *array[GRPC_BATCH_CALLOUTS_COUNT]; struct { struct grpc_linked_mdelem *path; + struct grpc_linked_mdelem *method; struct grpc_linked_mdelem *status; + struct grpc_linked_mdelem *authority; struct grpc_linked_mdelem *scheme; + struct grpc_linked_mdelem *te; struct grpc_linked_mdelem *grpc_message; + struct grpc_linked_mdelem *grpc_status; struct grpc_linked_mdelem *grpc_payload_bin; - struct grpc_linked_mdelem *grpc_accept_encoding; - struct grpc_linked_mdelem *grpc_internal_encoding_request; - struct grpc_linked_mdelem *host; - struct grpc_linked_mdelem *lb_cost_bin; struct grpc_linked_mdelem *grpc_encoding; + struct grpc_linked_mdelem *grpc_accept_encoding; struct grpc_linked_mdelem *content_type; + struct grpc_linked_mdelem *grpc_internal_encoding_request; struct grpc_linked_mdelem *user_agent; - struct grpc_linked_mdelem *authority; - struct grpc_linked_mdelem *method; - struct grpc_linked_mdelem *grpc_status; - struct grpc_linked_mdelem *te; + struct grpc_linked_mdelem *host; struct grpc_linked_mdelem *lb_token; + struct grpc_linked_mdelem *lb_cost_bin; } named; } grpc_metadata_batch_callouts; -grpc_metadata_batch_callouts_index grpc_batch_index_of(grpc_slice slice); +#define GRPC_BATCH_INDEX_OF(slice) \ + (GRPC_IS_STATIC_METADATA_STRING((slice)) \ + ? GPR_CLAMP(GRPC_STATIC_METADATA_INDEX((slice)), 0, \ + GRPC_BATCH_CALLOUTS_COUNT) \ + : GRPC_BATCH_CALLOUTS_COUNT) extern const uint8_t grpc_static_accept_encoding_metadata[8]; #define GRPC_MDELEM_ACCEPT_ENCODING_FOR_ALGORITHMS(algs) \ diff --git a/tools/codegen/core/gen_static_metadata.py b/tools/codegen/core/gen_static_metadata.py index 7d363a9af2e..bd05853f117 100755 --- a/tools/codegen/core/gen_static_metadata.py +++ b/tools/codegen/core/gen_static_metadata.py @@ -220,6 +220,8 @@ def put_banner(files, banner): all_strs = list() all_elems = list() static_userdata = {} +# put metadata batch callouts first, to make the check of if a static metadata +# string is a callout trivial for elem in METADATA_BATCH_CALLOUTS: if elem not in all_strs: all_strs.append(elem) @@ -482,10 +484,7 @@ for a, b in all_elems: print >>C, '};' print >>H, 'typedef enum {' -batch_keys = [str_idx(s) for s in METADATA_BATCH_CALLOUTS] -batch_hash = perfect_hash(batch_keys, 'batch') -ordered_callouts = sorted((batch_hash['pyfunc'](str_idx(elem)), elem) for elem in METADATA_BATCH_CALLOUTS) -for _, elem in ordered_callouts: +for elem in METADATA_BATCH_CALLOUTS: print >>H, ' %s,' % mangle(elem, 'batch').upper() print >>H, ' GRPC_BATCH_CALLOUTS_COUNT' print >>H, '} grpc_metadata_batch_callouts_index;' @@ -493,27 +492,14 @@ print >>H print >>H, 'typedef union {' print >>H, ' struct grpc_linked_mdelem *array[GRPC_BATCH_CALLOUTS_COUNT];' print >>H, ' struct {' -for _, elem in ordered_callouts: +for elem in METADATA_BATCH_CALLOUTS: print >>H, ' struct grpc_linked_mdelem *%s;' % mangle(elem, '').lower() print >>H, ' } named;' print >>H, '} grpc_metadata_batch_callouts;' print >>H -print >>H, 'grpc_metadata_batch_callouts_index grpc_batch_index_of(grpc_slice slice);' +print >>H, '#define GRPC_BATCH_INDEX_OF(slice) \\' +print >>H, ' (GRPC_IS_STATIC_METADATA_STRING((slice)) ? GPR_CLAMP(GRPC_STATIC_METADATA_INDEX((slice)), 0, GRPC_BATCH_CALLOUTS_COUNT) : GRPC_BATCH_CALLOUTS_COUNT)' print >>H -print >>C, batch_hash['code'] -batch_hash_to_idx = [0] * int(batch_hash['PHASHRANGE']) -for i, elem in enumerate( METADATA_BATCH_CALLOUTS): - batch_hash_to_idx[batch_hash['pyfunc'](str_idx(elem))] = str_idx(elem) -print >>C, 'static const uint8_t batch_hash_to_idx[] = {%s};' % ','.join('%d' % n for n in batch_hash_to_idx) -print >>C -print >>C, 'grpc_metadata_batch_callouts_index grpc_batch_index_of(grpc_slice slice) {' -print >>C, ' if (!GRPC_IS_STATIC_METADATA_STRING(slice)) return GRPC_BATCH_CALLOUTS_COUNT;' -print >>C, ' uint32_t idx = (uint32_t)GRPC_STATIC_METADATA_INDEX(slice);' -print >>C, ' uint32_t hash = batch_phash(idx);' -print >>C, ' if (hash < GPR_ARRAY_SIZE(batch_hash_to_idx) && batch_hash_to_idx[hash] == idx) return (grpc_metadata_batch_callouts_index)hash;' -print >>C, ' return GRPC_BATCH_CALLOUTS_COUNT;' -print >>C, '}' -print >>C print >>H, 'extern const uint8_t grpc_static_accept_encoding_metadata[%d];' % (1 << len(COMPRESSION_ALGORITHMS)) print >>C, 'const uint8_t grpc_static_accept_encoding_metadata[%d] = {' % (1 << len(COMPRESSION_ALGORITHMS))