Bug 1509720 - Inline atom refcounting. r=njn
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 28 Nov 2018 15:03:40 +0000
changeset 504962 9d680582489f0139227b36dfd908c6f8ac154d4b
parent 504961 66e1668bb44c322c83e89bfebae59e0a4d2a2f07
child 504963 164213f935e1dfc919dd0f1fbb78cc7cdea98679
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs1509720
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1509720 - Inline atom refcounting. r=njn We're paying two function calls from Gecko_AddRefAtom / Gecko_ReleaseAtom for no good reason, plus it's simple enough it's probably worth to inline it anyway for C++ callers. Differential Revision: https://phabricator.services.mozilla.com/D12860
xpcom/ds/nsAtom.h
xpcom/ds/nsAtomTable.cpp
--- a/xpcom/ds/nsAtom.h
+++ b/xpcom/ds/nsAtom.h
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef nsAtom_h
 #define nsAtom_h
 
 #include "nsISupportsImpl.h"
 #include "nsString.h"
+#include "mozilla/Atomics.h"
 #include "mozilla/UniquePtr.h"
 
 namespace mozilla {
 struct AtomsSizes;
 }
 
 class nsStaticAtom;
 class nsDynamicAtom;
@@ -45,19 +46,19 @@ public:
   bool Equals(const nsAString& aString) const
   {
     return Equals(aString.BeginReading(), aString.Length());
   }
 
   bool IsStatic() const { return mIsStatic; }
   bool IsDynamic() const { return !IsStatic(); }
 
-  const nsStaticAtom* AsStatic() const;
-  const nsDynamicAtom* AsDynamic() const;
-  nsDynamicAtom* AsDynamic();
+  inline const nsStaticAtom* AsStatic() const;
+  inline const nsDynamicAtom* AsDynamic() const;
+  inline nsDynamicAtom* AsDynamic();
 
   char16ptr_t GetUTF16String() const;
 
   uint32_t GetLength() const { return mLength; }
 
   operator mozilla::Span<const char16_t>() const
   {
     return mozilla::MakeSpan(static_cast<const char16_t*>(GetUTF16String()), GetLength());
@@ -77,18 +78,18 @@ public:
   // unchanged.
   bool IsAsciiLowercase() const
   {
     return mIsAsciiLowercase;
   }
 
   // We can't use NS_INLINE_DECL_THREADSAFE_REFCOUNTING because the refcounting
   // of this type is special.
-  MozExternalRefCountType AddRef();
-  MozExternalRefCountType Release();
+  inline MozExternalRefCountType AddRef();
+  inline MozExternalRefCountType Release();
 
   typedef mozilla::TrueType HasThreadSafeRefCnt;
 
 protected:
   // Used by nsStaticAtom.
   constexpr nsAtom(uint32_t aLength, uint32_t aHash, bool aIsAsciiLowercase)
     : mLength(aLength)
     , mIsStatic(true)
@@ -152,46 +153,113 @@ private:
   uint32_t mStringOffset;
 };
 
 class nsDynamicAtom : public nsAtom
 {
 public:
   // We can't use NS_INLINE_DECL_THREADSAFE_REFCOUNTING because the refcounting
   // of this type is special.
-  MozExternalRefCountType AddRef();
-  MozExternalRefCountType Release();
+  MozExternalRefCountType AddRef()
+  {
+    MOZ_ASSERT(int32_t(mRefCnt) >= 0, "illegal refcnt");
+    nsrefcnt count = ++mRefCnt;
+    if (count == 1) {
+      gUnusedAtomCount--;
+    }
+    return count;
+  }
+
+  MozExternalRefCountType Release()
+  {
+    #ifdef DEBUG
+    // We set a lower GC threshold for atoms in debug builds so that we exercise
+    // the GC machinery more often.
+    static const int32_t kAtomGCThreshold = 20;
+    #else
+    static const int32_t kAtomGCThreshold = 10000;
+    #endif
+
+    MOZ_ASSERT(int32_t(mRefCnt) > 0, "dup release");
+    nsrefcnt count = --mRefCnt;
+    if (count == 0) {
+      if (++gUnusedAtomCount >= kAtomGCThreshold) {
+        GCAtomTable();
+      }
+    }
+
+    return count;
+  }
 
   const char16_t* String() const
   {
     return reinterpret_cast<const char16_t*>(this + 1);
   }
 
   static nsDynamicAtom* FromChars(char16_t* chars)
   {
     return reinterpret_cast<nsDynamicAtom*>(chars) - 1;
   }
 
 private:
   friend class nsAtomTable;
   friend class nsAtomSubTable;
+  friend int32_t NS_GetUnusedAtomCount();
+
+  static mozilla::Atomic<int32_t,
+                         mozilla::ReleaseAcquire,
+                         mozilla::recordreplay::Behavior::DontPreserve> gUnusedAtomCount;
+  static void GCAtomTable();
 
   // These shouldn't be used directly, even by friend classes. The
   // Create()/Destroy() methods use them.
   nsDynamicAtom(const nsAString& aString, uint32_t aHash, bool aIsAsciiLowercase);
   ~nsDynamicAtom() {}
 
   static nsDynamicAtom* Create(const nsAString& aString, uint32_t aHash);
   static void Destroy(nsDynamicAtom* aAtom);
 
   mozilla::ThreadSafeAutoRefCnt mRefCnt;
 
   // The atom's chars are stored at the end of the struct.
 };
 
