Bug 1401873 - Slim down nsAtom a bit. r=froydnj. draft
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 21 Sep 2017 17:19:05 +1000
changeset 668166 a67aeed48a4fe3305afa83349c5e3ccd0f84d3f9
parent 668165 e61a718b0ecd133eec2b1245f211d626c4ab4086
child 668167 a9623a347803d81761462fd7c3dd1f2552f422e1
push id80945
push usernnethercote@mozilla.com
push dateThu, 21 Sep 2017 07:22:53 +0000
reviewersfroydnj
bugs1401873
milestone57.0a1
Bug 1401873 - Slim down nsAtom a bit. r=froydnj. GCKind, GCAtomTable(), and GCAtomTableLocked() don't really need to be in nsAtom. This patch moves them out, which requires adding a nsAtom::HasRefs() method. MozReview-Commit-ID: 1tHbJmo7hID
xpcom/ds/nsAtomTable.cpp
--- a/xpcom/ds/nsAtomTable.cpp
+++ b/xpcom/ds/nsAtomTable.cpp
@@ -120,26 +120,16 @@ public:
   }
 
   static nsAtom* CreateStatic(nsStringBuffer* aStringBuffer, uint32_t aLength,
                               uint32_t aHash)
   {
     return new nsAtom(aStringBuffer, aLength, aHash);
   }
 
-  static void GCAtomTable();
-
-  enum class GCKind {
-    RegularOperation,
-    Shutdown,
-  };
-
-  static void GCAtomTableLocked(const MutexAutoLock& aProofOfLock,
-                                GCKind aKind);
-
 private:
   // This constructor is for dynamic atoms.
   nsAtom(const nsAString& aString, uint32_t aHash)
     : mRefCnt(1)
   {
     mLength = aString.Length();
     SetKind(AtomKind::DynamicAtom);
     RefPtr<nsStringBuffer> buf = nsStringBuffer::FromString(aString);
@@ -213,16 +203,18 @@ public:
 
   NS_DECL_NSIATOM
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) final;
   typedef mozilla::TrueType HasThreadSafeRefCnt;
 
   MozExternalRefCountType DynamicAddRef();
   MozExternalRefCountType DynamicRelease();
 
+  bool HasRefs() { return mRefCnt > 0; }
+
 protected:
   ThreadSafeAutoRefCnt mRefCnt;
   NS_DECL_OWNINGTHREAD
 };
 
 NS_IMPL_QUERY_INTERFACE(nsAtom, nsIAtom);
 
 NS_IMETHODIMP
@@ -392,44 +384,40 @@ static const PLDHashTableOps AtomTableOp
 };
 
 //----------------------------------------------------------------------
 
 #define RECENTLY_USED_MAIN_THREAD_ATOM_CACHE_SIZE 31
 static nsAtom*
   sRecentlyUsedMainThreadAtoms[RECENTLY_USED_MAIN_THREAD_ATOM_CACHE_SIZE] = {};
 
-void
-nsAtom::GCAtomTable()
-{
-  if (NS_IsMainThread()) {
-    MutexAutoLock lock(*gAtomTableLock);
-    GCAtomTableLocked(lock, GCKind::RegularOperation);
-  }
-}
+enum class GCKind {
+  RegularOperation,
+  Shutdown,
+};
 
-void
-nsAtom::GCAtomTableLocked(const MutexAutoLock& aProofOfLock, GCKind aKind)
+static void
+GCAtomTableLocked(const MutexAutoLock& aProofOfLock, GCKind aKind)
 {
   MOZ_ASSERT(NS_IsMainThread());
   for (uint32_t i = 0; i < RECENTLY_USED_MAIN_THREAD_ATOM_CACHE_SIZE; ++i) {
     sRecentlyUsedMainThreadAtoms[i] = nullptr;
   }
 
   int32_t removedCount = 0; // Use a non-atomic temporary for cheaper increments.
   nsAutoCString nonZeroRefcountAtoms;
   uint32_t nonZeroRefcountAtomsCount = 0;
   for (auto i = gAtomTable->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) {
+    if (!atom->HasRefs()) {
       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
@@ -469,16 +457,25 @@ nsAtom::GCAtomTableLocked(const MutexAut
   // so we won't try to resurrect a zero refcount atom while trying to delete
   // it.
 
   MOZ_ASSERT_IF(aKind == GCKind::Shutdown, removedCount == gUnusedAtomCount);
 
   gUnusedAtomCount -= removedCount;
 }
 
+static void
+GCAtomTable()
+{
+  if (NS_IsMainThread()) {
+    MutexAutoLock lock(*gAtomTableLock);
+    GCAtomTableLocked(lock, GCKind::RegularOperation);
+  }
+}
+
 MozExternalRefCountType
 nsAtom::DynamicAddRef()
 {
   MOZ_ASSERT(IsDynamicAtom());
   nsrefcnt count = ++mRefCnt;
   if (count == 1) {
     gUnusedAtomCount--;
   }
@@ -599,17 +596,17 @@ NS_ShutdownAtomTable()
   delete gStaticAtomTable;
   gStaticAtomTable = nullptr;
 
 #ifdef NS_FREE_PERMANENT_DATA
   // Do a final GC to satisfy leak checking. We skip this step in release
   // builds.
   {
     MutexAutoLock lock(*gAtomTableLock);
-    nsAtom::GCAtomTableLocked(lock, nsAtom::GCKind::Shutdown);
+    GCAtomTableLocked(lock, GCKind::Shutdown);
   }
 #endif
 
   delete gAtomTable;
   gAtomTable = nullptr;
   delete gAtomTableLock;
   gAtomTableLock = nullptr;
 }
@@ -794,17 +791,17 @@ NS_AtomizeMainThread(const nsAString& aU
 
   sRecentlyUsedMainThreadAtoms[index] = he->mAtom;
   return retVal.forget();
 }
 
 nsrefcnt
 NS_GetNumberOfAtoms(void)
 {
-  nsAtom::GCAtomTable(); // Trigger a GC so we return a deterministic result.
+  GCAtomTable(); // Trigger a GC so we return a deterministic result.
   MutexAutoLock lock(*gAtomTableLock);
   return gAtomTable->EntryCount();
 }
 
 int32_t
 NS_GetUnusedAtomCount(void)
 {
   return gUnusedAtomCount;