Bug 1442433 - Remove the refcount from static atoms. r=froydnj
authorNicholas Nethercote <nnethercote@mozilla.com>
Mon, 05 Mar 2018 13:54:06 +1100
changeset 461907 7e4bac75138f31d2c66a9ef2a235b4303c573b0c
parent 461906 195cbf3d34334978e5a9d101d4b79f899598159c
child 461908 786c4905fe76d92f6f45cdce39ea4eb1228791b9
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1442433
milestone60.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 1442433 - Remove the refcount from static atoms. r=froydnj The refcount is only used for dynamic atoms. On 64-bit, this reduces sizeof(nsStaticAtom) from 24 bytes to 16 bytes, and the on-heap size from 32 bytes to 16 bytes. This saves 42 KiB per process. On 32-bit, this reduces sizeof(nsStaticAtom) from 16 bytes to 12 bytes, but the on-heap size stays at 16 bytes, so memory usage is unchanged. MozReview-Commit-ID: 7d9H7MRHN9a
xpcom/ds/nsAtom.h
xpcom/ds/nsAtomTable.cpp
--- a/xpcom/ds/nsAtom.h
+++ b/xpcom/ds/nsAtom.h
@@ -83,25 +83,25 @@ public:
 
   typedef mozilla::TrueType HasThreadSafeRefCnt;
 
 private:
   friend class nsAtomTable;
   friend class nsAtomSubTable;
   friend class nsHtml5AtomEntry;
 
-  // Dynamic atom construction is done by |friend|s.
+protected:
+  // Used by nsDynamicAtom and directly (by nsHtml5AtomEntry) for HTML5 atoms.
   nsAtom(AtomKind aKind, const nsAString& aString, uint32_t aHash);
 
-protected:
+  // Used by nsStaticAtom.
   nsAtom(const char16_t* aString, uint32_t aLength, uint32_t aHash);
 
   ~nsAtom();
 
-  mozilla::ThreadSafeAutoRefCnt mRefCnt;
   uint32_t mLength: 30;
   uint32_t mKind: 2; // nsAtom::AtomKind
   uint32_t mHash;
   // WARNING! For static atoms, this is a pointer to a static char buffer. For
   // non-static atoms it points to the chars in an nsStringBuffer. This means
   // that nsStringBuffer::FromData(mString) calls are only valid for non-static
   // atoms.
   char16_t* mString;