+const nsStaticAtom*
+nsAtom::AsStatic() const
+{
+  MOZ_ASSERT(IsStatic());
+  return static_cast<const nsStaticAtom*>(this);
+}
+
+const nsDynamicAtom*
+nsAtom::AsDynamic() const
+{
+  MOZ_ASSERT(IsDynamic());
+  return static_cast<const nsDynamicAtom*>(this);
+}
+
+nsDynamicAtom*
+nsAtom::AsDynamic()
+{
+  MOZ_ASSERT(IsDynamic());
+  return static_cast<nsDynamicAtom*>(this);
+}
+
+MozExternalRefCountType
+nsAtom::AddRef()
+{
+  return IsStatic() ? 2 : AsDynamic()->AddRef();
+}
+
+MozExternalRefCountType
+nsAtom::Release()
+{
+  return IsStatic() ? 1 : AsDynamic()->Release();
+}
+
 // The four forms of NS_Atomize (for use with |RefPtr<nsAtom>|) return the
 // atom for the string given. At any given time there will always be one atom
 // representing a given string. Atoms are intended to make string comparison
 // cheaper by simplifying it to pointer equality. A pointer to the atom that
 // does not own a reference is not guaranteed to be valid.
 
 // Find an atom that matches the given UTF-8 string. The string is assumed to
 // be zero terminated. Never returns null.
