[tests] Add unit tests for StringsStorage and document the API.

Change-Id: Iccc86d0116f5d23f523e25ff02696a9fb8312223
Reviewed-on: https://chromium-review.googlesource.com/1044545
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53022}

src/profiler/strings-storage.h

@@ -9,6 +9,7 @@
 
 #include "src/base/compiler-specific.h"
 #include "src/base/hashmap.h"
+#include "src/globals.h"
 
 namespace v8 {
 namespace internal {
@@ -17,25 +18,36 @@ class Name;
 
 // Provides a storage of strings allocated in C++ heap, to hold them
 // forever, even if they disappear from JS heap or external storage.
-class StringsStorage {
+class V8_EXPORT_PRIVATE StringsStorage {
  public:
   explicit StringsStorage(uint32_t hash_seed);
   ~StringsStorage();
 
+  // Copies the given c-string and stores it, returning the stored copy, or just
+  // returns the existing string in storage if it already exists.
   const char* GetCopy(const char* src);
+  // Returns a formatted string, de-duplicated via the storage.
   PRINTF_FORMAT(2, 3) const char* GetFormatted(const char* format, ...);
-  PRINTF_FORMAT(2, 0)
-  const char* GetVFormatted(const char* format, va_list args);
+  // Returns a stored string resulting from name, or "<symbol>" for a symbol.
   const char* GetName(Name* name);
+  // Returns the string representation of the int from the store.
   const char* GetName(int index);
+  // Appends string resulting from name to prefix, then returns the stored
+  // result.
   const char* GetConsName(const char* prefix, Name* name);
+  // Does exactly the same thing as GetName(Name* name).
   const char* GetFunctionName(Name* name);
+  // Does exactly the same thing as GetCopy(const char* name).
   const char* GetFunctionName(const char* name);
 
  private:
   static bool StringsMatch(void* key1, void* key2);
+  // Adds the string to storage and returns it, or if a matching string exists
+  // in the storage, deletes str and returns the matching string instead.
   const char* AddOrDisposeString(char* str, int len);
   base::CustomMatcherHashMap::Entry* GetEntry(const char* str, int len);
+  PRINTF_FORMAT(2, 0)
+  const char* GetVFormatted(const char* format, va_list args);
 
   uint32_t hash_seed_;
   base::CustomMatcherHashMap names_;

test/unittests/BUILD.gn

@@ -184,6 +184,7 @@ v8_source_set("unittests_sources") {
     "register-configuration-unittest.cc",
     "run-all-unittests.cc",
     "source-position-table-unittest.cc",
+    "strings-storage-unittest.cc",
     "test-helpers.cc",
     "test-helpers.h",
     "test-utils.cc",

test/unittests/strings-storage-unittest.cc

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/profiler/strings-storage.h"
+
+#include <cstdio>
+
+#include "test/unittests/test-utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace v8 {
+namespace internal {
+
+typedef TestWithIsolate StringsStorageWithIsolate;
+
+bool StringEq(const char* left, const char* right) {
+  return strcmp(left, right) == 0;
+}
+
+TEST_F(StringsStorageWithIsolate, GetNameFromString) {
+  StringsStorage storage(isolate()->heap()->HashSeed());
+
+  // One char strings are canonical on the v8 heap so use a 2 char string here.
+  Handle<String> str = isolate()->factory()->NewStringFromAsciiChecked("xy");
+  const char* stored_str = storage.GetName(*str);
+  CHECK(StringEq("xy", stored_str));
+
+  // The storage should de-duplicate the underlying char arrays and return the
+  // exact same pointer for equivalent input strings.
+  const char* stored_str_twice = storage.GetName(*str);
+  CHECK_EQ(stored_str, stored_str_twice);
+
+  // Even if the input string was a different one on the v8 heap, if the char
+  // array is the same, it should be de-duplicated.
+  Handle<String> str2 = isolate()->factory()->NewStringFromAsciiChecked("xy");
+  CHECK_NE(*str, *str2);
+  const char* stored_str_thrice = storage.GetName(*str2);
+  CHECK_EQ(stored_str_twice, stored_str_thrice);
+}
+
+TEST_F(StringsStorageWithIsolate, GetNameFromSymbol) {
+  StringsStorage storage(isolate()->heap()->HashSeed());
+
+  Handle<Symbol> symbol = isolate()->factory()->NewSymbol();
+  const char* stored_symbol = storage.GetName(*symbol);
+  CHECK(StringEq("<symbol>", stored_symbol));
+
+  Handle<Symbol> symbol2 = isolate()->factory()->NewSymbol();
+  CHECK_NE(*symbol, *symbol2);
+  const char* stored_symbol2 = storage.GetName(*symbol2);
+  CHECK_EQ(stored_symbol, stored_symbol2);
+}
+
+TEST_F(StringsStorageWithIsolate, GetConsName) {
+  StringsStorage storage(isolate()->heap()->HashSeed());
+
+  Handle<String> str = isolate()->factory()->NewStringFromAsciiChecked("xy");
+
+  const char* empty_prefix_str = storage.GetConsName("", *str);
+  CHECK(StringEq("xy", empty_prefix_str));
+
+  const char* get_str = storage.GetConsName("get ", *str);
+  CHECK(StringEq("get xy", get_str));
+}
+
+TEST_F(StringsStorageWithIsolate, GetNameFromInt) {
+  StringsStorage storage(isolate()->heap()->HashSeed());
+
+  const char* stored_str = storage.GetName(0);
+  CHECK(StringEq("0", stored_str));
+
+  stored_str = storage.GetName(2147483647);
+  CHECK(StringEq("2147483647", stored_str));
+
+  stored_str = storage.GetName(std::numeric_limits<int>::min());
+  char str_negative_int[12];
+  snprintf(str_negative_int, sizeof(str_negative_int), "%d",
+           std::numeric_limits<int>::min());
+  CHECK(StringEq(str_negative_int, stored_str));
+}
+
+TEST_F(StringsStorageWithIsolate, GetFunctionNameFromString) {
+  StringsStorage storage(isolate()->heap()->HashSeed());
+
+  Handle<String> str = isolate()->factory()->NewStringFromAsciiChecked("xy");
+  const char* stored_str = storage.GetFunctionName(*str);
+  CHECK(StringEq("xy", stored_str));
+
+  // Check that GetName and GetFunctionName use the same storage.
+  const char* stored_str_twice = storage.GetName(*str);
+  CHECK_EQ(stored_str, stored_str_twice);
+}
+
+TEST_F(StringsStorageWithIsolate, GetFunctionNameFromCString) {
+  StringsStorage storage(isolate()->heap()->HashSeed());
+
+  const char* xy = "xy";
+  const char* stored_str = storage.GetFunctionName("xy");
+  CHECK(StringEq("xy", stored_str));
+  // Check that the string is copied.
+  CHECK_NE(xy, stored_str);
+}
+
+TEST_F(StringsStorageWithIsolate, Format) {
+  StringsStorage storage(isolate()->heap()->HashSeed());
+
+  const char* xy = "xy";
+  const char* stored_str = storage.GetFormatted("%s", xy);
+  CHECK(StringEq("xy", stored_str));
+  // Check that the string is copied.
+  CHECK_NE(xy, stored_str);
+
+  const char* formatted_str = storage.GetFormatted("%s / %s", xy, xy);
+  CHECK(StringEq("xy / xy", formatted_str));
+
+  // A different format specifier that results in the same string should share
+  // the string in storage.
+  const char* formatted_str2 = storage.GetFormatted("%s", "xy / xy");
+  CHECK_EQ(formatted_str, formatted_str2);
+}
+
+TEST_F(StringsStorageWithIsolate, FormatAndGetShareStorage) {
+  StringsStorage storage(isolate()->heap()->HashSeed());
+
+  Handle<String> str = isolate()->factory()->NewStringFromAsciiChecked("xy");
+  const char* stored_str = storage.GetName(*str);
+
+  const char* formatted_str = storage.GetFormatted("%s", "xy");
+  CHECK_EQ(stored_str, formatted_str);
+}
+
+}  // namespace internal
+}  // namespace v8