Bug 1261735 (part 3) - De-virtualize nsIAtom::IsStaticAtom(). r=froydnj,erahm.

This avoids the need for some virtual function calls and also will help lead to
distinct representations for dynamic and static atoms.

--HG--
extra : rebase_source : 16bbe6f1e1309ee3e4fab7a0d222e638178a2a9c

parser/html/nsHtml5Atom.cpp

@@ -9,6 +9,7 @@
 nsHtml5Atom::nsHtml5Atom(const nsAString& aString)
 {
   mLength = aString.Length();
+  mIsStatic = false;
   RefPtr<nsStringBuffer> buf = nsStringBuffer::FromString(aString);
   if (buf) {
     mString = static_cast<char16_t*>(buf->Data());
@@ -68,12 +69,6 @@ nsHtml5Atom::ToUTF8String(nsACString& aReturn)
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
-NS_IMETHODIMP_(bool)
-nsHtml5Atom::IsStaticAtom()
-{
-  return false;
-}
-
 NS_IMETHODIMP
 nsHtml5Atom::ScriptableEquals(const nsAString& aString, bool* aResult)
 {

xpcom/ds/nsAtomTable.cpp

@@ -108,6 +108,7 @@ public:
   DynamicAtom(const nsAString& aString, uint32_t aHash)
   {
     mLength = aString.Length();
+    mIsStatic = false;
     RefPtr<nsStringBuffer> buf = nsStringBuffer::FromString(aString);
     if (buf) {
       mString = static_cast<char16_t*>(buf->Data());
@@ -152,9 +153,10 @@ class StaticAtom final : public nsIAtom
 
   // 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
+  // transmute that DynamicAtom into a StaticAtom. The constructor does four
   // notable things.
   // - Overwrites the vtable pointer (implicitly).
+  // - Inverts mIsStatic.
   // - 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.
@@ -166,6 +168,10 @@ class StaticAtom final : public nsIAtom
     static_assert(sizeof(DynamicAtom) >= sizeof(StaticAtom),
                   "can't safely transmute a smaller object to a bigger one");
 
+    // We must be transmuting an existing DynamicAtom.
+    MOZ_ASSERT(!mIsStatic);
+    mIsStatic = true;
+
     char16_t* staticString = static_cast<char16_t*>(aStaticBuffer->Data());
     MOZ_ASSERT(nsCRT::strcmp(staticString, mString) == 0);
     nsStringBuffer* dynamicBuffer = nsStringBuffer::FromData(mString);
@@ -178,6 +184,7 @@ public:
   StaticAtom(nsStringBuffer* aStringBuffer, uint32_t aLength, uint32_t aHash)
   {
     mLength = aLength;
+    mIsStatic = true;
     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.
@@ -421,18 +428,6 @@ StaticAtom::ScriptableEquals(const nsAString& aString, bool* aResult)
   return NS_OK;
 }
 
-NS_IMETHODIMP_(bool)
-DynamicAtom::IsStaticAtom()
-{
-  return false;
-}
-
-NS_IMETHODIMP_(bool)
-StaticAtom::IsStaticAtom()
-{
-  return true;
-}
-
 NS_IMETHODIMP_(size_t)
 DynamicAtom::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf)
 {

xpcom/ds/nsIAtom.idl

@@ -33,11 +33,6 @@ 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);
 
@@ -47,6 +42,10 @@ interface nsIAtom : nsISupports
     return aString.Equals(nsDependentString(mString, mLength));
   }
 
+  inline bool IsStaticAtom() const {
+    return mIsStatic;
+  }
+
   inline char16ptr_t GetUTF16String() const {
     return mString;
   }
@@ -75,7 +74,8 @@ interface nsIAtom : nsISupports
   }
 
 protected:
-  uint32_t mLength;
+  uint32_t mLength:31;
+  uint32_t mIsStatic:1;
   uint32_t mHash;
   /**
    * WARNING! There is an invisible constraint on |mString|: the chars it