--- a/xpcom/ds/nsAtomTable.cpp
+++ b/xpcom/ds/nsAtomTable.cpp
@@ -59,17 +59,18 @@ enum class GCKind {
 // atom gets a reference again. The atom table relies on this value to
 // schedule GC. This value can temporarily go below zero when multiple
 // threads are operating the same atom, so it has to be signed so that
 // we wouldn't use overflow value for comparison.
 // See nsAtom::AddRef() and nsAtom::Release().
 // This atomic can be accessed during the GC and other places where recorded
 // events are not allowed, so its value is not preserved when recording or
 // replaying.
-static Atomic<int32_t, ReleaseAcquire, recordreplay::Behavior::DontPreserve> gUnusedAtomCount(0);
+Atomic<int32_t, ReleaseAcquire, recordreplay::Behavior::DontPreserve>
+  nsDynamicAtom::gUnusedAtomCount;
 
 nsDynamicAtom::nsDynamicAtom(const nsAString& aString, uint32_t aHash, bool aIsAsciiLowercase)
   : nsAtom(aString, aHash, aIsAsciiLowercase)
   , mRefCnt(1)
 {
 }
 
 // Returns true if ToLowercaseASCII would return the string unchanged.
@@ -109,37 +110,16 @@ nsDynamicAtom::Create(const nsAString& a
 
 void
 nsDynamicAtom::Destroy(nsDynamicAtom* aAtom)
 {
   aAtom->~nsDynamicAtom();
   free(aAtom);
 }
 
-const nsStaticAtom*
-nsAtom::AsStatic() const
-{
-  MOZ_ASSERT(IsStatic());
-  return static_cast<const nsStaticAtom*>(this);
-}
-
-const nsDynamicAtom*
-nsAtom::AsDynamic() const
-{
-  MOZ_ASSERT(IsDynamic());
-  return static_cast<const nsDynamicAtom*>(this);
-}
-
-nsDynamicAtom*
-nsAtom::AsDynamic()
-{
-  MOZ_ASSERT(IsDynamic());
-  return static_cast<nsDynamicAtom*>(this);
-}
-
 void
 nsAtom::ToString(nsAString& aString) const
 {
   // See the comment on |mString|'s declaration.
   if (IsStatic()) {
     // AssignLiteral() lets us assign without copying. This isn't a string
     // literal, but it's a static atom and thus has an unbounded lifetime,
     // which is what's important.
@@ -445,17 +425,17 @@ void nsAtomTable::GC(GCKind aKind)
   // a regular GC. But we can (and do) assert this just after the last GC at
   // shutdown.
   //
   // Note that, barring refcounting bugs, an atom can only go from a zero
   // refcount to a non-zero refcount while the atom table lock is held, so
   // so we won't try to resurrect a zero refcount atom while trying to delete
   // it.
 
-  MOZ_ASSERT_IF(aKind == GCKind::Shutdown, gUnusedAtomCount == 0);
+  MOZ_ASSERT_IF(aKind == GCKind::Shutdown, nsDynamicAtom::gUnusedAtomCount == 0);
 }
 
 size_t
 nsAtomTable::RacySlowCount()
 {
   // Trigger a GC so that the result is deterministic modulo other threads.
   GC(GCKind::RegularOperation);
   size_t count = 0;
@@ -514,73 +494,28 @@ nsAtomSubTable::GCLocked(GCKind aKind)
 
   }
   if (nonZeroRefcountAtomsCount) {
     nsPrintfCString msg("%d dynamic atom(s) with non-zero refcount: %s",
                         nonZeroRefcountAtomsCount, nonZeroRefcountAtoms.get());
     NS_ASSERTION(nonZeroRefcountAtomsCount == 0, msg.get());
   }
 
-  gUnusedAtomCount -= removedCount;
+  nsDynamicAtom::gUnusedAtomCount -= removedCount;
 }
 
-static void
-GCAtomTable()
+void
+nsDynamicAtom::GCAtomTable()
 {
   MOZ_ASSERT(gAtomTable);
   if (NS_IsMainThread()) {
     gAtomTable->GC(GCKind::RegularOperation);
   }
 }
 
-MOZ_ALWAYS_INLINE MozExternalRefCountType
-nsDynamicAtom::AddRef()
-{
-  MOZ_ASSERT(int32_t(mRefCnt) >= 0, "illegal refcnt");
-  nsrefcnt count = ++mRefCnt;
-  if (count == 1) {
-    gUnusedAtomCount--;
-  }
-  return count;
-}
-
-MOZ_ALWAYS_INLINE MozExternalRefCountType
-nsDynamicAtom::Release()
-{
-  #ifdef DEBUG
-  // We set a lower GC threshold for atoms in debug builds so that we exercise
-  // the GC machinery more often.
-  static const int32_t kAtomGCThreshold = 20;
-  #else
-  static const int32_t kAtomGCThreshold = 10000;
-  #endif
-
-  MOZ_ASSERT(int32_t(mRefCnt) > 0, "dup release");
-  nsrefcnt count = --mRefCnt;
-  if (count == 0) {
-    if (++gUnusedAtomCount >= kAtomGCThreshold) {
-      GCAtomTable();
-    }
-  }
-
-  return count;
-}
-
-MozExternalRefCountType
-nsAtom::AddRef()
-{
-  return IsStatic() ? 2 : AsDynamic()->AddRef();
-}
-
-MozExternalRefCountType
-nsAtom::Release()
-{
-  return IsStatic() ? 1 : AsDynamic()->Release();
-}
-
 //----------------------------------------------------------------------
 
 // Have the static atoms been inserted into the table?
 static bool gStaticAtomsDone = false;
 
 void
 NS_InitAtomTable()
 {
@@ -787,17 +722,17 @@ NS_GetNumberOfAtoms(void)
 {
   MOZ_ASSERT(gAtomTable);
   return gAtomTable->RacySlowCount();
 }
 
 int32_t
 NS_GetUnusedAtomCount(void)
 {
-  return gUnusedAtomCount;
+  return nsDynamicAtom::gUnusedAtomCount;
 }
 
 nsStaticAtom*
 NS_GetStaticAtom(const nsAString& aUTF16String)
 {
   MOZ_ASSERT(gStaticAtomsDone, "Static atom setup not yet done.");
   MOZ_ASSERT(gAtomTable);
   return gAtomTable->GetStaticAtom(aUTF16String);