Bug 1261735 (part 1) - Overhaul the atom implementation. r=froydnj,erahm.
authorNicholas Nethercote <nnethercote@mozilla.com>
Fri, 01 Apr 2016 11:18:06 +1100
changeset 291874 06d9d9cd6ab1187c284a46aa6830aa963d86456d
parent 291873 39c895b67af2e6fe91e70a1ea7bf8581401af274
child 291875 38204fd2ece962c9504ae59d58c99bf38b5626bd
push id74702
push usernnethercote@mozilla.com
push dateWed, 06 Apr 2016 10:30:00 +0000
treeherdermozilla-inbound@06d9d9cd6ab1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj, erahm
bugs1261735
milestone48.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 1261735 (part 1) - Overhaul the atom implementation. r=froydnj,erahm. This patch changes things so that dynamic atoms and static atoms have distinct implementations. This is a step towards allowing dynamic atoms and static atoms to have different layouts in memory, which will allow static atoms to be represented more compactly. Specifically, the patch does the following. - It renames AtomImpl as DynamicAtom and PermanentAtomImpl as StaticAtom, and the latter is no longer a subclass of the former. This required duplicating some methods from the former into the latter: ScriptableToString(), ToUTF8String(), ScriptableEquals(), IsStaticAtom(). (This duplication will disappear in the future if the representations of dynamic atoms and static atoms diverge. Indeed, SizeOfIncludingThis() is already different in the two classes.) - It replaces all mentions of "permanent"/"non-permanent" atoms with "static"/"dynamic". - In ~DynamicAtom() it removes the check that causes gAtomTable to be deleted when it becomes empty. This will only happen at shutdown and so doesn't seem useful. - It documents better various things, especially the basics of the dynamic/static split, the transmutation of dynamic atoms to static atoms, and the details of the SizeOf functions.
parser/html/nsHtml5Atom.cpp
xpcom/ds/nsAtomTable.cpp
xpcom/ds/nsIAtom.idl
xpcom/tests/gtest/TestAtoms.cpp
--- a/parser/html/nsHtml5Atom.cpp
+++ b/parser/html/nsHtml5Atom.cpp
@@ -75,8 +75,16 @@ nsHtml5Atom::IsStaticAtom()
 }
 
 NS_IMETHODIMP
 nsHtml5Atom::ScriptableEquals(const nsAString& aString, bool* aResult)
 {
   NS_NOTREACHED("Should not call ScriptableEquals.");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
+
+NS_IMETHODIMP_(size_t)
+nsHtml5Atom::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf)
+{
+  NS_NOTREACHED("Should not call SizeOfIncludingThis.");
+  return 0;
+}
+
--- a/xpcom/ds/nsAtomTable.cpp
+++ b/xpcom/ds/nsAtomTable.cpp
@@ -19,16 +19,28 @@
 #include "PLDHashTable.h"
 #include "prenv.h"
 #include "nsThreadUtils.h"
 #include "nsDataHashtable.h"
 #include "nsHashKeys.h"
 #include "nsAutoPtr.h"
 #include "nsUnicharUtils.h"
 
+// There are two kinds of atoms handled by this module.
+//
+// - DynamicAtom: the atom itself is heap allocated, as is the nsStringBuffer it
+//   points to. |gAtomTable| holds weak references to them DynamicAtoms, and
+//   when they are destroyed (due to their refcount reaching zero) they remove
+//   themselves from |gAtomTable|.
+//
+// - StaticAtom: the atom itself is heap allocated, but it points to a static
+//   nsStringBuffer. |gAtomTable| effectively owns StaticAtoms, because such
+//   atoms ignore all AddRef/Release calls, which ensures they stay alive until
+//   |gAtomTable| itself is destroyed whereupon they are explicitly deleted.
+
 using namespace mozilla;
 
 #if defined(__clang__)
 #  pragma GCC diagnostic ignored "-Wdelete-non-virtual-dtor"
 #elif MOZ_IS_GCC
 #  if MOZ_GCC_VERSION_AT_LEAST(4, 7, 0)
 #    pragma GCC diagnostic ignored "-Wdelete-non-virtual-dtor"
 #  endif
