From 7d52885c10233f7b3d73769927c2fefb9933f677 Mon Sep 17 00:00:00 2001
From: Jan de Mooij <jdemooij@mozilla.com>
Date: Tue, 8 Nov 2022 13:56:53 +0000
Subject: [PATCH] Bug 1799628 - Cache most recent lookups in StringToAtomCache.
 r=jonco

`js::AtomizeString` is pretty hot and caching the most recent lookups helps avoid
slower hash table lookups in 30-65% of cases (> 60% on Speedometer 2).

Differential Revision: https://phabricator.services.mozilla.com/D161571
---
 js/src/gc/Tenuring.cpp |  2 +-
 js/src/vm/Caches.h     | 63 ++++++++++++++++++++++++++++++++++--------
 2 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/js/src/gc/Tenuring.cpp b/js/src/gc/Tenuring.cpp
index 9d7f02e810e0..b9571622a921 100644
--- a/js/src/gc/Tenuring.cpp
+++ b/js/src/gc/Tenuring.cpp
@@ -711,7 +711,7 @@ JSString* js::TenuringTracer::moveToTenured(JSString* src) {
   if (src->inStringToAtomCache() && src->isDeduplicatable() &&
       !src->hasBase()) {
     JSLinearString* linear = &src->asLinear();
-    JSAtom* atom = runtime()->caches().stringToAtomCache.lookup(linear);
+    JSAtom* atom = runtime()->caches().stringToAtomCache.lookupInMap(linear);
     MOZ_ASSERT(atom, "Why was the cache purged before minor GC?");
 
     // Only deduplicate if both strings have the same encoding, to not confuse
diff --git a/js/src/vm/Caches.h b/js/src/vm/Caches.h
index 087bc2e90b08..390a86105614 100644
--- a/js/src/vm/Caches.h
+++ b/js/src/vm/Caches.h
@@ -237,24 +237,34 @@ class MegamorphicCache {
 };
 
 // Cache for AtomizeString, mapping JSLinearString* to the corresponding
-// JSAtom*. Also used by nursery GC to de-duplicate strings to atoms.
-// Purged on minor and major GC.
+// JSAtom*. The cache has two different optimizations:
+//
+// * The two most recent lookups are cached. This has a hit rate of 30-65% on
+//   typical web workloads.
+//
+// * For longer strings, there's also a JSLinearString* => JSAtom* HashMap,
+//   because hashing the string characters repeatedly can be slow.
+//   This map is also used by nursery GC to de-duplicate strings to atoms.
+//
+// This cache is purged on minor and major GC.
 class StringToAtomCache {
   using Map = HashMap<JSLinearString*, JSAtom*, PointerHasher<JSLinearString*>,
                       SystemAllocPolicy>;
   Map map_;
 
+  struct LastEntry {
+    JSLinearString* string = nullptr;
+    JSAtom* atom = nullptr;
+  };
+  static constexpr size_t NumLastEntries = 2;
+  mozilla::Array<LastEntry, NumLastEntries> lastLookups_;
+
  public:
-  // Don't use the cache for short strings. Hashing them is less expensive.
+  // Don't use the HashMap for short strings. Hashing them is less expensive.
   static constexpr size_t MinStringLength = 30;
 
-  JSAtom* lookup(JSLinearString* s) {
-    MOZ_ASSERT(!s->isAtom());
-    if (!s->inStringToAtomCache()) {
-      MOZ_ASSERT(!map_.lookup(s));
-      return nullptr;
-    }
-
+  JSAtom* lookupInMap(JSLinearString* s) const {
+    MOZ_ASSERT(s->inStringToAtomCache());
     MOZ_ASSERT(s->length() >= MinStringLength);
 
     auto p = map_.lookup(s);
@@ -263,8 +273,33 @@ class StringToAtomCache {
     return atom;
   }
 
+  MOZ_ALWAYS_INLINE JSAtom* lookup(JSLinearString* s) const {
+    MOZ_ASSERT(!s->isAtom());
+
+    for (const LastEntry& entry : lastLookups_) {
+      if (entry.string == s) {
+        MOZ_ASSERT(EqualStrings(s, entry.atom));
+        return entry.atom;
+      }
+    }
+
+    if (!s->inStringToAtomCache()) {
+      MOZ_ASSERT(!map_.lookup(s));
+      return nullptr;
+    }
+
+    return lookupInMap(s);
+  }
+
   void maybePut(JSLinearString* s, JSAtom* atom) {
     MOZ_ASSERT(!s->isAtom());
+
+    for (size_t i = NumLastEntries - 1; i > 0; i--) {
+      lastLookups_[i] = lastLookups_[i - 1];
+    }
+    lastLookups_[0].string = s;
+    lastLookups_[0].atom = atom;
+
     if (s->length() < MinStringLength) {
       return;
     }
@@ -274,7 +309,13 @@ class StringToAtomCache {
     s->setInStringToAtomCache();
   }
 
-  void purge() { map_.clearAndCompact(); }
+  void purge() {
+    map_.clearAndCompact();
+    for (LastEntry& entry : lastLookups_) {
+      entry.string = nullptr;
+      entry.atom = nullptr;
+    }
+  }
 };
 
 class RuntimeCaches {