Bug 1666274 - [Beta82] Always use inline ParserAtomEntries. r=djvj, a=jcristau
authorTed Campbell <tcampbell@mozilla.com>
Mon, 21 Sep 2020 19:41:42 +0000
changeset 614819 b4d8caa99362c45011c138d348734f3d7f4e900c
parent 614818 cf51513a73e9a7bf6705e95a3922e4b17d81dafb
child 614820 d3fe34f9b009b6264f5aec394adc0520afe03926
push id14029
push userjcristau@mozilla.com
push dateSat, 26 Sep 2020 07:33:07 +0000
treeherdermozilla-beta@7dd5c43f1b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdjvj, jcristau
bugs1666274
milestone82.0
Bug 1666274 - [Beta82] Always use inline ParserAtomEntries. r=djvj, a=jcristau In practice we always allocated clones of the string contents for non-inline ParserAtoms which was simply an extra allocation. Any hash tables over ParserAtoms are already using pointers so they are unaffected. Depends on D91424 Differential Revision: https://phabricator.services.mozilla.com/D91425
js/src/frontend/ParserAtom.cpp
js/src/frontend/ParserAtom.h
--- a/js/src/frontend/ParserAtom.cpp
+++ b/js/src/frontend/ParserAtom.cpp
@@ -100,38 +100,20 @@ static JSAtom* GetWellKnownAtom(JSContex
   return (&cx->names().abort)[int32_t(kind)];
 }
 
 mozilla::GenericErrorResult<OOM&> RaiseParserAtomsOOMError(JSContext* cx) {
   js::ReportOutOfMemory(cx);
   return mozilla::Err(PARSER_ATOMS_OOM);
 }
 
-template <typename CharT>
-/* static */ JS::Result<UniquePtr<ParserAtomEntry>, OOM&>
-ParserAtomEntry::allocate(JSContext* cx,
-                          mozilla::UniquePtr<CharT[], JS::FreePolicy>&& ptr,
-                          uint32_t length, HashNumber hash) {
-  MOZ_ASSERT(length > MaxInline<CharT>());
-
-  ParserAtomEntry* entryPtr = cx->pod_malloc<ParserAtomEntry>();
-  if (!entryPtr) {
-    return RaiseParserAtomsOOMError(cx);
-  }
-  return UniquePtr<ParserAtomEntry>(
-      new (entryPtr) ParserAtomEntry(std::move(ptr), length, hash));
-}
-
 template <typename CharT, typename SeqCharT>
 /* static */ JS::Result<UniquePtr<ParserAtomEntry>, OOM&>
