Simplify hash table API: remove per-entry vtable and take ownership of

keys and values.

7 files changed, 31 insertions, 66 deletions

diff --git a/src/core/ext/filters/client_channel/client_channel.c b/src/core/ext/filters/client_channel/client_channel.c
index ce9abdad61..8d28e829d8 100644
--- a/src/core/ext/filters/client_channel/client_channel.c
+++ b/src/core/ext/filters/client_channel/client_channel.c
@@ -96,17 +96,10 @@ static void method_parameters_unref(method_parameters *method_params) {
   }
 }
 
-static void *method_parameters_copy(void *value) {
-  return method_parameters_ref(value);
-}
-
 static void method_parameters_free(grpc_exec_ctx *exec_ctx, void *value) {
   method_parameters_unref(value);
 }
 
-static const grpc_slice_hash_table_vtable method_parameters_vtable = {
-    method_parameters_free, method_parameters_copy};
-
 static bool parse_wait_for_ready(grpc_json *field,
                                  wait_for_ready_value *wait_for_ready) {
   if (field->type != GRPC_JSON_TRUE && field->type != GRPC_JSON_FALSE) {
@@ -472,7 +465,7 @@ static void on_resolver_result_changed_locked(grpc_exec_ctx *exec_ctx,
         grpc_uri_destroy(uri);
         method_params_table = grpc_service_config_create_method_config_table(
             exec_ctx, service_config, method_parameters_create_from_json,
-            &method_parameters_vtable);
+            method_parameters_free);
         grpc_service_config_destroy(service_config);
       }
     }
diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.c b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.c
index ff8d319309..3fe3f056d3 100644
--- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.c
+++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.c
@@ -750,18 +750,11 @@ static void destroy_balancer_name(grpc_exec_ctx *exec_ctx,
   gpr_free(balancer_name);
 }
 
-static void *copy_balancer_name(void *balancer_name) {
-  return gpr_strdup(balancer_name);
-}
-
 static grpc_slice_hash_table_entry targets_info_entry_create(
     const char *address, const char *balancer_name) {
-  static const grpc_slice_hash_table_vtable vtable = {destroy_balancer_name,
-                                                      copy_balancer_name};
   grpc_slice_hash_table_entry entry;
   entry.key = grpc_slice_from_copied_string(address);
-  entry.value = (void *)balancer_name;
-  entry.vtable = &vtable;
+  entry.value = gpr_strdup(balancer_name);
   return entry;
 }
 
@@ -825,11 +818,8 @@ static char *get_lb_uri_target_addresses(grpc_exec_ctx *exec_ctx,
                uri_path);
   gpr_free(uri_path);
 
-  *targets_info =
-      grpc_slice_hash_table_create(num_grpclb_addrs, targets_info_entries);
-  for (size_t i = 0; i < num_grpclb_addrs; i++) {
-    grpc_slice_unref_internal(exec_ctx, targets_info_entries[i].key);
-  }
+  *targets_info = grpc_slice_hash_table_create(
+      num_grpclb_addrs, targets_info_entries, destroy_balancer_name);
   gpr_free(targets_info_entries);
 
   return target_uri_str;
diff --git a/src/core/ext/filters/message_size/message_size_filter.c b/src/core/ext/filters/message_size/message_size_filter.c
index db0f011905..69ebbc7414 100644
--- a/src/core/ext/filters/message_size/message_size_filter.c
+++ b/src/core/ext/filters/message_size/message_size_filter.c
@@ -50,19 +50,10 @@ typedef struct message_size_limits {
   int max_recv_size;
 } message_size_limits;
 
-static void* message_size_limits_copy(void* value) {
-  void* new_value = gpr_malloc(sizeof(message_size_limits));
-  memcpy(new_value, value, sizeof(message_size_limits));
-  return new_value;
-}
-
 static void message_size_limits_free(grpc_exec_ctx* exec_ctx, void* value) {
   gpr_free(value);
 }
 
-static const grpc_slice_hash_table_vtable message_size_limits_vtable = {
-    message_size_limits_free, message_size_limits_copy};
-
 static void* message_size_limits_create_from_json(const grpc_json* json) {
   int max_request_message_bytes = -1;
   int max_response_message_bytes = -1;
@@ -255,7 +246,7 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx,
       chand->method_limit_table =
           grpc_service_config_create_method_config_table(
               exec_ctx, service_config, message_size_limits_create_from_json,
-              &message_size_limits_vtable);
+              message_size_limits_free);
       grpc_service_config_destroy(service_config);
     }
   }
