Simplify batch callout index calculation

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) {
+  if (batch->idx.array[idx] == NULL) {
-    return grpc_attach_md_to_error(
+    batch->idx.array[idx] = storage;
-        GRPC_ERROR_CREATE("Unallowed duplicate metadata"), storage->md);
+    return GRPC_ERROR_NONE;
   }
-  batch->idx.array[idx] = storage;
+  return grpc_attach_md_to_error(
-  return GRPC_ERROR_NONE;
+      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;
   }

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};

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_HOST,
-  GRPC_BATCH_METHOD,
-  GRPC_BATCH_GRPC_STATUS,
-  GRPC_BATCH_TE,
   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 *host;
-    struct grpc_linked_mdelem *method;
-    struct grpc_linked_mdelem *grpc_status;
-    struct grpc_linked_mdelem *te;
     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)                       \

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]
+for elem 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:
   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))