update

BUILD

@@ -299,6 +299,7 @@ grpc_cc_library(
         "src/core/ext/census/gen/trace_context.pb.h",
         "src/core/ext/census/grpc_filter.h",
         "src/core/ext/census/intrusive_hash_map.h",
+        "src/core/ext/census/intrusive_hash_map_internal.h",
         "src/core/ext/census/mlog.h",
         "src/core/ext/census/resource.h",
         "src/core/ext/census/rpc_metric_id.h",

build.yaml

@@ -28,6 +28,7 @@ filegroups:
   - src/core/ext/census/gen/trace_context.pb.h
   - src/core/ext/census/grpc_filter.h
   - src/core/ext/census/intrusive_hash_map.h
+  - src/core/ext/census/intrusive_hash_map_internal.h
   - src/core/ext/census/mlog.h
   - src/core/ext/census/resource.h
   - src/core/ext/census/rpc_metric_id.h

config.w32

@@ -299,6 +299,7 @@ if (PHP_GRPC != "no") {
     "src\\core\\ext\\census\\grpc_filter.c " +
     "src\\core\\ext\\census\\grpc_plugin.c " +
     "src\\core\\ext\\census\\initialize.c " +
+    "src\\core\\ext\\census\\intrusive_hash_map.c " +
     "src\\core\\ext\\census\\mlog.c " +
     "src\\core\\ext\\census\\operation.c " +
     "src\\core\\ext\\census\\placeholders.c " +

gRPC-Core.podspec

@@ -464,6 +464,7 @@ Pod::Spec.new do |s|
                       'src/core/ext/census/gen/trace_context.pb.h',
                       'src/core/ext/census/grpc_filter.h',
                       'src/core/ext/census/intrusive_hash_map.h',
+                      'src/core/ext/census/intrusive_hash_map_internal.h',
                       'src/core/ext/census/mlog.h',
                       'src/core/ext/census/resource.h',
                       'src/core/ext/census/rpc_metric_id.h',
@@ -949,6 +950,7 @@ Pod::Spec.new do |s|
                               'src/core/ext/census/gen/trace_context.pb.h',
                               'src/core/ext/census/grpc_filter.h',
                               'src/core/ext/census/intrusive_hash_map.h',
+                              'src/core/ext/census/intrusive_hash_map_internal.h',
                               'src/core/ext/census/mlog.h',
                               'src/core/ext/census/resource.h',
                               'src/core/ext/census/rpc_metric_id.h',

grpc.gemspec

@@ -380,6 +380,7 @@ Gem::Specification.new do |s|
   s.files += %w( src/core/ext/census/gen/trace_context.pb.h )
   s.files += %w( src/core/ext/census/grpc_filter.h )
   s.files += %w( src/core/ext/census/intrusive_hash_map.h )
+  s.files += %w( src/core/ext/census/intrusive_hash_map_internal.h )
   s.files += %w( src/core/ext/census/mlog.h )
   s.files += %w( src/core/ext/census/resource.h )
   s.files += %w( src/core/ext/census/rpc_metric_id.h )

package.xml

@@ -394,6 +394,7 @@
     <file baseinstalldir="/" name="src/core/ext/census/gen/trace_context.pb.h" role="src" />
     <file baseinstalldir="/" name="src/core/ext/census/grpc_filter.h" role="src" />
     <file baseinstalldir="/" name="src/core/ext/census/intrusive_hash_map.h" role="src" />
+    <file baseinstalldir="/" name="src/core/ext/census/intrusive_hash_map_internal.h" role="src" />
     <file baseinstalldir="/" name="src/core/ext/census/mlog.h" role="src" />
     <file baseinstalldir="/" name="src/core/ext/census/resource.h" role="src" />
     <file baseinstalldir="/" name="src/core/ext/census/rpc_metric_id.h" role="src" />

src/core/ext/census/intrusive_hash_map.c

@@ -27,8 +27,7 @@ static inline uint32_t chunked_vector_hasher(uint64_t key) {
 /* Vector chunks are 1MiB divided by pointer size. */
 static const size_t VECTOR_CHUNK_SIZE = (1 << 20) / sizeof(void *);
 
-/* Helper functions which return buckets from the chunked vector. These are
+/* Helper functions which return buckets from the chunked vector. */
-   meant for internal use only within the intrusive_hash_map data structure. */
 static inline void **get_mutable_bucket(const chunked_vector *buckets,
                                         uint32_t index) {
   if (index < VECTOR_CHUNK_SIZE) {
@@ -68,7 +67,7 @@ static void chunked_vector_clear(chunked_vector *vec) {
   }
   if (vec->rest_ != NULL) {
     size_t rest_size = RestSize(vec);
-    for (uint32_t i = 0; i < rest_size; ++i) {
+    for (size_t i = 0; i < rest_size; ++i) {
       if (vec->rest_[i] != NULL) {
         gpr_free(vec->rest_[i]);
       }

src/core/ext/census/intrusive_hash_map.h

@@ -17,21 +17,17 @@
 #ifndef GRPC_CORE_EXT_CENSUS_INTRUSIVE_HASH_MAP_H
 #define GRPC_CORE_EXT_CENSUS_INTRUSIVE_HASH_MAP_H
 
-#include <grpc/support/alloc.h>
+#include "src/core/ext/census/intrusive_hash_map_internal.h"
-#include <grpc/support/log.h>
+
-#include <grpc/support/useful.h>
+/* intrusive_hash_map is a fast chained hash table. This hash map is faster than
-#include <stdbool.h>
+ * a dense hash map when the application calls insert and erase more often than
-
+ * find. When the workload is dominated by find() a dense hash map may be
-/* intrusive_hash_map is a fast chained hash table. It is almost always faster
+ * faster.
- * than STL hash_map, since this hash map avoids malloc and free during insert
- * and erase. This hash map is faster than a dense hash map when the application
- * calls insert and erase more often than find. When the workload is dominated
- * by find() a dense hash map may be faster.
  *
  * intrusive_hash_map uses an intrusive header placed within a user defined
- * struct. IHM_key MUST be set to a valid value before insertion into the hash
+ * struct. The header field IHM_key MUST be set to a valid value before
- * map or undefined behavior may occur.  IHM_hash_link needs to be set to NULL
+ * insertion into the hash map or undefined behavior may occur. The header field
- * initially.
+ * IHM_hash_link MUST to be set to NULL initially.
  *
  * EXAMPLE USAGE:
  *
@@ -53,6 +49,8 @@
  *    return item;
  *  }
  *
+ *  intrusive_hash_map hash_map;
+ *  intrusive_hash_map_init(&hash_map, 4);
  *  string_item *new_item1 = make_string_item(10, "test1", 5);
  *  bool ok = intrusive_hash_map_insert(&hash_map, (hm_item *)new_item1);
  *
@@ -61,9 +59,10 @@
  */
 
 /* Hash map item. Stores key and a pointer to the actual object. A user defined
- * version of this can be passed in as long as the first 2 entries (key and
+ * version of this can be passed in provided the first 2 entries (key and
- * hash_link) are the same. Pointer to struct will need to be cast as
+ * hash_link) are the same. These entries must be first in the user defined
- * (hm_item *) when passed to hash map. This allows it to be intrusive. */
+ * struct. Pointer to struct will need to be cast as (hm_item *) when passed to
+ * hash map. This allows it to be intrusive. */
 typedef struct hm_item {
   uint64_t key;
   struct hm_item *hash_link;
@@ -71,33 +70,12 @@ typedef struct hm_item {
 } hm_item;
 
 /* Macro provided for ease of use.  This must be first in the user defined
- * struct. */
+ * struct (i.e. uint64_t key and hm_item * must be the first two elements in
+ * that order). */
 #define INTRUSIVE_HASH_MAP_HEADER \
   uint64_t IHM_key;               \
   struct hm_item *IHM_hash_link
 
-/* The chunked vector is a data structure that allocates buckets for use in the
- * hash map. ChunkedVector is logically equivalent to T*[N] (cast void* as
- * T*). It's internally implemented as an array of 1MB arrays to avoid
- * allocating large consecutive memory chunks. This is an internal data
- * structure that should never be accessed directly. */
-typedef struct chunked_vector {
-  size_t size_;
-  void **first_;
-  void ***rest_;
-} chunked_vector;
-
-/* Core intrusive hash map data structure. All internal elements are managed by
- * functions and should not be altered manually. intrusive_hash_map_init()
- * must first be called before an intrusive_hash_map can be used. */
-typedef struct intrusive_hash_map {
-  uint32_t num_items;
-  uint32_t extend_threshold;
-  uint32_t log2_num_buckets;
-  uint32_t hash_mask;
-  chunked_vector buckets;
-} intrusive_hash_map;
-
 /* Index struct which acts as a pseudo-iterator within the hash map. */
 typedef struct hm_index {
   uint32_t bucket_index;  // hash map bucket index.
@@ -110,7 +88,10 @@ inline bool hm_index_compare(const hm_index *A, const hm_index *B) {
   return (A->item == B->item && A->bucket_index == B->bucket_index);
 }
 
-/* Helper functions for iterating over the hash map. */
+/*
+  Helper functions for iterating over the hash map.
+  */
+
 /* On return idx will contain an invalid index which is always equal to
  * hash_map->buckets.size_ */
 void intrusive_hash_map_end(const intrusive_hash_map *hash_map, hm_index *idx);
@@ -152,18 +133,18 @@ hm_item *intrusive_hash_map_erase(intrusive_hash_map *hash_map, uint64_t key);
  * Otherwise, it will insert the new item and return true. */
 bool intrusive_hash_map_insert(intrusive_hash_map *hash_map, hm_item *item);
 
-/* Clear entire contents of the hash map, but leaves internal data structure
+/* Clears entire contents of the hash map, but leaves internal data structure
- * untouched. Second argument takes a function pointer to a method that will
+ * untouched. Second argument takes a function pointer to a function that will
  * free the object designated by the user and pointed to by hash_map->value. */
 void intrusive_hash_map_clear(intrusive_hash_map *hash_map,
                               void (*free_object)(void *));
 
 /* Erase all contents of hash map and free the memory. Hash map is invalid
  * after calling this function and cannot be used until it has been
- * reinitialized (intrusive_hash_map_init()). takes a function pointer to a
+ * reinitialized (intrusive_hash_map_init()). This function takes a function
- * method that will free the object designated by the user and pointed to by
+ * pointer to a function that will free the object designated by the user and
- * hash_map->value.*/
+ * pointed to by hash_map->value. */
 void intrusive_hash_map_free(intrusive_hash_map *hash_map,
                              void (*free_object)(void *));
 
-#endif
+#endif // GRPC_CORE_EXT_CENSUS_INTRUSIVE_HASH_MAP_H

src/core/ext/census/intrusive_hash_map_internal.h

@@ -0,0 +1,46 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef GRPC_CORE_EXT_CENSUS_INTRUSIVE_HASH_MAP_INTERNAL_H
+#define GRPC_CORE_EXT_CENSUS_INTRUSIVE_HASH_MAP_INTERNAL_H
+
+#include <grpc/support/alloc.h>
+#include <grpc/support/log.h>
+#include <grpc/support/useful.h>
+#include <stdbool.h>
+
+/* The chunked vector is a data structure that allocates buckets for use in the
+ * hash map. ChunkedVector is logically equivalent to T*[N] (cast void* as
+ * T*). It's internally implemented as an array of 1MB arrays to avoid
+ * allocating large consecutive memory chunks. This is an internal data
+ * structure that should never be accessed directly. */
+typedef struct chunked_vector {
+  size_t size_;
+  void **first_;
+  void ***rest_;
+} chunked_vector;
+
+/* Core intrusive hash map data structure. All internal elements are managed by
+ * functions and should not be altered manually. */
+typedef struct intrusive_hash_map {
+  uint32_t num_items;
+  uint32_t extend_threshold;
+  uint32_t log2_num_buckets;
+  uint32_t hash_mask;
+  chunked_vector buckets;
+} intrusive_hash_map;
+
+#endif // GRPC_CORE_EXT_CENSUS_INTRUSIVE_HASH_MAP_INTERNAL_H

test/core/census/intrusive_hash_map_test.c

@@ -28,14 +28,17 @@
 /* The initial size of an intrusive hash map will be 2 to this power. */
 static const uint32_t kInitialLog2Size = 4;
 
+/* Simple object used for testing intrusive_hash_map. */
 typedef struct object { uint64_t val; } object;
 
+/* Helper function to allocate and initialize object. */
 static inline object *make_new_object(uint64_t val) {
   object *obj = (object *)gpr_malloc(sizeof(object));
   obj->val = val;
   return obj;
 }
 
+/* Wrapper struct for object. */
 typedef struct ptr_item {
   INTRUSIVE_HASH_MAP_HEADER;
   object *obj;
@@ -51,8 +54,10 @@ static inline ptr_item *make_ptr_item(uint64_t key, uint64_t value) {
   return new_item;
 }
 
+/* Helper function to deallocate ptr_item. */
 static void free_ptr_item(void *ptr) { gpr_free(((ptr_item *)ptr)->obj); }
 
+/* Simple string object used for testing intrusive_hash_map. */
 typedef struct string_item {
   INTRUSIVE_HASH_MAP_HEADER;
   // User data.
@@ -60,6 +65,7 @@ typedef struct string_item {
   uint16_t len;
 } string_item;
 
+/* Helper function to allocate and initialize string object. */
 static string_item *make_string_item(uint64_t key, const char *buf,
                                      uint16_t len) {
   string_item *item = (string_item *)gpr_malloc(sizeof(string_item));
@@ -70,6 +76,7 @@ static string_item *make_string_item(uint64_t key, const char *buf,
   return item;
 }
 
+/* Helper function for comparing two string objects. */
 static bool compare_string_item(const string_item *A, const string_item *B) {
   if (A->IHM_key != B->IHM_key || A->len != B->len)
     return false;
@@ -90,7 +97,7 @@ void test_empty() {
   intrusive_hash_map_free(&hash_map, NULL);
 }
 
-void test_basic() {
+void test_single_item() {
   intrusive_hash_map hash_map;
   intrusive_hash_map_init(&hash_map, kInitialLog2Size);
 
@@ -113,7 +120,7 @@ void test_basic() {
   intrusive_hash_map_free(&hash_map, &free_ptr_item);
 }
 
-void test_basic2() {
+void test_two_items() {
   intrusive_hash_map hash_map;
   intrusive_hash_map_init(&hash_map, kInitialLog2Size);
 
@@ -215,10 +222,12 @@ void test_stress() {
   intrusive_hash_map_init(&hash_map, kInitialLog2Size);
   size_t n = 0;
 
+  // Randomly add and insert entries 1000000 times.
   for (uint64_t i = 0; i < 1000000; ++i) {
     int op = rand() & 0x1;
 
     switch (op) {
+      // Case 0 is insertion of entry.
       case 0: {
         uint64_t key = (uint64_t)(rand() % 10000);
         ptr_item *item = make_ptr_item(key, key);
@@ -231,6 +240,7 @@ void test_stress() {
         }
         break;
       }
+      // Case 1 is removal of entry.
       case 1: {
         uint64_t key = (uint64_t)(rand() % 10000);
         ptr_item *item = (ptr_item *)intrusive_hash_map_find(&hash_map, key);
@@ -262,8 +272,8 @@ int main(int argc, char **argv) {
   srand((unsigned)gpr_now(GPR_CLOCK_REALTIME).tv_nsec);
 
   test_empty();
-  test_basic();
+  test_single_item();
-  test_basic2();
+  test_two_items();
   test_reset_clear();
   test_extend();
   test_stress();

tools/doxygen/Doxyfile.core.internal

@@ -885,6 +885,7 @@ src/core/ext/census/grpc_plugin.c \
 src/core/ext/census/initialize.c \
 src/core/ext/census/intrusive_hash_map.c \
 src/core/ext/census/intrusive_hash_map.h \
+src/core/ext/census/intrusive_hash_map_internal.h \
 src/core/ext/census/mlog.c \
 src/core/ext/census/mlog.h \
 src/core/ext/census/operation.c \

tools/run_tests/generated/sources_and_headers.json

@@ -7559,6 +7559,7 @@
       "src/core/ext/census/gen/trace_context.pb.h", 
       "src/core/ext/census/grpc_filter.h", 
       "src/core/ext/census/intrusive_hash_map.h", 
+      "src/core/ext/census/intrusive_hash_map_internal.h", 
       "src/core/ext/census/mlog.h", 
       "src/core/ext/census/resource.h", 
       "src/core/ext/census/rpc_metric_id.h", 
@@ -7591,6 +7592,7 @@
       "src/core/ext/census/initialize.c", 
       "src/core/ext/census/intrusive_hash_map.c", 
       "src/core/ext/census/intrusive_hash_map.h", 
+      "src/core/ext/census/intrusive_hash_map_internal.h", 
       "src/core/ext/census/mlog.c", 
       "src/core/ext/census/mlog.h", 
       "src/core/ext/census/operation.c", 

vsprojects/vcxproj/grpc/grpc.vcxproj

@@ -505,6 +505,7 @@
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\gen\trace_context.pb.h" />
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\grpc_filter.h" />
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\intrusive_hash_map.h" />
+    <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\intrusive_hash_map_internal.h" />
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\mlog.h" />
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\resource.h" />
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\rpc_metric_id.h" />

vsprojects/vcxproj/grpc/grpc.vcxproj.filters

@@ -1460,6 +1460,9 @@
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\intrusive_hash_map.h">
       <Filter>src\core\ext\census</Filter>
     </ClInclude>
+    <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\intrusive_hash_map_internal.h">
+      <Filter>src\core\ext\census</Filter>
+    </ClInclude>
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\mlog.h">
       <Filter>src\core\ext\census</Filter>
     </ClInclude>

vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj

@@ -470,6 +470,7 @@
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\gen\trace_context.pb.h" />
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\grpc_filter.h" />
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\intrusive_hash_map.h" />
+    <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\intrusive_hash_map_internal.h" />
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\mlog.h" />
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\resource.h" />
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\rpc_metric_id.h" />

vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters

@@ -1295,6 +1295,9 @@
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\intrusive_hash_map.h">
       <Filter>src\core\ext\census</Filter>
     </ClInclude>
+    <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\intrusive_hash_map_internal.h">
+      <Filter>src\core\ext\census</Filter>
+    </ClInclude>
     <ClInclude Include="$(SolutionDir)\..\src\core\ext\census\mlog.h">
       <Filter>src\core\ext\census</Filter>
     </ClInclude>