[cpu-profiler] Add support for refcounting to StringsStorage

In order to implement CodeEntry deallocation when profiles are stopped,
we need to be able to effectively deallocate strings. Introduce a simple
imperative refcounting API using the existing HashMap slots for
StringsStorage to enable this.

Design doc: https://docs.google.com/document/d/1OTwlBnAMXZEaOICtuz16c01QnkPPdqHBoHpfGwnk5SY/edit

Bug: chromium:956688
Change-Id: Iaa1142925f40aa66c064d011b2a0630de72037fe
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2121575Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Andrew Comminos <acomminos@fb.com>
Cr-Commit-Position: refs/heads/master@{#66911}

src/profiler/strings-storage.cc

@@ -22,7 +22,7 @@ StringsStorage::StringsStorage() : names_(StringsMatch) {}
 StringsStorage::~StringsStorage() {
   for (base::HashMap::Entry* p = names_.Start(); p != nullptr;
        p = names_.Next(p)) {
-    DeleteArray(reinterpret_cast<const char*>(p->value));
+    DeleteArray(reinterpret_cast<const char*>(p->key));
   }
 }
 
@@ -34,9 +34,10 @@ const char* StringsStorage::GetCopy(const char* src) {
     StrNCpy(dst, src, len);
     dst[len] = '\0';
     entry->key = dst.begin();
-    entry->value = entry->key;
   }
-  return reinterpret_cast<const char*>(entry->value);
+  entry->value =
+      reinterpret_cast<void*>(reinterpret_cast<size_t>(entry->value) + 1);
+  return reinterpret_cast<const char*>(entry->key);
 }
 
 const char* StringsStorage::GetFormatted(const char* format, ...) {
@@ -52,11 +53,12 @@ const char* StringsStorage::AddOrDisposeString(char* str, int len) {
   if (entry->value == nullptr) {
     // New entry added.
     entry->key = str;
-    entry->value = str;
   } else {
     DeleteArray(str);
   }
-  return reinterpret_cast<const char*>(entry->value);
+  entry->value =
+      reinterpret_cast<void*>(reinterpret_cast<size_t>(entry->value) + 1);
+  return reinterpret_cast<const char*>(entry->key);
 }
 
 const char* StringsStorage::GetVFormatted(const char* format, va_list args) {
@@ -106,6 +108,30 @@ const char* StringsStorage::GetConsName(const char* prefix, Name name) {
   return "";
 }
 
+bool StringsStorage::Release(const char* str) {
+  int len = static_cast<int>(strlen(str));
+  uint32_t hash = StringHasher::HashSequentialString(str, len, kZeroHashSeed);
+  base::HashMap::Entry* entry = names_.Lookup(const_cast<char*>(str), hash);
+  DCHECK(entry);
+  if (!entry) {
+    return false;
+  }
+
+  DCHECK(entry->value);
+  entry->value =
+      reinterpret_cast<void*>(reinterpret_cast<size_t>(entry->value) - 1);
+
+  if (entry->value == 0) {
+    names_.Remove(const_cast<char*>(str), hash);
+    DeleteArray(str);
+  }
+  return true;
+}
+
+size_t StringsStorage::GetStringCountForTesting() const {
+  return names_.occupancy();
+}
+
 base::HashMap::Entry* StringsStorage::GetEntry(const char* str, int len) {
   uint32_t hash = StringHasher::HashSequentialString(str, len, kZeroHashSeed);
   return names_.LookupOrInsert(const_cast<char*>(str), hash);

src/profiler/strings-storage.h

@@ -35,6 +35,13 @@ class V8_EXPORT_PRIVATE StringsStorage {
   // Appends string resulting from name to prefix, then returns the stored
   // result.
   const char* GetConsName(const char* prefix, Name name);
+  // Reduces the refcount of the given string, freeing it if no other
+  // references are made to it.
+  // Returns true if the string was successfully unref'd.
+  bool Release(const char* str);
+
+  // Returns the number of strings in the store.
+  size_t GetStringCountForTesting() const;
 
  private:
   static bool StringsMatch(void* key1, void* key2);

test/unittests/profiler/strings-storage-unittest.cc

@@ -108,5 +108,54 @@ TEST_F(StringsStorageWithIsolate, FormatAndGetShareStorage) {
   CHECK_EQ(stored_str, formatted_str);
 }
 
+TEST_F(StringsStorageWithIsolate, Refcounting) {
+  StringsStorage storage;
+
+  const char* a = storage.GetCopy("12");
+  CHECK_EQ(storage.GetStringCountForTesting(), 1);
+
+  const char* b = storage.GetCopy("12");
+  CHECK_EQ(storage.GetStringCountForTesting(), 1);
+
+  // Ensure that we deduplicate the string.
+  CHECK_EQ(a, b);
+
+  CHECK(storage.Release(a));
+  CHECK_EQ(storage.GetStringCountForTesting(), 1);
+  CHECK(storage.Release(b));
+  CHECK_EQ(storage.GetStringCountForTesting(), 0);
+#if !DEBUG
+  CHECK(!storage.Release("12"));
+#endif  // !DEBUG
+
+  // Verify that other constructors refcount as intended.
+  const char* c = storage.GetFormatted("%d", 12);
+  CHECK_EQ(storage.GetStringCountForTesting(), 1);
+
+  const char* d = storage.GetName(12);
+  CHECK_EQ(storage.GetStringCountForTesting(), 1);
+
+  CHECK_EQ(c, d);
+
+  CHECK(storage.Release(c));
+  CHECK_EQ(storage.GetStringCountForTesting(), 1);
+  CHECK(storage.Release(d));
+  CHECK_EQ(storage.GetStringCountForTesting(), 0);
+#if !DEBUG
+  CHECK(!storage.Release("12"));
+#endif  // !DEBUG
+}
+
+TEST_F(StringsStorageWithIsolate, InvalidRelease) {
+  StringsStorage storage;
+
+  // If a refcount becomes invalid, throw in debug builds.
+#ifdef DEBUG
+  ASSERT_DEATH_IF_SUPPORTED(storage.Release("12"), "check failed");
+#else
+  CHECK(!storage.Release("12"));
+#endif  // DEBUG
+}
+
 }  // namespace internal
 }  // namespace v8