@@ -46,32 +58,36 @@ static PLDHashTable* gAtomTable;
 class StaticAtomEntry : public PLDHashEntryHdr
 {
 public:
   typedef const nsAString& KeyType;
   typedef const nsAString* KeyTypePointer;
 
   explicit StaticAtomEntry(KeyTypePointer aKey) {}
   StaticAtomEntry(const StaticAtomEntry& aOther) : mAtom(aOther.mAtom) {}
+
+  // We do not delete the atom because that's done when gAtomTable is
+  // destroyed -- which happens immediately after gStaticAtomTable is destroyed
+  // -- in NS_PurgeAtomTable().
   ~StaticAtomEntry() {}
 
   bool KeyEquals(KeyTypePointer aKey) const
   {
     return mAtom->Equals(*aKey);
   }
 
   static KeyTypePointer KeyToPointer(KeyType aKey) { return &aKey; }
   static PLDHashNumber HashKey(KeyTypePointer aKey)
   {
     return HashString(*aKey);
   }
 
   enum { ALLOW_MEMMOVE = true };
 
-  // mAtom only points to objects of type PermanentAtomImpl, which are not
+  // mAtom only points to objects of type StaticAtom, which are not
   // really refcounted.  But since these entries live in a global hashtable,
   // this reference is essentially owning.
   nsIAtom* MOZ_OWNING_REF mAtom;
 };
 
 /**
  * A hashtable of static atoms that existed at app startup. This hashtable
  * helps nsHtml5AtomTable.
@@ -81,111 +97,84 @@ static StaticAtomTable* gStaticAtomTable
 
 /**
  * Whether it is still OK to add atoms to gStaticAtomTable.
  */
 static bool gStaticAtomTableSealed = false;
 
 //----------------------------------------------------------------------
 
-/**
- * Note that AtomImpl objects are sometimes converted into PermanentAtomImpl
- * objects using placement new and just overwriting the vtable pointer.
- */
-
-class AtomImpl : public nsIAtom
+class DynamicAtom final : public nsIAtom
 {
 public:
-  AtomImpl(const nsAString& aString, uint32_t aHash);
-
-  // This is currently only used during startup when creating a permanent atom
-  // from NS_RegisterStaticAtoms
-  AtomImpl(nsStringBuffer* aData, uint32_t aLength, uint32_t aHash);
+  DynamicAtom(const nsAString& aString, uint32_t aHash);
 
-protected:
-  // This is only intended to be used when a normal atom is turned into a
-  // permanent one.
-  AtomImpl()
-  {
-    // We can't really assert that mString is a valid nsStringBuffer string,
-    // so do the best we can do and check for some consistencies.
-    NS_ASSERTION((mLength + 1) * sizeof(char16_t) <=
-                 nsStringBuffer::FromData(mString)->StorageSize() &&
-                 mString[mLength] == 0,
-                 "Not initialized atom");
-  }
-
-  // We don't need a virtual destructor here because PermanentAtomImpl
-  // deletions aren't handled through Release().
-  ~AtomImpl();
+private:
+  // We don't need a virtual destructor because we always delete via a
+  // DynamicAtom* pointer (in Release(), defined via NS_IMPL_ISUPPORTS), not an
+  // nsIAtom* pointer.
+  ~DynamicAtom();
 
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIATOM
 
-  enum { REFCNT_PERMANENT_SENTINEL = UINT32_MAX };
-
-  virtual bool IsPermanent();
-
-  // We can't use the virtual function in the base class destructor.
-  bool IsPermanentInDestructor()
-  {
-    return mRefCnt == REFCNT_PERMANENT_SENTINEL;
-  }
-
-  // for |#ifdef NS_BUILD_REFCNT_LOGGING| access to reference count
-  nsrefcnt GetRefCount() { return mRefCnt; }
-
-  size_t SizeOfIncludingThis(MallocSizeOf aMallocSizeOf);
+  void TransmuteToStatic(nsStringBuffer* aStringBuffer);
 };
 