-ParserAtomEntry::allocateInline(JSContext* cx,
-                                InflatedChar16Sequence<SeqCharT> seq,
-                                uint32_t length, HashNumber hash) {
-  MOZ_ASSERT(length <= MaxInline<CharT>());
-
+ParserAtomEntry::allocate(JSContext* cx, InflatedChar16Sequence<SeqCharT> seq,
+                          uint32_t length, HashNumber hash) {
   ParserAtomEntry* uninitEntry =
       cx->pod_malloc_with_extra<ParserAtomEntry, CharT>(length);
   if (!uninitEntry) {
     return RaiseParserAtomsOOMError(cx);
   }
 
   CharT* entryBuf = ParserAtomEntry::inlineBufferPtr<CharT>(
       reinterpret_cast<ParserAtomEntry*>(uninitEntry));
@@ -267,58 +249,32 @@ JS::Result<const ParserAtom*, OOM&> Pars
 }
 
 JS::Result<const ParserAtom*, OOM&> ParserAtomsTable::internLatin1Seq(
     JSContext* cx, EntrySet::AddPtr& addPtr, HashNumber hash,
     const Latin1Char* latin1Ptr, uint32_t length) {
   MOZ_ASSERT(!addPtr);
 
   InflatedChar16Sequence<Latin1Char> seq(latin1Ptr, length);
+
   UniquePtr<ParserAtomEntry> entry;
-
-  // Allocate an entry for inline strings.
-  if (length <= ParserAtomEntry::MaxInline<Latin1Char>()) {
-    MOZ_TRY_VAR(entry, ParserAtomEntry::allocateInline<Latin1Char>(
-                           cx, seq, length, hash));
-    return addEntry(cx, addPtr, std::move(entry));
-  }
-
-  UniqueLatin1Chars copy = js::DuplicateString(cx, latin1Ptr, length);
-  if (!copy) {
-    return RaiseParserAtomsOOMError(cx);
-  }
   MOZ_TRY_VAR(entry,
-              ParserAtomEntry::allocate(cx, std::move(copy), length, hash));
+              ParserAtomEntry::allocate<Latin1Char>(cx, seq, length, hash));
   return addEntry(cx, addPtr, std::move(entry));
 }
 
 template <typename AtomCharT, typename SeqCharT>
 JS::Result<const ParserAtom*, OOM&> ParserAtomsTable::internChar16Seq(
     JSContext* cx, EntrySet::AddPtr& addPtr, HashNumber hash,
     InflatedChar16Sequence<SeqCharT> seq, uint32_t length) {
   MOZ_ASSERT(!addPtr);
 
   UniquePtr<ParserAtomEntry> entry;
-
-  // Allocate a fat entry for inline strings.
-  if (length <= ParserAtomEntry::MaxInline<AtomCharT>()) {
-    MOZ_TRY_VAR(entry, ParserAtomEntry::allocateInline<AtomCharT>(
-                           cx, seq, length, hash));
-    return addEntry(cx, addPtr, std::move(entry));
-  }
-
-  // Or copy to out-of-line contents.
-  using UniqueCharsT = mozilla::UniquePtr<AtomCharT[], JS::FreePolicy>;
-  UniqueCharsT copy(cx->pod_malloc<AtomCharT>(length));
-  if (!copy) {
-    return RaiseParserAtomsOOMError(cx);
-  }
-  ParserAtomEntry::drainChar16Seq<AtomCharT, SeqCharT>(copy.get(), seq, length);
   MOZ_TRY_VAR(entry,
-              ParserAtomEntry::allocate(cx, std::move(copy), length, hash));
+              ParserAtomEntry::allocate<AtomCharT>(cx, seq, length, hash));
   return addEntry(cx, addPtr, std::move(entry));
 }
 
 static const uint16_t MAX_LATIN1_CHAR = 0xff;
 
 JS::Result<const ParserAtom*, OOM&> ParserAtomsTable::internAscii(
     JSContext* cx, const char* asciiPtr, uint32_t length) {
   // ASCII strings are strict subsets of Latin1 strings.
@@ -553,40 +509,21 @@ bool WellKnownParserAtoms::initSingle(JS
     return true;
   }
 
   InflatedChar16Sequence<Latin1Char> seq(
       reinterpret_cast<const Latin1Char*>(str), len);
   SpecificParserAtomLookup<Latin1Char> lookup(seq);
   HashNumber hash = lookup.hash();
 
-  UniquePtr<ParserAtomEntry> entry = nullptr;
-
-  // Check for inline allocation.
-  if (len <= ParserAtomEntry::MaxInline<Latin1Char>()) {
-    auto maybeEntry =
-        ParserAtomEntry::allocateInline<Latin1Char>(cx, seq, len, hash);
-    if (maybeEntry.isErr()) {
-      return false;
-    }
-    entry = maybeEntry.unwrap();
-
-    // Do heap-allocation of contents.
-  } else {
-    UniqueLatin1Chars copy(cx->pod_malloc<Latin1Char>(len));
-    if (!copy) {
-      return false;
-    }
-    mozilla::PodCopy(copy.get(), reinterpret_cast<const Latin1Char*>(str), len);
-    auto maybeEntry = ParserAtomEntry::allocate(cx, std::move(copy), len, hash);
-    if (maybeEntry.isErr()) {
-      return false;
-    }
-    entry = maybeEntry.unwrap();
+  auto maybeEntry = ParserAtomEntry::allocate<Latin1Char>(cx, seq, len, hash);
+  if (maybeEntry.isErr()) {
+    return false;
   }
+  UniquePtr<ParserAtomEntry> entry = maybeEntry.unwrap();
   entry->setWellKnownAtomId(kind);
 
   // Save name for returning after moving entry into set.
   const ParserName* nm = entry.get()->asName();
   if (!wellKnownSet_.putNew(lookup, std::move(entry))) {
     js::ReportOutOfMemory(cx);
     return false;
   }
@@ -606,18 +543,17 @@ bool WellKnownParserAtoms::initStaticStr
 
     constexpr size_t len = 1;
     MOZ_ASSERT(atom->length() == len);
 
     InflatedChar16Sequence<Latin1Char> seq(atom->latin1Chars(nogc), len);
     SpecificParserAtomLookup<Latin1Char> lookup(seq);
     HashNumber hash = lookup.hash();
 
-    auto maybeEntry =
-        ParserAtomEntry::allocateInline<Latin1Char>(cx, seq, len, hash);
+    auto maybeEntry = ParserAtomEntry::allocate<Latin1Char>(cx, seq, len, hash);
     if (maybeEntry.isErr()) {
       return false;
     }
 
     length1StaticTable_[i] = maybeEntry.unwrap();
     length1StaticTable_[i]->setStaticParserString1(StaticParserString1(i));
   }
 
@@ -629,18 +565,17 @@ bool WellKnownParserAtoms::initStaticStr
 
     constexpr size_t len = 2;
     MOZ_ASSERT(atom->length() == len);
 
     InflatedChar16Sequence<Latin1Char> seq(atom->latin1Chars(nogc), len);
     SpecificParserAtomLookup<Latin1Char> lookup(seq);
     HashNumber hash = lookup.hash();
 
-    auto maybeEntry =
-        ParserAtomEntry::allocateInline<Latin1Char>(cx, seq, len, hash);
+    auto maybeEntry = ParserAtomEntry::allocate<Latin1Char>(cx, seq, len, hash);
     if (maybeEntry.isErr()) {
       return false;
     }
 
     length2StaticTable_[i] = maybeEntry.unwrap();
     length2StaticTable_[i]->setStaticParserString2(StaticParserString2(i));
   }
 
