[runtime] Micro-optimize StringHasher

If we're hashing a single sequential string we don't need the state that the
string hasher itself tracks. This also drops first_char since we can simply
check that array_index is still 0.

Change-Id: Icb69709267426358f7c301eeb45936843ba261b0
Reviewed-on: https://chromium-review.googlesource.com/c/1340258
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57635}

src/string-hasher-inl.h

@@ -10,6 +10,7 @@
 #include "src/char-predicates-inl.h"
 #include "src/objects.h"
 #include "src/objects/string-inl.h"
+#include "src/utils-inl.h"
 
 namespace v8 {
 namespace internal {
@@ -18,8 +19,7 @@ StringHasher::StringHasher(int length, uint64_t seed)
     : length_(length),
       raw_running_hash_(static_cast<uint32_t>(seed)),
       array_index_(0),
-      is_array_index_(0 < length_ && length_ <= String::kMaxArrayIndexSize),
-      is_first_char_(true) {
+      is_array_index_(IsInRange(length, 1, String::kMaxArrayIndexSize)) {
   DCHECK(FLAG_randomize_hashes || raw_running_hash_ == 0);
 }
 
@@ -43,28 +43,18 @@ uint32_t StringHasher::GetHashCore(uint32_t running_hash) {
   return running_hash | (kZeroHash & mask);
 }
 
+template <typename Char>
 uint32_t StringHasher::ComputeRunningHash(uint32_t running_hash,
-                                          const uc16* chars, int length) {
-  DCHECK_NOT_NULL(chars);
-  DCHECK_GE(length, 0);
-  for (int i = 0; i < length; ++i) {
+                                          const Char* chars, int length) {
+  DCHECK_LE(0, length);
+  DCHECK_IMPLIES(0 < length, chars != nullptr);
+  const Char* end = &chars[length];
+  while (chars != end) {
     running_hash = AddCharacterCore(running_hash, *chars++);
   }
   return running_hash;
 }
 
-uint32_t StringHasher::ComputeRunningHashOneByte(uint32_t running_hash,
-                                                 const char* chars,
-                                                 int length) {
-  DCHECK_NOT_NULL(chars);
-  DCHECK_GE(length, 0);
-  for (int i = 0; i < length; ++i) {
-    uint16_t c = static_cast<uint16_t>(*chars++);
-    running_hash = AddCharacterCore(running_hash, c);
-  }
-  return running_hash;
-}
-
 void StringHasher::AddCharacter(uint16_t c) {
   // Use the Jenkins one-at-a-time hash function to update the hash
   // for the given character.
@@ -73,24 +63,12 @@ void StringHasher::AddCharacter(uint16_t c) {
 
 bool StringHasher::UpdateIndex(uint16_t c) {
   DCHECK(is_array_index_);
-  if (!IsDecimalDigit(c)) {
+  if (!TryAddIndexChar(&array_index_, c)) {
     is_array_index_ = false;
     return false;
   }
-  int d = c - '0';
-  if (is_first_char_) {
-    is_first_char_ = false;
-    if (d == 0 && length_ > 1) {
-      is_array_index_ = false;
-      return false;
-    }
-  }
-  if (array_index_ > 429496729U - ((d + 3) >> 3)) {
-    is_array_index_ = false;
-    return false;
-  }
-  array_index_ = array_index_ * 10 + d;
-  return true;
+  is_array_index_ = array_index_ != 0 || length_ == 1;
+  return is_array_index_;
 }
 
 template <typename Char>
@@ -106,18 +84,50 @@ inline void StringHasher::AddCharacters(const Char* chars, int length) {
       }
     }
   }
-  for (; i < length; i++) {
-    DCHECK(!is_array_index_);
-    AddCharacter(chars[i]);
-  }
+  raw_running_hash_ =
+      ComputeRunningHash(raw_running_hash_, &chars[i], length - i);
 }
 
 template <typename schar>
 uint32_t StringHasher::HashSequentialString(const schar* chars, int length,
                                             uint64_t seed) {
+#ifdef DEBUG
   StringHasher hasher(length, seed);
   if (!hasher.has_trivial_hash()) hasher.AddCharacters(chars, length);
-  return hasher.GetHashField();
+  uint32_t expected = hasher.GetHashField();
+#endif
+
+  // Check whether the string is a valid array index. In that case, compute the
+  // array index hash. It'll fall through to compute a regular string hash from
+  // the start if it turns out that the string isn't a valid array index.
+  if (IsInRange(length, 1, String::kMaxArrayIndexSize)) {
+    if (IsDecimalDigit(chars[0]) && (length == 1 || chars[0] != '0')) {
+      uint32_t index = chars[0] - '0';
+      int i = 1;
+      do {
+        if (i == length) {
+          uint32_t result = MakeArrayIndexHash(index, length);
+          DCHECK_EQ(expected, result);
+          return result;
+        }
+      } while (TryAddIndexChar(&index, chars[i++]));
+    }
+  } else if (length > String::kMaxHashCalcLength) {
+    // String hash of a large string is simply the length.
+    uint32_t result =
+        (length << String::kHashShift) | String::kIsNotArrayIndexMask;
+    DCHECK_EQ(result, expected);
+    return result;
+  }
+
+  // Non-array-index hash.
+  uint32_t hash =
+      ComputeRunningHash(static_cast<uint32_t>(seed), chars, length);
+
+  uint32_t result =
+      (GetHashCore(hash) << String::kHashShift) | String::kIsNotArrayIndexMask;
+  DCHECK_EQ(result, expected);
+  return result;
 }
 
 IteratingStringHasher::IteratingStringHasher(int len, uint64_t seed)

src/string-hasher.h

@@ -41,11 +41,9 @@ class V8_EXPORT_PRIVATE StringHasher {
   // Reusable parts of the hashing algorithm.
   V8_INLINE static uint32_t AddCharacterCore(uint32_t running_hash, uint16_t c);
   V8_INLINE static uint32_t GetHashCore(uint32_t running_hash);
+  template <typename Char>
   V8_INLINE static uint32_t ComputeRunningHash(uint32_t running_hash,
-                                               const uc16* chars, int length);
-  V8_INLINE static uint32_t ComputeRunningHashOneByte(uint32_t running_hash,
-                                                      const char* chars,
-                                                      int length);
+                                               const Char* chars, int length);
 
  protected:
   // Returns the value to store in the hash field of a string with
@@ -68,7 +66,6 @@ class V8_EXPORT_PRIVATE StringHasher {
   uint32_t raw_running_hash_;
   uint32_t array_index_;
   bool is_array_index_;
-  bool is_first_char_;
   DISALLOW_COPY_AND_ASSIGN(StringHasher);
 };
 

src/utils-inl.h

@@ -32,6 +32,15 @@ class TimedScope {
   double* result_;
 };
 
+template <typename Char>
+bool TryAddIndexChar(uint32_t* index, Char c) {
+  if (!IsDecimalDigit(c)) return false;
+  int d = c - '0';
+  if (*index > 429496729U - ((d + 3) >> 3)) return false;
+  *index = (*index) * 10 + d;
+  return true;
+}
+
 template <typename Stream>
 bool StringToArrayIndex(Stream* stream, uint32_t* index) {
   uint16_t ch = stream->GetNext();
@@ -48,12 +57,7 @@ bool StringToArrayIndex(Stream* stream, uint32_t* index) {
   int d = ch - '0';
   uint32_t result = d;
   while (stream->HasMore()) {
-    ch = stream->GetNext();
-    if (!IsDecimalDigit(ch)) return false;
-    d = ch - '0';
-    // Check that the new result is below the 32 bit limit.
-    if (result > 429496729U - ((d + 3) >> 3)) return false;
-    result = (result * 10) + d;
+    if (!TryAddIndexChar(&result, stream->GetNext())) return false;
   }
 
   *index = result;

src/utils.h

@@ -1562,6 +1562,9 @@ class StringBuilder : public SimpleStringBuilder {
 
 bool DoubleToBoolean(double d);
 
+template <typename Char>
+bool TryAddIndexChar(uint32_t* index, Char c);
+
 template <typename Stream>
 bool StringToArrayIndex(Stream* stream, uint32_t* index);