-/**
- * A non-refcounted implementation of nsIAtom.
- */
-
-class PermanentAtomImpl final : public AtomImpl
+class StaticAtom final : public nsIAtom
 {
-public:
-  PermanentAtomImpl(const nsAString& aString, PLDHashNumber aKeyHash)
-    : AtomImpl(aString, aKeyHash)
-  {
-  }
-  PermanentAtomImpl(nsStringBuffer* aData, uint32_t aLength,
-                    PLDHashNumber aKeyHash)
-    : AtomImpl(aData, aLength, aKeyHash)
+  // This is the function that calls the private constructor.
+  friend void DynamicAtom::TransmuteToStatic(nsStringBuffer*);
+
+  // This constructor must only be used in conjunction with placement new on an
+  // existing DynamicAtom (in DynamicAtom::TransmuteToStatic()) in order to
+  // transmute that DynamicAtom into a StaticAtom. The constructor does three
+  // notable things.
+  // - Overwrites the vtable pointer (implicitly).
+  // - Zeroes the refcount (via the nsIAtom constructor). Having a zero refcount
+  //   doesn't matter because StaticAtom's AddRef/Release methods don't consult
+  //   the refcount.
+  // - Releases the existing heap-allocated string buffer (explicitly),
+  //   replacing it with the static string buffer (which must contain identical
+  //   chars).
+  explicit StaticAtom(nsStringBuffer* aStaticBuffer)
   {
-  }
-  PermanentAtomImpl() {}
-
-  ~PermanentAtomImpl();
-
-  virtual bool IsPermanent();
+    static_assert(sizeof(DynamicAtom) >= sizeof(StaticAtom),
+                  "can't safely transmute a smaller object to a bigger one");
 
-  // SizeOfIncludingThis() isn't needed -- the one inherited from AtomImpl is
-  // good enough, because PermanentAtomImpl doesn't add any new data members.
-
-  void* operator new(size_t aSize, AtomImpl* aAtom) CPP_THROW_NEW;
-  void* operator new(size_t aSize) CPP_THROW_NEW
-  {
-    return ::operator new(aSize);
+    char16_t* staticString = static_cast<char16_t*>(aStaticBuffer->Data());
+    MOZ_ASSERT(nsCRT::strcmp(staticString, mString) == 0);
+    nsStringBuffer* dynamicBuffer = nsStringBuffer::FromData(mString);
+    mString = staticString;
+    dynamicBuffer->Release();
   }
 
-private:
-  NS_IMETHOD_(MozExternalRefCountType) AddRef();
-  NS_IMETHOD_(MozExternalRefCountType) Release();
+public:
+  // This is the normal constructor.
+  StaticAtom(nsStringBuffer* aData, uint32_t aLength, PLDHashNumber aKeyHash);
+
+  // We don't need a virtual destructor because we always delete via a
+  // StaticAtom* pointer (in AtomTableClearEntry()), not an nsIAtom* pointer.
+  ~StaticAtom();
+
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSIATOM
 };
 
