Bug 1261735 (part 3) - De-virtualize nsIAtom::IsStaticAtom(). r=froydnj,erahm.
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 06 Apr 2016 11:28:40 +1000
changeset 291986 916383c80fc7ebf99ff3eee28aad272a81c4682f
parent 291985 492d49a7e4f06587f470c78f4411957f9e4fb4d0
child 291987 3e21ac5e6a8b8364c769867c2118efa9c8e89134
push id30149
push usercbook@mozilla.com
push dateThu, 07 Apr 2016 09:52:31 +0000
treeherdermozilla-central@b6683e141c47 [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 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.
parser/html/nsHtml5Atom.cpp
xpcom/ds/nsAtomTable.cpp
xpcom/ds/nsIAtom.idl
--- a/parser/html/nsHtml5Atom.cpp
+++ b/parser/html/nsHtml5Atom.cpp
@@ -4,16 +4,17 @@
 
 #include "nsHtml5Atom.h"
 #include "nsAutoPtr.h"
 #include "mozilla/unused.h"
 
 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());
   } else {
     buf = nsStringBuffer::Alloc((mLength + 1) * sizeof(char16_t));
     mString = static_cast<char16_t*>(buf->Data());
     CopyUnicodeTo(aString, 0, mString, mLength);
     mString[mLength] = char16_t(0);
@@ -63,22 +64,16 @@ nsHtml5Atom::ScriptableToString(nsAStrin
 
 NS_IMETHODIMP
 nsHtml5Atom::ToUTF8String(nsACString& aReturn)
 {
   NS_NOTREACHED("Should not attempt to convert to an UTF-8 string.");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
-NS_IMETHODIMP_(bool)
-nsHtml5Atom::IsStaticAtom()
-{
-  return false;
-}
-
 NS_IMETHODIMP
 nsHtml5Atom::ScriptableEquals(const nsAString& aString, bool* aResult)
 {
   NS_NOTREACHED("Should not call ScriptableEquals.");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP_(size_t)
--- a/xpcom/ds/nsAtomTable.cpp
+++ b/xpcom/ds/nsAtomTable.cpp
@@ -103,16 +103,17 @@ static bool gStaticAtomTableSealed = fal
 //----------------------------------------------------------------------
 
 class DynamicAtom final : public nsIAtom
 {
 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());
     } else {
       buf = nsStringBuffer::Alloc((mLength + 1) * sizeof(char16_t));
       mString = static_cast<char16_t*>(buf->Data());
       CopyUnicodeTo(aString, 0, mString, mLength);
       mString[mLength] = char16_t(0);
@@ -147,42 +148,48 @@ NS_IMPL_ISUPPORTS(DynamicAtom, nsIAtom)
 
 class StaticAtom final : public nsIAtom
 {
   // 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
+  // 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.
   // - Releases the existing heap-allocated string buffer (explicitly),
   //   replacing it with the static string buffer (which must contain identical
   //   chars).
   explicit StaticAtom(nsStringBuffer* aStaticBuffer)
   {
     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);
     mString = staticString;
     dynamicBuffer->Release();
   }
 
 public:
   // This is the normal constructor.
   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.
     aStringBuffer->AddRef();
 
     mHash = aHash;
     MOZ_ASSERT(mHash == HashString(mString, mLength));
 
@@ -416,28 +423,16 @@ DynamicAtom::ScriptableEquals(const nsAS
 
 NS_IMETHODIMP
 StaticAtom::ScriptableEquals(const nsAString& aString, bool* aResult)
 {
   *aResult = aString.Equals(nsDependentString(mString, mLength));
   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)
 {
   size_t n = aMallocSizeOf(this);
   n += nsStringBuffer::FromData(mString)->SizeOfIncludingThisIfUnshared(
          aMallocSizeOf);
   return n;
 }
--- a/xpcom/ds/nsIAtom.idl
+++ b/xpcom/ds/nsIAtom.idl
@@ -28,30 +28,29 @@ interface nsIAtom : nsISupports
   [noscript] AUTF8String toUTF8String();
   
   /**
    * Compare the atom to a specific string value
    * Note that this will NEVER return/throw an error condition.
    */
   [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 bool IsStaticAtom() const {
+    return mIsStatic;
+  }
+
   inline char16ptr_t GetUTF16String() const {
     return mString;
   }
 
   inline uint32_t GetLength() const {
     return mLength;
   }
 
@@ -70,17 +69,18 @@ interface nsIAtom : nsISupports
    * pointer, for use in situations that need a well-distributed
    * hashcode.
    */
   inline uint32_t hash() const {
     return mHash;
   }
 
 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
    * points to must belong to an nsStringBuffer. This is so that the
    * nsStringBuffer::FromData() calls above are valid.
    */
   char16_t* mString;
 %}