--- a/js/src/frontend/ParserAtom.h
+++ b/js/src/frontend/ParserAtom.h
@@ -56,33 +56,24 @@ enum class WellKnownAtomId : uint32_t {
 // These types correspond into indices in the StaticStrings arrays.
 enum class StaticParserString1 : uint8_t;
 enum class StaticParserString2 : uint16_t;
 
 /**
  * A ParserAtomEntry is an in-parser representation of an interned atomic
  * string.  It mostly mirrors the information carried by a JSAtom*.
  *
- * The atom contents are stored in one of four locations:
+ * The atom contents are stored in one of two locations:
  *  1. Inline Latin1Char storage (immediately after the ParserAtomEntry memory).
  *  2. Inline char16_t storage (immediately after the ParserAtomEntry memory).
- *  3. An owned pointer to a heap-allocated Latin1Char buffer.
- *  4. An owned pointer to a heap-allocated char16_t buffer.
  */
 class alignas(alignof(void*)) ParserAtomEntry {
   friend class ParserAtomsTable;
   friend class WellKnownParserAtoms;
 
-  template <typename CharT>
-  static constexpr uint32_t MaxInline() {
-    return std::is_same_v<CharT, Latin1Char>
-               ? JSFatInlineString::MAX_LENGTH_LATIN1
-               : JSFatInlineString::MAX_LENGTH_TWO_BYTE;
-  }
-
   /**
    * This single-word variant struct multiplexes between four representations.
    *    1. An owned pointer to a heap-allocated Latin1Char buffer.
    *    2. An owned pointer to a heap-allocated char16_t buffer.
    *    3. A weak Latin1Char pointer to the inline buffer in an entry.
    *    4. A weak char16_t pointer to the inline buffer in an entry.
    *
    * The lowest bit of the tagged pointer is used to distinguish between
@@ -221,53 +212,48 @@ class alignas(alignof(void*)) ParserAtom
   //
   // Otherwise, this should hold AtomIndex into CompilationInfo.atoms,
   // or empty if the JSAtom isn't yet allocated.
   using AtomIndexType =
       mozilla::Variant<mozilla::Nothing, AtomIndex, WellKnownAtomId,
                        StaticParserString1, StaticParserString2>;
   mutable AtomIndexType atomIndex_ = AtomIndexType(mozilla::Nothing());
 
- public:
+  // End of fields.
+
   static const uint32_t MAX_LENGTH = JSString::MAX_LENGTH;
 
   template <typename CharT>
   ParserAtomEntry(mozilla::UniquePtr<CharT[], JS::FreePolicy> chars,
                   uint32_t length, HashNumber hash)
       : variant_(std::move(chars)), length_(length), hash_(hash) {}
 
   template <typename CharT>
   ParserAtomEntry(const CharT* chars, uint32_t length, HashNumber hash)
       : variant_(chars, /* isInline = */ true), length_(length), hash_(hash) {}
 
+ public:
   template <typename CharT>
   static CharT* inlineBufferPtr(ParserAtomEntry* entry) {
     return reinterpret_cast<CharT*>(entry + 1);
   }
   template <typename CharT>
   static const CharT* inlineBufferPtr(const ParserAtomEntry* entry) {
     return reinterpret_cast<const CharT*>(entry + 1);
   }
 
-  template <typename CharT>
-  static JS::Result<UniquePtr<ParserAtomEntry>, OOM&> allocate(
-      JSContext* cx, mozilla::UniquePtr<CharT[], JS::FreePolicy>&& ptr,
-      uint32_t length, HashNumber hash);
-
-  template <typename CharT, typename SeqCharT>
-  static JS::Result<UniquePtr<ParserAtomEntry>, OOM&> allocateInline(
-      JSContext* cx, InflatedChar16Sequence<SeqCharT> seq, uint32_t length,
-      HashNumber hash);
-
- public:
   // ParserAtomEntries may own their content buffers in variant_, and thus
   // cannot be copy-constructed - as a new chars would need to be allocated.
   ParserAtomEntry(const ParserAtomEntry&) = delete;
+  ParserAtomEntry(ParserAtomEntry&& other) = delete;
 
-  ParserAtomEntry(ParserAtomEntry&& other) = delete;
+  template <typename CharT, typename SeqCharT>
+  static JS::Result<UniquePtr<ParserAtomEntry>, OOM&> allocate(
+      JSContext* cx, InflatedChar16Sequence<SeqCharT> seq, uint32_t length,
+      HashNumber hash);
 
   ParserAtom* asAtom() { return reinterpret_cast<ParserAtom*>(this); }
   const ParserAtom* asAtom() const {
     return reinterpret_cast<const ParserAtom*>(this);
   }
 
   inline ParserName* asName();
   inline const ParserName* asName() const;