+NS_IMPL_QUERY_INTERFACE(StaticAtom, nsIAtom)
+
 //----------------------------------------------------------------------
 
 struct AtomTableEntry : public PLDHashEntryHdr
 {
-  // These references are either to non-permanent atoms, in which case they are
-  // non-owning, or they are to permanent atoms that are not really refcounted.
-  // The exact lifetime rules are documented in AtomTableClearEntry.
-  AtomImpl* MOZ_NON_OWNING_REF mAtom;
+  // These references are either to DynamicAtoms, in which case they are
+  // non-owning, or they are to StaticAtoms, which aren't really refcounted.
+  // See the comment at the top of this file for more details.
+  nsIAtom* MOZ_NON_OWNING_REF mAtom;
 };
 
 struct AtomTableKey
 {
   AtomTableKey(const char16_t* aUTF16String, uint32_t aLength, uint32_t aHash)
     : mUTF16String(aUTF16String)
     , mUTF8String(nullptr)
     , mLength(aLength)
@@ -262,95 +251,74 @@ AtomTableMatchKey(const PLDHashEntryHdr*
 
   return memcmp(he->mAtom->GetUTF16String(),
                 k->mUTF16String, length * sizeof(char16_t)) == 0;
 }
 
 static void
 AtomTableClearEntry(PLDHashTable* aTable, PLDHashEntryHdr* aEntry)
 {
-  // Normal |AtomImpl| atoms are deleted when their refcount hits 0, and
-  // they then remove themselves from the table.  In other words, they
-  // are owned by the callers who own references to them.
-  // |PermanentAtomImpl| permanent atoms ignore their refcount and are
-  // deleted when they are removed from the table at table destruction.
-  // In other words, they are owned by the atom table.
-
-  AtomImpl* atom = static_cast<AtomTableEntry*>(aEntry)->mAtom;
-  if (atom->IsPermanent()) {
-    // Note that the cast here is important since AtomImpls doesn't have a
-    // virtual dtor.
-    delete static_cast<PermanentAtomImpl*>(atom);
+  auto entry = static_cast<AtomTableEntry*>(aEntry);
+  nsIAtom* atom = entry->mAtom;
+  if (atom->IsStaticAtom()) {
+    // This case -- when the entry being cleared holds a StaticAtom -- only
+    // occurs when gAtomTable is destroyed, whereupon all StaticAtoms within it
+    // must be explicitly deleted. The cast is required because StaticAtom
+    // doesn't have a virtual destructor.
+    delete static_cast<StaticAtom*>(atom);
   }
 }
 
 static void
 AtomTableInitEntry(PLDHashEntryHdr* aEntry, const void* aKey)
 {
   static_cast<AtomTableEntry*>(aEntry)->mAtom = nullptr;
 }
 
-
 static const PLDHashTableOps AtomTableOps = {
   AtomTableGetHash,
   AtomTableMatchKey,
   PLDHashTable::MoveEntryStub,
   AtomTableClearEntry,
   AtomTableInitEntry
 };
 
-
-static inline
-void
-PromoteToPermanent(AtomImpl* aAtom)
-{
-#ifdef NS_BUILD_REFCNT_LOGGING
-  {
-    nsrefcnt refcount = aAtom->GetRefCount();
-    do {
-      NS_LOG_RELEASE(aAtom, --refcount, "AtomImpl");
-    } while (refcount);
-  }
-#endif
-  aAtom = new (aAtom) PermanentAtomImpl();
-}
-
 void
 NS_PurgeAtomTable()
 {
   delete gStaticAtomTable;
   gStaticAtomTable = nullptr;
 
   if (gAtomTable) {
 #ifdef DEBUG
     const char* dumpAtomLeaks = PR_GetEnv("MOZ_DUMP_ATOM_LEAKS");
     if (dumpAtomLeaks && *dumpAtomLeaks) {
       uint32_t leaked = 0;
-      printf("*** %d atoms still exist (including permanent):\n",
+      printf("*** %d atoms still exist (including static):\n",
              gAtomTable->EntryCount());
       for (auto iter = gAtomTable->Iter(); !iter.Done(); iter.Next()) {
         auto entry = static_cast<AtomTableEntry*>(iter.Get());
-        AtomImpl* atom = entry->mAtom;
-        if (!atom->IsPermanent()) {
+        nsIAtom* atom = entry->mAtom;
+        if (!atom->IsStaticAtom()) {
           leaked++;
           nsAutoCString str;
           atom->ToUTF8String(str);
           fputs(str.get(), stdout);
           fputs("\n", stdout);
         }
       }
-      printf("*** %u non-permanent atoms leaked\n", leaked);
+      printf("*** %u dynamic atoms leaked\n", leaked);
     }
 #endif
     delete gAtomTable;
     gAtomTable = nullptr;
   }
 }
 
-AtomImpl::AtomImpl(const nsAString& aString, uint32_t aHash)
+DynamicAtom::DynamicAtom(const nsAString& aString, uint32_t aHash)
 {
   mLength = aString.Length();
   RefPtr<nsStringBuffer> buf = nsStringBuffer::FromString(aString);
   if (buf) {
     mString = static_cast<char16_t*>(buf->Data());
   } else {
     buf = nsStringBuffer::Alloc((mLength + 1) * sizeof(char16_t));
     mString = static_cast<char16_t*>(buf->Data());
@@ -365,156 +333,163 @@ AtomImpl::AtomImpl(const nsAString& aStr
   NS_ASSERTION(buf && buf->StorageSize() >= (mLength + 1) * sizeof(char16_t),
                "enough storage");
   NS_ASSERTION(Equals(aString), "correct data");
 
   // Take ownership of buffer
   mozilla::Unused << buf.forget();
 }
 
-AtomImpl::AtomImpl(nsStringBuffer* aStringBuffer, uint32_t aLength,
-                   uint32_t aHash)
+StaticAtom::StaticAtom(nsStringBuffer* aStringBuffer, uint32_t aLength,
+                       uint32_t aHash)
 {
   mLength = aLength;
   mString = static_cast<char16_t*>(aStringBuffer->Data());
   // Technically we could currently avoid doing this addref by instead making
   // the static atom buffers have an initial refcount of 2.
   aStringBuffer->AddRef();
 
   mHash = aHash;
   MOZ_ASSERT(mHash == HashString(mString, mLength));
 
   NS_ASSERTION(mString[mLength] == char16_t(0), "null terminated");
   NS_ASSERTION(aStringBuffer &&
                aStringBuffer->StorageSize() == (mLength + 1) * sizeof(char16_t),
                "correct storage");
 }
 
-AtomImpl::~AtomImpl()
+DynamicAtom::~DynamicAtom()
 {
   MOZ_ASSERT(gAtomTable, "uninitialized atom hashtable");
-  // Permanent atoms are removed from the hashtable at shutdown, and we
-  // don't want to remove them twice.  See comment above in
-  // |AtomTableClearEntry|.
-  if (!IsPermanentInDestructor()) {
-    AtomTableKey key(mString, mLength, mHash);
-    gAtomTable->Remove(&key);
-    if (gAtomTable->EntryCount() == 0) {
-      delete gAtomTable;
-      gAtomTable = nullptr;
-    }
-  }
+
+  // DynamicAtoms must be removed from gAtomTable when their refcount reaches
+  // zero and they are released.
+  AtomTableKey key(mString, mLength, mHash);
+  gAtomTable->Remove(&key);
 
   nsStringBuffer::FromData(mString)->Release();
 }
 
-NS_IMPL_ISUPPORTS(AtomImpl, nsIAtom)
+NS_IMPL_ISUPPORTS(DynamicAtom, nsIAtom)
 
-PermanentAtomImpl::~PermanentAtomImpl()
+StaticAtom::~StaticAtom()
 {
-  // So we can tell if we were permanent while running the base class dtor.
-  mRefCnt = REFCNT_PERMANENT_SENTINEL;
 }
 
 NS_IMETHODIMP_(MozExternalRefCountType)
-PermanentAtomImpl::AddRef()
+StaticAtom::AddRef()
 {
   MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
   return 2;
 }
 
 NS_IMETHODIMP_(MozExternalRefCountType)
-PermanentAtomImpl::Release()
+StaticAtom::Release()
 {
   MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
   return 1;
 }
 
-/* virtual */ bool
-AtomImpl::IsPermanent()
-{
-  return false;
-}
-
-/* virtual */ bool
-PermanentAtomImpl::IsPermanent()
+NS_IMETHODIMP
+DynamicAtom::ScriptableToString(nsAString& aBuf)
 {
-  return true;
-}
-
-void*
-PermanentAtomImpl::operator new(size_t aSize, AtomImpl* aAtom) CPP_THROW_NEW
-{
-  MOZ_ASSERT(!aAtom->IsPermanent(),
-             "converting atom that's already permanent");
-
-  // Just let the constructor overwrite the vtable pointer.
-  return aAtom;
+  nsStringBuffer::FromData(mString)->ToString(mLength, aBuf);
+  return NS_OK;
 }
 
 NS_IMETHODIMP
-AtomImpl::ScriptableToString(nsAString& aBuf)
+StaticAtom::ScriptableToString(nsAString& aBuf)
 {
   nsStringBuffer::FromData(mString)->ToString(mLength, aBuf);
   return NS_OK;
 }
 
 NS_IMETHODIMP
-AtomImpl::ToUTF8String(nsACString& aBuf)
+DynamicAtom::ToUTF8String(nsACString& aBuf)
 {
   CopyUTF16toUTF8(nsDependentString(mString, mLength), aBuf);
   return NS_OK;
 }
 
 NS_IMETHODIMP
-AtomImpl::ScriptableEquals(const nsAString& aString, bool* aResult)
+StaticAtom::ToUTF8String(nsACString& aBuf)
+{
+  CopyUTF16toUTF8(nsDependentString(mString, mLength), aBuf);
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+DynamicAtom::ScriptableEquals(const nsAString& aString, bool* aResult)
+{
+  *aResult = aString.Equals(nsDependentString(mString, mLength));
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+StaticAtom::ScriptableEquals(const nsAString& aString, bool* aResult)
 {
   *aResult = aString.Equals(nsDependentString(mString, mLength));
   return NS_OK;
 }
 
 NS_IMETHODIMP_(bool)
-AtomImpl::IsStaticAtom()
+DynamicAtom::IsStaticAtom()
 {
-  return IsPermanent();
+  return false;
 }
 
-size_t
-AtomImpl::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf)
+NS_IMETHODIMP_(bool)
+StaticAtom::IsStaticAtom()
+{
+  return true;
+}
+
+NS_IMETHODIMP_(size_t)
+DynamicAtom::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf)
 {
   size_t n = aMallocSizeOf(this);
+  n += nsStringBuffer::FromData(mString)->SizeOfIncludingThisIfUnshared(
+         aMallocSizeOf);
+  return n;
+}
 
-  // Don't measure static atoms. Nb: here "static" means "permanent", and while
-  // it's not guaranteed that permanent atoms are actually stored in static
-  // data, it is very likely. And we don't want to call |aMallocSizeOf| on
-  // static data, so we err on the side of caution.
-  if (!IsStaticAtom()) {
-    n += nsStringBuffer::FromData(mString)->SizeOfIncludingThisIfUnshared(
-           aMallocSizeOf);
-  }
+NS_IMETHODIMP_(size_t)
+StaticAtom::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf)
+{
+  size_t n = aMallocSizeOf(this);
+  // Don't measure the string buffer pointed to by the StaticAtom because it's
+  // in static memory.
   return n;
 }
 
+// See the comment on the private StaticAtom constructor for details of how
+// this works.
+void
+DynamicAtom::TransmuteToStatic(nsStringBuffer* aStringBuffer)
+{
+  new (this) StaticAtom(aStringBuffer);
+}
+
 //----------------------------------------------------------------------
 
 void
 NS_SizeOfAtomTablesIncludingThis(MallocSizeOf aMallocSizeOf,
                                  size_t* aMain, size_t* aStatic)
 {
   *aMain = 0;
   if (gAtomTable) {
     *aMain += gAtomTable->ShallowSizeOfIncludingThis(aMallocSizeOf);
     for (auto iter = gAtomTable->Iter(); !iter.Done(); iter.Next()) {
       auto entry = static_cast<AtomTableEntry*>(iter.Get());
       *aMain += entry->mAtom->SizeOfIncludingThis(aMallocSizeOf);
     }
   }
 
-  // The atoms in the this table are almost certainly stored in static data, so
-  // we don't need to measure entries separately.
+  // The atoms pointed to by gStaticAtomTable are also pointed to by gAtomTable,
+  // and they're measured by the loop above. So no need to measure them here.
   *aStatic = gStaticAtomTable
            ? gStaticAtomTable->ShallowSizeOfIncludingThis(aMallocSizeOf)
            : 0;
 }
 
 // The atom table very quickly gets 10,000+ entries in it (or even 100,000+).
 // But choosing the best initial length has some subtleties: we add ~2700
 // static atoms to the table at start-up, and then we start adding and removing
@@ -578,44 +553,47 @@ class CheckStaticAtomSizes
 void
 RegisterStaticAtoms(const nsStaticAtom* aAtoms, uint32_t aAtomCount)
 {
   if (!gStaticAtomTable && !gStaticAtomTableSealed) {
     gStaticAtomTable = new StaticAtomTable();
   }
 
   for (uint32_t i = 0; i < aAtomCount; ++i) {
-    NS_ASSERTION(nsCRT::IsAscii((char16_t*)aAtoms[i].mStringBuffer->Data()),
-                 "Static atoms must be ASCII!");
+    nsStringBuffer* stringBuffer = aAtoms[i].mStringBuffer;
+    nsIAtom** atomp = aAtoms[i].mAtom;
 
-    uint32_t stringLen =
-      aAtoms[i].mStringBuffer->StorageSize() / sizeof(char16_t) - 1;
+    MOZ_ASSERT(nsCRT::IsAscii(static_cast<char16_t*>(stringBuffer->Data())));
+
+    uint32_t stringLen = stringBuffer->StorageSize() / sizeof(char16_t) - 1;
 
     uint32_t hash;
     AtomTableEntry* he =
-      GetAtomHashEntry((char16_t*)aAtoms[i].mStringBuffer->Data(),
+      GetAtomHashEntry(static_cast<char16_t*>(stringBuffer->Data()),
                        stringLen, &hash);
 
-    AtomImpl* atom = he->mAtom;
+    nsIAtom* atom = he->mAtom;
     if (atom) {
-      if (!atom->IsPermanent()) {
-        // We wanted to create a static atom but there is already a non-static
-        // atom there. So convert it to a non-refcounting permanent atom.
-        PromoteToPermanent(atom);
+      if (!atom->IsStaticAtom()) {
+        // A rare case: we're creating a StaticAtom but there is already a
+        // DynamicAtom of the same name. Transmute the DynamicAtom into a
+        // StaticAtom.
+        static_cast<DynamicAtom*>(atom)->TransmuteToStatic(stringBuffer);
       }
     } else {
-      atom = new PermanentAtomImpl(aAtoms[i].mStringBuffer, stringLen, hash);
+      atom = new StaticAtom(stringBuffer, stringLen, hash);
       he->mAtom = atom;
     }
-    *aAtoms[i].mAtom = atom;
+    *atomp = atom;
 
     if (!gStaticAtomTableSealed) {
       StaticAtomEntry* entry =
         gStaticAtomTable->PutEntry(nsDependentAtomString(atom));
-      entry->mAtom = atom;
+      MOZ_ASSERT(atom->IsStaticAtom());
+      entry->mAtom = static_cast<StaticAtom*>(atom);
     }
   }
 }
 
 already_AddRefed<nsIAtom>
 NS_Atomize(const char* aUTF8String)
 {
   return NS_Atomize(nsDependentCString(aUTF8String));
@@ -635,17 +613,17 @@ NS_Atomize(const nsACString& aUTF8String
     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<AtomImpl> atom = new AtomImpl(str, hash);
+  RefPtr<DynamicAtom> atom = new DynamicAtom(str, hash);
 
   he->mAtom = atom;
 
   return atom.forget();
 }
 
 already_AddRefed<nsIAtom>
 NS_Atomize(const char16_t* aUTF16String)
@@ -662,17 +640,17 @@ NS_Atomize(const nsAString& aUTF16String
                                         &hash);
 
   if (he->mAtom) {
     nsCOMPtr<nsIAtom> atom = he->mAtom;
 
     return atom.forget();
   }
 
-  RefPtr<AtomImpl> atom = new AtomImpl(aUTF16String, hash);
+  RefPtr<DynamicAtom> atom = new DynamicAtom(aUTF16String, hash);
   he->mAtom = atom;
 
   return atom.forget();
 }
 
 nsrefcnt
 NS_GetNumberOfAtoms(void)
 {
--- a/xpcom/ds/nsIAtom.idl
+++ b/xpcom/ds/nsIAtom.idl
@@ -5,16 +5,18 @@
 #include "nsISupports.idl"
 
 %{C++
 #include "nsStringGlue.h"
 #include "nsCOMPtr.h"
 #include "nsStringBuffer.h"
 %}
 
+native MallocSizeOf(mozilla::MallocSizeOf);
+
 /*
  * Should this really be scriptable?  Using atoms from script or proxies
  * could be dangerous since double-wrapping could lead to loss of
  * pointer identity.
  */
  
 [scriptable, builtinclass, uuid(8b8c11d4-3ed5-4079-8974-73c7576cdb34)]
 interface nsIAtom : nsISupports
@@ -31,16 +33,19 @@ interface nsIAtom : nsISupports
    */
   [binaryname(ScriptableEquals)] boolean equals(in AString aString);
 
   /**
    * Returns true if the atom is static and false otherwise.
    */
   [noscript, notxpcom] boolean isStaticAtom();
 
+  [noscript, notxpcom]
+  size_t SizeOfIncludingThis(in MallocSizeOf aMallocSizeOf);
+
 %{C++
   // note this is NOT virtual so this won't muck with the vtable!
   inline bool Equals(const nsAString& aString) const {
     return aString.Equals(nsDependentString(mString, mLength));
   }
 
   inline char16ptr_t GetUTF16String() const {
     return mString;
--- a/xpcom/tests/gtest/TestAtoms.cpp
+++ b/xpcom/tests/gtest/TestAtoms.cpp
@@ -56,17 +56,17 @@ TEST(Atoms, BufferSharing)
   nsString unique;
   unique.AssignLiteral("this is a unique string !@#$");
 
   nsCOMPtr<nsIAtom> atom = NS_Atomize(unique);
 
   EXPECT_EQ(unique.get(), atom->GetUTF16String());
 }
 
-TEST(Atoms, NUll)
+TEST(Atoms, Null)
 {
   nsAutoString str(NS_LITERAL_STRING("string with a \0 char"));
   nsDependentString strCut(str.get());
 
   EXPECT_FALSE(str.Equals(strCut));
 
   nsCOMPtr<nsIAtom> atomCut = NS_Atomize(strCut);
   nsCOMPtr<nsIAtom> atom = NS_Atomize(str);
@@ -148,31 +148,31 @@ isStaticAtom(nsIAtom* atom)
   rv &= (atom->Release() == 1);
   return rv;
 }
 
 TEST(Atoms, Table)
 {
   nsrefcnt count = NS_GetNumberOfAtoms();
 
-  nsCOMPtr<nsIAtom> thirdNonPerm = NS_Atomize(THIRD_ATOM_STR);
+  nsCOMPtr<nsIAtom> thirdDynamic = NS_Atomize(THIRD_ATOM_STR);
 
-  EXPECT_FALSE(isStaticAtom(thirdNonPerm));
+  EXPECT_FALSE(isStaticAtom(thirdDynamic));
 
-  EXPECT_TRUE(thirdNonPerm);
+  EXPECT_TRUE(thirdDynamic);
   EXPECT_EQ(NS_GetNumberOfAtoms(), count + 1);
 
   NS_RegisterStaticAtoms(sAtoms_info);
 
   EXPECT_TRUE(sAtom1);
   EXPECT_TRUE(sAtom1->Equals(NS_LITERAL_STRING(FIRST_ATOM_STR)));
   EXPECT_TRUE(isStaticAtom(sAtom1));
   EXPECT_TRUE(sAtom2);
   EXPECT_TRUE(sAtom2->Equals(NS_LITERAL_STRING(SECOND_ATOM_STR)));
   EXPECT_TRUE(isStaticAtom(sAtom2));
   EXPECT_TRUE(sAtom3);
   EXPECT_TRUE(sAtom3->Equals(NS_LITERAL_STRING(THIRD_ATOM_STR)));
   EXPECT_TRUE(isStaticAtom(sAtom3));
   EXPECT_EQ(NS_GetNumberOfAtoms(), count + 3);
-  EXPECT_EQ(thirdNonPerm, sAtom3);
+  EXPECT_EQ(thirdDynamic, sAtom3);
 }
 
 }