diff --git a/src/core/lib/slice/slice_hash_table.c b/src/core/lib/slice/slice_hash_table.c
index 219567f36f..7a8b79cc33 100644
--- a/src/core/lib/slice/slice_hash_table.c
+++ b/src/core/lib/slice/slice_hash_table.c
@@ -42,12 +42,13 @@
 
 struct grpc_slice_hash_table {
   gpr_refcount refs;
+  void (*destroy_value)(grpc_exec_ctx *exec_ctx, void *value);
   size_t size;
   grpc_slice_hash_table_entry* entries;
 };
 
 static bool is_empty(grpc_slice_hash_table_entry* entry) {
-  return entry->vtable == NULL;
+  return entry->value == NULL;
 }
 
 // Helper function for insert and get operations that performs quadratic
@@ -67,23 +68,23 @@ static size_t grpc_slice_hash_table_find_index(
   return table->size;  // Not found.
 }
 
-static void grpc_slice_hash_table_add(
-    grpc_slice_hash_table* table, grpc_slice key, void* value,
-    const grpc_slice_hash_table_vtable* vtable) {
+static void grpc_slice_hash_table_add(grpc_slice_hash_table* table,
+                                      grpc_slice key, void* value) {
   GPR_ASSERT(value != NULL);
   const size_t idx =
       grpc_slice_hash_table_find_index(table, key, true /* find_empty */);
   GPR_ASSERT(idx != table->size);  // Table should never be full.
   grpc_slice_hash_table_entry* entry = &table->entries[idx];
-  entry->key = grpc_slice_ref_internal(key);
-  entry->value = vtable->copy_value(value);
-  entry->vtable = vtable;
+  entry->key = key;
+  entry->value = value;
 }
 
 grpc_slice_hash_table* grpc_slice_hash_table_create(
-    size_t num_entries, grpc_slice_hash_table_entry* entries) {
+    size_t num_entries, grpc_slice_hash_table_entry* entries,
+    void (*destroy_value)(grpc_exec_ctx *exec_ctx, void *value)) {
   grpc_slice_hash_table* table = gpr_zalloc(sizeof(*table));
   gpr_ref_init(&table->refs, 1);
+  table->destroy_value = destroy_value;
   // Quadratic probing gets best performance when the table is no more
   // than half full.
   table->size = num_entries * 2;
@@ -91,7 +92,7 @@ grpc_slice_hash_table* grpc_slice_hash_table_create(
   table->entries = gpr_zalloc(entry_size);
   for (size_t i = 0; i < num_entries; ++i) {
     grpc_slice_hash_table_entry* entry = &entries[i];
-    grpc_slice_hash_table_add(table, entry->key, entry->value, entry->vtable);
+    grpc_slice_hash_table_add(table, entry->key, entry->value);
   }
   return table;
 }
@@ -108,7 +109,7 @@ void grpc_slice_hash_table_unref(grpc_exec_ctx* exec_ctx,
       grpc_slice_hash_table_entry* entry = &table->entries[i];
       if (!is_empty(entry)) {
         grpc_slice_unref_internal(exec_ctx, entry->key);
-        entry->vtable->destroy_value(exec_ctx, entry->value);
+        table->destroy_value(exec_ctx, entry->value);
       }
     }
     gpr_free(table->entries);
diff --git a/src/core/lib/slice/slice_hash_table.h b/src/core/lib/slice/slice_hash_table.h
index d0c27122d7..81e92b6e21 100644
--- a/src/core/lib/slice/slice_hash_table.h
+++ b/src/core/lib/slice/slice_hash_table.h
@@ -41,29 +41,24 @@
  * probing (https://en.wikipedia.org/wiki/Quadratic_probing).
  *
  * The keys are \a grpc_slice objects.  The values are arbitrary pointers
- * with a common vtable.
+ * with a common destroy function.
  *
  * Hash tables are intentionally immutable, to avoid the need for locking.
  */
 
 typedef struct grpc_slice_hash_table grpc_slice_hash_table;
 
-typedef struct grpc_slice_hash_table_vtable {
-  void (*destroy_value)(grpc_exec_ctx *exec_ctx, void *value);
-  void *(*copy_value)(void *value);
-} grpc_slice_hash_table_vtable;
-
 typedef struct grpc_slice_hash_table_entry {
   grpc_slice key;
   void *value; /* Must not be NULL. */
-  const grpc_slice_hash_table_vtable *vtable;
 } grpc_slice_hash_table_entry;
 
 /** Creates a new hash table of containing \a entries, which is an array
-    of length \a num_entries.
-    Creates its own copy of all keys and values from \a entries. */
+    of length \a num_entries.  Takes ownership of all keys and values in
+    \a entries.  Values will be cleaned up via \a destroy_value(). */
 grpc_slice_hash_table *grpc_slice_hash_table_create(
-    size_t num_entries, grpc_slice_hash_table_entry *entries);
+    size_t num_entries, grpc_slice_hash_table_entry *entries,
+    void (*destroy_value)(grpc_exec_ctx *exec_ctx, void *value));
 
 grpc_slice_hash_table *grpc_slice_hash_table_ref(grpc_slice_hash_table *table);
 void grpc_slice_hash_table_unref(grpc_exec_ctx *exec_ctx,
diff --git a/src/core/lib/transport/service_config.c b/src/core/lib/transport/service_config.c
index 1195f75044..3cda394723 100644
--- a/src/core/lib/transport/service_config.c
+++ b/src/core/lib/transport/service_config.c
@@ -162,7 +162,7 @@ static char* parse_json_method_name(grpc_json* json) {
 static bool parse_json_method_config(
     grpc_exec_ctx* exec_ctx, grpc_json* json,
     void* (*create_value)(const grpc_json* method_config_json),
-    const grpc_slice_hash_table_vtable* vtable,
+    void (*destroy_value)(grpc_exec_ctx* exec_ctx, void* value),
     grpc_slice_hash_table_entry* entries, size_t* idx) {
   // Construct value.
   void* method_config = create_value(json);
@@ -185,13 +185,12 @@ static bool parse_json_method_config(
   // Add entry for each path.
   for (size_t i = 0; i < paths.count; ++i) {
     entries[*idx].key = grpc_slice_from_copied_string(paths.strs[i]);
-    entries[*idx].value = vtable->copy_value(method_config);
-    entries[*idx].vtable = vtable;
+    entries[*idx].value = method_config;
     ++*idx;
   }
   success = true;
 done:
-  vtable->destroy_value(exec_ctx, method_config);
+  destroy_value(exec_ctx, method_config);
   gpr_strvec_destroy(&paths);
   return success;
 }
@@ -199,7 +198,7 @@ done:
 grpc_slice_hash_table* grpc_service_config_create_method_config_table(
     grpc_exec_ctx* exec_ctx, const grpc_service_config* service_config,
     void* (*create_value)(const grpc_json* method_config_json),
-    const grpc_slice_hash_table_vtable* vtable) {
+    void (*destroy_value)(grpc_exec_ctx* exec_ctx, void* value)) {
   const grpc_json* json = service_config->json_tree;
   // Traverse parsed JSON tree.
   if (json->type != GRPC_JSON_OBJECT || json->key != NULL) return NULL;
@@ -220,8 +219,8 @@ grpc_slice_hash_table* grpc_service_config_create_method_config_table(
       size_t idx = 0;
       for (grpc_json* method = field->child; method != NULL;
            method = method->next) {
-        if (!parse_json_method_config(exec_ctx, method, create_value, vtable,
-                                      entries, &idx)) {
+        if (!parse_json_method_config(exec_ctx, method, create_value,
+                                      destroy_value, entries, &idx)) {
           return NULL;
         }
       }
@@ -231,12 +230,8 @@ grpc_slice_hash_table* grpc_service_config_create_method_config_table(
   // Instantiate method config table.
   grpc_slice_hash_table* method_config_table = NULL;
   if (entries != NULL) {
-    method_config_table = grpc_slice_hash_table_create(num_entries, entries);
-    // Clean up.
-    for (size_t i = 0; i < num_entries; ++i) {
-      grpc_slice_unref_internal(exec_ctx, entries[i].key);
-      vtable->destroy_value(exec_ctx, entries[i].value);
-    }
+    method_config_table =
+        grpc_slice_hash_table_create(num_entries, entries, destroy_value);
     gpr_free(entries);
   }
   return method_config_table;
diff --git a/src/core/lib/transport/service_config.h b/src/core/lib/transport/service_config.h
index ebfc59b534..e0548b9c3f 100644
--- a/src/core/lib/transport/service_config.h
+++ b/src/core/lib/transport/service_config.h
@@ -57,12 +57,12 @@ const char* grpc_service_config_get_lb_policy_name(
 /// Creates a method config table based on the data in \a json.
 /// The table's keys are request paths.  The table's value type is
 /// returned by \a create_value(), based on data parsed from the JSON tree.
-/// \a vtable provides methods used to manage the values.
+/// \a destroy_value is used to clean up values.
 /// Returns NULL on error.
 grpc_slice_hash_table* grpc_service_config_create_method_config_table(
     grpc_exec_ctx* exec_ctx, const grpc_service_config* service_config,
     void* (*create_value)(const grpc_json* method_config_json),
-    const grpc_slice_hash_table_vtable* vtable);
+    void (*destroy_value)(grpc_exec_ctx* exec_ctx, void* value));
 
 /// A helper function for looking up values in the table returned by
 /// \a grpc_service_config_create_method_config_table().