@@ -113,17 +113,17 @@ protected:
 //
 // This class would be |final| if it wasn't for nsICSSAnonBoxPseudo and
 // nsICSSPseudoElement, which are trivial subclasses used to ensure only
 // certain atoms are passed to certain functions.
 class nsStaticAtom : public nsAtom
 {
 public:
   // These are deleted so it's impossible to RefPtr<nsStaticAtom>. Raw
-  // nsStaticAtom atoms should be used instead.
+  // nsStaticAtom pointers should be used instead.
   MozExternalRefCountType AddRef() = delete;
   MozExternalRefCountType Release() = delete;
 
   already_AddRefed<nsAtom> ToAddRefed() {
     return already_AddRefed<nsAtom>(static_cast<nsAtom*>(this));
   }
 
 private:
--- a/xpcom/ds/nsAtomTable.cpp
+++ b/xpcom/ds/nsAtomTable.cpp
@@ -59,20 +59,47 @@ enum class GCKind {
 // (and thus turned into unused state), and decremented when an unused
 // 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().
 static Atomic<int32_t, ReleaseAcquire> gUnusedAtomCount(0);
 
+// Dynamic atoms need a ref count; this class adds that to nsAtom.
+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();
+
+  static nsDynamicAtom* As(nsAtom* aAtom)
+  {
+    MOZ_ASSERT(aAtom->IsDynamicAtom());
+    return static_cast<nsDynamicAtom*>(aAtom);
+  }
+
+private:
+  friend class nsAtomTable;
+  friend class nsAtomSubTable;
+
+  // Construction is done by |friend|s.
+  nsDynamicAtom(const nsAString& aString, uint32_t aHash)
+    : nsAtom(AtomKind::DynamicAtom, aString, aHash)
+    , mRefCnt(1)
+  {}
+
+  mozilla::ThreadSafeAutoRefCnt mRefCnt;
+};
+
 // This constructor is for dynamic atoms and HTML5 atoms.
 nsAtom::nsAtom(AtomKind aKind, const nsAString& aString, uint32_t aHash)
-  : mRefCnt(1)
-  , mLength(aString.Length())
+  : mLength(aString.Length())
   , mKind(static_cast<uint32_t>(aKind))
   , mHash(aHash)
 {
   MOZ_ASSERT(aKind == AtomKind::DynamicAtom || aKind == AtomKind::HTML5Atom);
   RefPtr<nsStringBuffer> buf = nsStringBuffer::FromString(aString);
   if (buf) {
     mString = static_cast<char16_t*>(buf->Data());
   } else {
@@ -83,17 +110,17 @@ nsAtom::nsAtom(AtomKind aKind, const nsA
       // handle them more gracefully in a constructor.
       NS_ABORT_OOM(size);
     }
     mString = static_cast<char16_t*>(buf->Data());
     CopyUnicodeTo(aString, 0, mString, mLength);
     mString[mLength] = char16_t(0);
   }
 
-  MOZ_ASSERT_IF(IsDynamicAtom(), mHash == HashString(mString, mLength));
+  MOZ_ASSERT_IF(!IsHTML5Atom(), mHash == HashString(mString, mLength));
 
   MOZ_ASSERT(mString[mLength] == char16_t(0), "null terminated");
   MOZ_ASSERT(buf && buf->StorageSize() >= (mLength + 1) * sizeof(char16_t),
              "enough storage");
   MOZ_ASSERT(Equals(aString), "correct data");
 
   // Take ownership of buffer
   mozilla::Unused << buf.forget();
@@ -469,17 +496,18 @@ nsAtomSubTable::GCLocked(GCKind aKind)
   uint32_t nonZeroRefcountAtomsCount = 0;
   for (auto i = mTable.Iter(); !i.Done(); i.Next()) {
     auto entry = static_cast<AtomTableEntry*>(i.Get());
     if (entry->mAtom->IsStaticAtom()) {
       continue;
     }
 
     nsAtom* atom = entry->mAtom;
-    if (atom->mRefCnt == 0) {
+    MOZ_ASSERT(!atom->IsHTML5Atom());
+    if (atom->IsDynamicAtom() && nsDynamicAtom::As(atom)->mRefCnt == 0) {
       i.Remove();
       delete atom;
       ++removedCount;
     }
 #ifdef NS_FREE_PERMANENT_DATA
     else if (aKind == GCKind::Shutdown && PR_GetEnv("XPCOM_MEM_BLOAT_LOG")) {
       // Only report leaking atoms in leak-checking builds in a run where we
       // are checking for leaks, during shutdown. If something is anomalous,
@@ -511,42 +539,30 @@ static void
 GCAtomTable()
 {
   MOZ_ASSERT(gAtomTable);
   if (NS_IsMainThread()) {
     gAtomTable->GC(GCKind::RegularOperation);
   }
 }
 
-MozExternalRefCountType
-nsAtom::AddRef()
+MOZ_ALWAYS_INLINE MozExternalRefCountType
+nsDynamicAtom::AddRef()
 {
-  MOZ_ASSERT(!IsHTML5Atom(), "Attempt to AddRef an HTML5 atom");
-  if (!IsDynamicAtom()) {
-    MOZ_ASSERT(IsStaticAtom());
-    return 2;
-  }
-
   MOZ_ASSERT(int32_t(mRefCnt) >= 0, "illegal refcnt");
   nsrefcnt count = ++mRefCnt;
   if (count == 1) {
     gUnusedAtomCount--;
   }
   return count;
 }
 
-MozExternalRefCountType
-nsAtom::Release()
+MOZ_ALWAYS_INLINE MozExternalRefCountType
+nsDynamicAtom::Release()
 {
-  MOZ_ASSERT(!IsHTML5Atom(), "Attempt to Release an HTML5 atom");
-  if (!IsDynamicAtom()) {
-    MOZ_ASSERT(IsStaticAtom());
-    return 1;
-  }
-
   #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
 
@@ -556,16 +572,32 @@ nsAtom::Release()
     if (++gUnusedAtomCount >= kAtomGCThreshold) {
       GCAtomTable();
     }
   }
 
   return count;
 }
 
+MozExternalRefCountType
+nsAtom::AddRef()
+{
+  MOZ_ASSERT(!IsHTML5Atom(), "Attempt to AddRef an HTML5 atom");
+
+  return IsStaticAtom() ? 2 : nsDynamicAtom::As(this)->AddRef();
+}
+
+MozExternalRefCountType
+nsAtom::Release()
+{
+  MOZ_ASSERT(!IsHTML5Atom(), "Attempt to Release an HTML5 atom");
+
+  return IsStaticAtom() ? 1 : nsDynamicAtom::As(this)->Release();
+}
+
 //----------------------------------------------------------------------
 
 // Have the static atoms been inserted into the table?
 static bool gStaticAtomsDone = false;
 
 class DefaultAtoms
 {
 public:
@@ -701,18 +733,17 @@ nsAtomTable::Atomize(const nsACString& a
     return atom.forget();
   }
 
   // This results in an extra addref/release of the nsStringBuffer.
   // Unfortunately there doesn't seem to be any APIs to avoid that.
   // Actually, now there is, sort of: ForgetSharedBuffer.
   nsString str;
   CopyUTF8toUTF16(aUTF8String, str);
-  RefPtr<nsAtom> atom =
-    dont_AddRef(new nsAtom(nsAtom::AtomKind::DynamicAtom, str, hash));
+  RefPtr<nsAtom> atom = dont_AddRef(new nsDynamicAtom(str, hash));
 
   he->mAtom = atom;
 
   return atom.forget();
 }
 
 already_AddRefed<nsAtom>
 NS_Atomize(const nsACString& aUTF8String)
@@ -738,18 +769,17 @@ nsAtomTable::Atomize(const nsAString& aU
   AtomTableEntry* he = table.Add(key);
 
   if (he->mAtom) {
     RefPtr<nsAtom> atom = he->mAtom;
 
     return atom.forget();
   }
 
-  RefPtr<nsAtom> atom =
-    dont_AddRef(new nsAtom(nsAtom::AtomKind::DynamicAtom, aUTF16String, hash));
+  RefPtr<nsAtom> atom = dont_AddRef(new nsDynamicAtom(aUTF16String, hash));
   he->mAtom = atom;
 
   return atom.forget();
 }
 
 already_AddRefed<nsAtom>
 NS_Atomize(const nsAString& aUTF16String)
 {
@@ -778,18 +808,17 @@ nsAtomTable::AtomizeMainThread(const nsA
 
   nsAtomSubTable& table = SelectSubTable(key);
   MutexAutoLock lock(table.mLock);
   AtomTableEntry* he = table.Add(key);
 
   if (he->mAtom) {
     retVal = he->mAtom;
   } else {
-    RefPtr<nsAtom> newAtom = dont_AddRef(
-      new nsAtom(nsAtom::AtomKind::DynamicAtom, aUTF16String, hash));
+    RefPtr<nsAtom> newAtom = dont_AddRef(new nsDynamicAtom(aUTF16String, hash));
     he->mAtom = newAtom;
     retVal = newAtom.forget();
   }
 
   sRecentlyUsedMainThreadAtoms[index] = he->mAtom;
   return retVal.forget();
 }