Shared mem hash set cleanup debug scaffolding

source/common/common/shared_memory_hash_set.h

@@ -21,9 +21,6 @@ struct SharedMemoryHashSetOptions {
   std::string toString() const {
     return fmt::format("capacity={}, num_slots={}", capacity, num_slots);
   }
-  std::string version() const {
-    return fmt::format("capacity={}, num_slots={}", capacity, num_slots);
-  }
   bool operator==(const SharedMemoryHashSetOptions& that) const {
     return capacity == that.capacity && num_slots == that.num_slots;
   }
@@ -101,58 +98,39 @@ template <class Value> class SharedMemoryHashSet : public Logger::Loggable<Logge
     RELEASE_ASSERT(control_->size <= control_->options.capacity);
 
     // As a sanity check, make sure there are control_->size values
-    // reachable from the slots, each of which has a valid char_offset
+    // reachable from the slots, each of which has a valid
+    // char_offset.
+    //
+    // Avoid infinite loops if there is a next_cell cycle within a
+    // slot. Note that the num_values message will be emitted outside
+    // the loop.
     uint32_t num_values = 0;
     for (uint32_t slot = 0; slot < control_->options.num_slots; ++slot) {
       uint32_t next = 0; // initialized to silence compilers.
-      for (uint32_t cell_index = slots_[slot]; cell_index != Sentinal; cell_index = next) {
+      for (uint32_t cell_index = slots_[slot];
+           (cell_index != Sentinal) && (num_values <= control_->size); cell_index = next) {
         RELEASE_ASSERT(cell_index < control_->options.capacity);
         Cell& cell = getCell(cell_index);
         absl::string_view key = cell.value.key();
         RELEASE_ASSERT(computeSlot(key) == slot);
         next = cell.next_cell;
         ++num_values;
-        // Avoid infinite loops if there is a next_cell cycle within
-        // a slot. Note that the num_values message will be emitted
-        // outside the loop.
-        if (num_values > control_->size) {
-          break;
-        }
       }
     }
     RELEASE_ASSERT(num_values == control_->size);
 
     uint32_t num_free_entries = 0;
     uint32_t expected_free_entries = control_->options.capacity - control_->size;
-    for (uint32_t cell_index = control_->free_cell_index; cell_index != Sentinal;
+
+    // Don't infinite-loop with a corruption; break when we see there's a problem.
+    for (uint32_t cell_index = control_->free_cell_index;
+         (cell_index != Sentinal) && (num_free_entries <= expected_free_entries);
          cell_index = getCell(cell_index).next_cell) {
       ++num_free_entries;
-      if (num_free_entries > expected_free_entries) {
-        // Don't infinite-loop with a corruption; break when we see there's a problem.
-        break;
-      }
     }
     RELEASE_ASSERT(num_free_entries == expected_free_entries);
   }
 
-  /**
-   * Returns a string describing the contents of the map, including the control
-   * bits and the keys in each slot.
-   */
-  std::string toString() {
-    std::string ret;
-    ret =
-        fmt::format("options={}\ncontrol={}\n", control_->options.toString(), control_->toString());
-    for (uint32_t i = 0; i < control_->options.num_slots; ++i) {
-      ret += fmt::format("slot {}:", i);
-      for (uint32_t j = slots_[i]; j != Sentinal; j = getCell(j).next_cell) {
-        ret += " " + std::string(getCell(j).value.key());
-      }
-      ret += "\n";
-    }
-    return ret;
-  }
-
   /**
    * Inserts a value into the set. If successful (e.g. map has
    * capacity) then put returns a pointer to the value object, which
@@ -241,6 +219,8 @@ template <class Value> class SharedMemoryHashSet : public Logger::Loggable<Logge
   }
 
 private:
+  friend class SharedMemoryHashSetTest;
+
   /**
    * Initializes a hash-map on raw memory. No expectations are made about the state of the memory
    * coming in.
@@ -308,11 +288,6 @@ template <class Value> class SharedMemoryHashSet : public Logger::Loggable<Logge
    * Represents control-values for the hash-table.
    */
   struct Control {
-    std::string toString() const {
-      return fmt::format("{} size={} free_cell_index={}", options.toString(), size,
-                         free_cell_index);
-    }
-
     SharedMemoryHashSetOptions options; // Options established at map construction time.
     uint64_t hash_signature;            // Hash of a constant signature string.
     uint32_t num_bytes;                 // Bytes allocated on behalf of the map.

test/common/common/shared_memory_hash_set_test.cc

@@ -50,6 +50,28 @@ class SharedMemoryHashSetTest : public testing::Test {
     memset(memory_.get(), 0, mem_size);
   }
 
+  /**
+   * Returns a string describing the contents of the map, including the control
+   * bits and the keys in each slot.
+   */
+  template <class TestValueClass>
+  std::string hashSetToString(SharedMemoryHashSet<TestValueClass>& hs) {
+    std::string ret;
+    static const uint32_t sentinal = SharedMemoryHashSet<TestValueClass>::Sentinal;
+    std::string control_string =
+        fmt::format("{} size={} free_cell_index={}", hs.control_->options.toString(),
+                    hs.control_->size, hs.control_->free_cell_index);
+    ret = fmt::format("options={}\ncontrol={}\n", hs.control_->options.toString(), control_string);
+    for (uint32_t i = 0; i < hs.control_->options.num_slots; ++i) {
+      ret += fmt::format("slot {}:", i);
+      for (uint32_t j = hs.slots_[i]; j != sentinal; j = hs.getCell(j).next_cell) {
+        ret += " " + std::string(hs.getCell(j).value.key());
+      }
+      ret += "\n";
+    }
+    return ret;
+  }
+
   SharedMemoryHashSetOptions options_;
   std::unique_ptr<uint8_t[]> memory_;
 };
@@ -102,7 +124,7 @@ TEST_F(SharedMemoryHashSetTest, putRemove) {
     SharedMemoryHashSet<TestValue> hash_set2(options_, false, memory_.get());
     EXPECT_EQ(1, hash_set2.size());
     EXPECT_EQ(nullptr, hash_set2.get("no such key"));
-    EXPECT_EQ(6789, hash_set2.get("good key")->number) << hash_set2.toString();
+    EXPECT_EQ(6789, hash_set2.get("good key")->number) << hashSetToString<TestValue>(hash_set2);
     EXPECT_FALSE(hash_set2.remove("no such key"));
     hash_set2.sanityCheck();
     EXPECT_TRUE(hash_set2.remove("good key"));