Bug 1482866: Simplify AttrArray's implementation. r=smaug
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 13 Aug 2018 15:35:57 +0200
changeset 486801 cba97273eb10aa51a654d7a802c843b3e149b903
parent 486800 c8fa2901262d8e063b3b8f4d6c476888666c23ea
child 486802 052b350b861bbc68c4400cd3909c4e81fadd0d3c
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1482866
milestone63.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 1482866: Simplify AttrArray's implementation. r=smaug No need for void* stuff, but I had to keep the semi-manual memory management to allocate everything inline as we currently do. Differential Revision: https://phabricator.services.mozilla.com/D3218
dom/base/AttrArray.cpp
dom/base/AttrArray.h
--- a/dom/base/AttrArray.cpp
+++ b/dom/base/AttrArray.cpp
@@ -19,86 +19,56 @@
 #include "nsString.h"
 #include "nsHTMLStyleSheet.h"
 #include "nsMappedAttributes.h"
 #include "nsUnicharUtils.h"
 #include "nsContentUtils.h" // nsAutoScriptBlocker
 
 using mozilla::CheckedUint32;
 
-/**
- * Due to a compiler bug in VisualAge C++ for AIX, we need to return the
- * address of the first index into mBuffer here, instead of simply returning
- * mBuffer itself.
- *
- * See Bug 231104 for more information.
- */
-#define ATTRS(_impl) \
-  reinterpret_cast<InternalAttr*>(&((_impl)->mBuffer[0]))
-
-
-#define NS_IMPL_EXTRA_SIZE \
-  ((sizeof(Impl) - sizeof(mImpl->mBuffer)) / sizeof(void*))
-
-AttrArray::AttrArray()
-  : mImpl(nullptr)
+AttrArray::Impl::~Impl()
 {
-}
-
-AttrArray::~AttrArray()
-{
-  if (!mImpl) {
-    return;
+  for (InternalAttr& attr : NonMappedAttrs()) {
+    attr.~InternalAttr();
   }
 
-  Clear();
-
-  free(mImpl);
-}
-
-uint32_t
-AttrArray::AttrCount() const
-{
-  return NonMappedAttrCount() + MappedAttrCount();
+  NS_IF_RELEASE(mMappedAttrs);
 }
 
 const nsAttrValue*
 AttrArray::GetAttr(nsAtom* aLocalName, int32_t aNamespaceID) const
 {
-  uint32_t i, slotCount = AttrSlotCount();
   if (aNamespaceID == kNameSpaceID_None) {
     // This should be the common case so lets make an optimized loop
-    for (i = 0; i < slotCount && AttrSlotIsTaken(i); ++i) {
-      if (ATTRS(mImpl)[i].mName.Equals(aLocalName)) {
-        return &ATTRS(mImpl)[i].mValue;
+    for (const InternalAttr& attr : NonMappedAttrs()) {
+      if (attr.mName.Equals(aLocalName)) {
+        return &attr.mValue;
       }
     }
 
     if (mImpl && mImpl->mMappedAttrs) {
       return mImpl->mMappedAttrs->GetAttr(aLocalName);
     }
-  }
-  else {
-    for (i = 0; i < slotCount && AttrSlotIsTaken(i); ++i) {
-      if (ATTRS(mImpl)[i].mName.Equals(aLocalName, aNamespaceID)) {
-        return &ATTRS(mImpl)[i].mValue;
+  } else {
+    for (const InternalAttr& attr : NonMappedAttrs()) {
+      if (attr.mName.Equals(aLocalName, aNamespaceID)) {
+        return &attr.mValue;
       }
     }
   }
 
   return nullptr;
 }
 
 const nsAttrValue*
 AttrArray::GetAttr(const nsAString& aLocalName) const
 {
-  uint32_t i, slotCount = AttrSlotCount();
-  for (i = 0; i < slotCount && AttrSlotIsTaken(i); ++i) {
-    if (ATTRS(mImpl)[i].mName.Equals(aLocalName)) {
-      return &ATTRS(mImpl)[i].mValue;
+  for (const InternalAttr& attr : NonMappedAttrs()) {
+    if (attr.mName.Equals(aLocalName)) {
+      return &attr.mValue;
     }
   }
 
   if (mImpl && mImpl->mMappedAttrs) {
     return mImpl->mMappedAttrs->GetAttr(aLocalName);
   }
 
   return nullptr;
@@ -114,120 +84,116 @@ AttrArray::GetAttr(const nsAString& aNam
       nsContentUtils::StringContainsASCIIUpper(aName)) {
     // Try again with a lowercased name, but make sure we can't reenter this
     // block by passing eCaseSensitive for aCaseSensitive.
     nsAutoString lowercase;
     nsContentUtils::ASCIIToLower(aName, lowercase);
     return GetAttr(lowercase, eCaseMatters);
   }
 
-  uint32_t i, slotCount = AttrSlotCount();
-  for (i = 0; i < slotCount && AttrSlotIsTaken(i); ++i) {
-    if (ATTRS(mImpl)[i].mName.QualifiedNameEquals(aName)) {
-      return &ATTRS(mImpl)[i].mValue;
+  for (const InternalAttr& attr : NonMappedAttrs()) {
+    if (attr.mName.QualifiedNameEquals(aName)) {
+      return &attr.mValue;
     }
   }
 
   if (mImpl && mImpl->mMappedAttrs) {
-    const nsAttrValue* val =
-      mImpl->mMappedAttrs->GetAttr(aName);
-    if (val) {
-      return val;
-    }
+    return mImpl->mMappedAttrs->GetAttr(aName);
   }
 
   return nullptr;
 }
 
 const nsAttrValue*
 AttrArray::AttrAt(uint32_t aPos) const
 {
   NS_ASSERTION(aPos < AttrCount(),
                "out-of-bounds access in AttrArray");
 
   uint32_t nonmapped = NonMappedAttrCount();
   if (aPos < nonmapped) {
-    return &ATTRS(mImpl)[aPos].mValue;
+    return &mImpl->NonMappedAttrs()[aPos].mValue;
   }
 
   return mImpl->mMappedAttrs->AttrAt(aPos - nonmapped);
 }
 
+template<typename Name>
+inline nsresult
+AttrArray::AddNewAttribute(Name* aName, nsAttrValue& aValue)
+{
+  MOZ_ASSERT(!mImpl || mImpl->mCapacity >= mImpl->mAttrCount);
+  if (!mImpl || mImpl->mCapacity == mImpl->mAttrCount) {
+    if (!GrowBy(1)) {
+      return NS_ERROR_OUT_OF_MEMORY;
+    }
+  }
+
+  InternalAttr& attr = mImpl->mBuffer[mImpl->mAttrCount++];
+  new (&attr.mName) nsAttrName(aName);
+  new (&attr.mValue) nsAttrValue();
+  attr.mValue.SwapValueWith(aValue);
+  return NS_OK;
+}
+
 nsresult
 AttrArray::SetAndSwapAttr(nsAtom* aLocalName, nsAttrValue& aValue,
                           bool* aHadValue)
 {
   *aHadValue = false;
-  uint32_t i, slotCount = AttrSlotCount();
-  for (i = 0; i < slotCount && AttrSlotIsTaken(i); ++i) {
-    if (ATTRS(mImpl)[i].mName.Equals(aLocalName)) {
-      ATTRS(mImpl)[i].mValue.SwapValueWith(aValue);
+
+  for (InternalAttr& attr : NonMappedAttrs()) {
+    if (attr.mName.Equals(aLocalName)) {
+      attr.mValue.SwapValueWith(aValue);
       *aHadValue = true;
       return NS_OK;
     }
   }
 
-  if (i == slotCount && !AddAttrSlot()) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-
-  new (&ATTRS(mImpl)[i].mName) nsAttrName(aLocalName);
-  new (&ATTRS(mImpl)[i].mValue) nsAttrValue();
-  ATTRS(mImpl)[i].mValue.SwapValueWith(aValue);
-
-  return NS_OK;
+  return AddNewAttribute(aLocalName, aValue);
 }
 
 nsresult
 AttrArray::SetAndSwapAttr(mozilla::dom::NodeInfo* aName,
                           nsAttrValue& aValue, bool* aHadValue)
 {
   int32_t namespaceID = aName->NamespaceID();
   nsAtom* localName = aName->NameAtom();
   if (namespaceID == kNameSpaceID_None) {
     return SetAndSwapAttr(localName, aValue, aHadValue);
   }
 
   *aHadValue = false;
-  uint32_t i, slotCount = AttrSlotCount();
-  for (i = 0; i < slotCount && AttrSlotIsTaken(i); ++i) {
-    if (ATTRS(mImpl)[i].mName.Equals(localName, namespaceID)) {
-      ATTRS(mImpl)[i].mName.SetTo(aName);
-      ATTRS(mImpl)[i].mValue.SwapValueWith(aValue);
+  for (InternalAttr& attr : NonMappedAttrs()) {
+    if (attr.mName.Equals(localName, namespaceID)) {
+      attr.mName.SetTo(aName);
+      attr.mValue.SwapValueWith(aValue);
       *aHadValue = true;
       return NS_OK;
     }
   }
 
-  if (i == slotCount && !AddAttrSlot()) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-
-  new (&ATTRS(mImpl)[i].mName) nsAttrName(aName);
-  new (&ATTRS(mImpl)[i].mValue) nsAttrValue();
-  ATTRS(mImpl)[i].mValue.SwapValueWith(aValue);
-
-  return NS_OK;
+  return AddNewAttribute(aName, aValue);
 }
 
 nsresult
 AttrArray::RemoveAttrAt(uint32_t aPos, nsAttrValue& aValue)
 {
   NS_ASSERTION(aPos < AttrCount(), "out-of-bounds");
 
   uint32_t nonmapped = NonMappedAttrCount();
   if (aPos < nonmapped) {
-    ATTRS(mImpl)[aPos].mValue.SwapValueWith(aValue);
-    ATTRS(mImpl)[aPos].~InternalAttr();
+    mImpl->mBuffer[aPos].mValue.SwapValueWith(aValue);
+    mImpl->mBuffer[aPos].~InternalAttr();
 
-    uint32_t slotCount = AttrSlotCount();
-    memmove(&ATTRS(mImpl)[aPos],
-            &ATTRS(mImpl)[aPos + 1],
-            (slotCount - aPos - 1) * sizeof(InternalAttr));
-    memset(&ATTRS(mImpl)[slotCount - 1], 0, sizeof(InternalAttr));
+    memmove(mImpl->mBuffer + aPos,
+            mImpl->mBuffer + aPos + 1,
+            (mImpl->mAttrCount - aPos - 1) * sizeof(InternalAttr));
+
+    --mImpl->mAttrCount;
 
     return NS_OK;
   }
 
   if (MappedAttrCount() == 1) {
     // We're removing the last mapped attribute.  Can't swap in this
     // case; have to copy.
     aValue.SetTo(*mImpl->mMappedAttrs->AttrAt(0));
@@ -247,106 +213,105 @@ AttrArray::RemoveAttrAt(uint32_t aPos, n
 mozilla::dom::BorrowedAttrInfo
 AttrArray::AttrInfoAt(uint32_t aPos) const
 {
   NS_ASSERTION(aPos < AttrCount(),
                "out-of-bounds access in AttrArray");
 
   uint32_t nonmapped = NonMappedAttrCount();
   if (aPos < nonmapped) {
-    return BorrowedAttrInfo(&ATTRS(mImpl)[aPos].mName, &ATTRS(mImpl)[aPos].mValue);
+    InternalAttr& attr = mImpl->mBuffer[aPos];
+    return BorrowedAttrInfo(&attr.mName, &attr.mValue);
   }
 
-  return BorrowedAttrInfo(mImpl->mMappedAttrs->NameAt(aPos - nonmapped),
-                    mImpl->mMappedAttrs->AttrAt(aPos - nonmapped));
+  return BorrowedAttrInfo(
+      mImpl->mMappedAttrs->NameAt(aPos - nonmapped),
+      mImpl->mMappedAttrs->AttrAt(aPos - nonmapped));
 }
 
 const nsAttrName*
 AttrArray::AttrNameAt(uint32_t aPos) const
 {
   NS_ASSERTION(aPos < AttrCount(),
                "out-of-bounds access in AttrArray");
 
   uint32_t nonmapped = NonMappedAttrCount();
   if (aPos < nonmapped) {
-    return &ATTRS(mImpl)[aPos].mName;
+    return &mImpl->mBuffer[aPos].mName;
   }
 
   return mImpl->mMappedAttrs->NameAt(aPos - nonmapped);
 }
 
 const nsAttrName*
 AttrArray::GetSafeAttrNameAt(uint32_t aPos) const
 {
   uint32_t nonmapped = NonMappedAttrCount();
   if (aPos < nonmapped) {
-    void** pos = mImpl->mBuffer + aPos * ATTRSIZE;
-    if (!*pos) {
-      return nullptr;
-    }
-
-    return &reinterpret_cast<InternalAttr*>(pos)->mName;
+    return &mImpl->mBuffer[aPos].mName;
   }
 
   if (aPos >= AttrCount()) {
     return nullptr;
   }
 
   return mImpl->mMappedAttrs->NameAt(aPos - nonmapped);
 }
 
 const nsAttrName*
 AttrArray::GetExistingAttrNameFromQName(const nsAString& aName) const
 {
-  uint32_t i, slotCount = AttrSlotCount();
-  for (i = 0; i < slotCount && AttrSlotIsTaken(i); ++i) {
-    if (ATTRS(mImpl)[i].mName.QualifiedNameEquals(aName)) {
-      return &ATTRS(mImpl)[i].mName;
+  for (const InternalAttr& attr : NonMappedAttrs()) {
+    if (attr.mName.QualifiedNameEquals(aName)) {
+      return &attr.mName;
     }
   }
 
   if (mImpl && mImpl->mMappedAttrs) {
     return mImpl->mMappedAttrs->GetExistingAttrNameFromQName(aName);
   }
 
   return nullptr;
 }
 
 int32_t
 AttrArray::IndexOfAttr(nsAtom* aLocalName, int32_t aNamespaceID) const
 {
+  if (!mImpl) {
+    return -1;
+  }
+
   int32_t idx;
-  if (mImpl && mImpl->mMappedAttrs && aNamespaceID == kNameSpaceID_None) {
+  if (mImpl->mMappedAttrs && aNamespaceID == kNameSpaceID_None) {
     idx = mImpl->mMappedAttrs->IndexOfAttr(aLocalName);
     if (idx >= 0) {
       return NonMappedAttrCount() + idx;
     }
   }
 
-  uint32_t i;
-  uint32_t slotCount = AttrSlotCount();
+  uint32_t i = 0;
   if (aNamespaceID == kNameSpaceID_None) {
     // This should be the common case so lets make an optimized loop
     // Note that here we don't check for AttrSlotIsTaken() in the loop
     // condition for the sake of performance because comparing aLocalName
     // against null would fail in the loop body (since Equals() just compares
     // the raw pointer value of aLocalName to what AttrSlotIsTaken() would be
     // checking.
-    for (i = 0; i < slotCount; ++i) {
-      if (ATTRS(mImpl)[i].mName.Equals(aLocalName)) {
-        MOZ_ASSERT(AttrSlotIsTaken(i), "sanity check");
+    for (const InternalAttr& attr : NonMappedAttrs()) {
+      if (attr.mName.Equals(aLocalName)) {
         return i;
       }
+      ++i;
     }
-  }
-  else {
-    for (i = 0; i < slotCount && AttrSlotIsTaken(i); ++i) {
-      if (ATTRS(mImpl)[i].mName.Equals(aLocalName, aNamespaceID)) {
+  } else {
+    for (const InternalAttr& attr : NonMappedAttrs()) {
+      if (attr.mName.Equals(aLocalName, aNamespaceID)) {
         return i;
       }
+      ++i;
     }
   }
 
   return -1;
 }
 
 nsresult
 AttrArray::SetAndSwapMappedAttr(nsAtom* aLocalName,
@@ -401,76 +366,39 @@ AttrArray::DoUpdateMappedAttrRuleMapper(
 
 void
 AttrArray::Compact()
 {
   if (!mImpl) {
     return;
   }
 
-  // First compress away empty attrslots
-  uint32_t slotCount = AttrSlotCount();
-  uint32_t attrCount = NonMappedAttrCount();
-
-  if (attrCount < slotCount) {
-    SetAttrSlotCount(attrCount);
+  if (!mImpl->mAttrCount && !mImpl->mMappedAttrs) {
+    mImpl.reset();
+    return;
   }
 
-  // Then resize or free buffer
-  uint32_t newSize = attrCount * ATTRSIZE;
-  if (!newSize && !mImpl->mMappedAttrs) {
-    free(mImpl);
-    mImpl = nullptr;
-  }
-  else if (newSize < mImpl->mBufferSize) {
-    mImpl = static_cast<Impl*>(realloc(mImpl, (newSize + NS_IMPL_EXTRA_SIZE) * sizeof(nsIContent*)));
-    NS_ASSERTION(mImpl, "failed to reallocate to smaller buffer");
-
-    mImpl->mBufferSize = newSize;
-  }
-}
-
-void
-AttrArray::Clear()
-{
-  if (!mImpl) {
+  // Nothing to do.
+  if (mImpl->mAttrCount == mImpl->mCapacity) {
     return;
   }
 
-  if (mImpl->mMappedAttrs) {
-    NS_RELEASE(mImpl->mMappedAttrs);
-  }
-
-  uint32_t i, slotCount = AttrSlotCount();
-  for (i = 0; i < slotCount && AttrSlotIsTaken(i); ++i) {
-    ATTRS(mImpl)[i].~InternalAttr();
-  }
-
-  SetAttrSlotCount(0);
+  Impl* impl = mImpl.release();
+  impl = static_cast<Impl*>(
+    realloc(impl, Impl::AllocationSizeForAttributes(impl->mAttrCount)));
+  MOZ_ASSERT(impl, "failed to reallocate to a smaller buffer!");
+  impl->mCapacity = impl->mAttrCount;
+  mImpl.reset(impl);
 }
 
 uint32_t
-AttrArray::NonMappedAttrCount() const
+AttrArray::DoGetMappedAttrCount() const
 {
-  if (!mImpl) {
-    return 0;
-  }
-
-  uint32_t count = AttrSlotCount();
-  while (count > 0 && !mImpl->mBuffer[(count - 1) * ATTRSIZE]) {
-    --count;
-  }
-
-  return count;
-}
-
-uint32_t
-AttrArray::MappedAttrCount() const
-{
-  return mImpl && mImpl->mMappedAttrs ? (uint32_t)mImpl->mMappedAttrs->Count() : 0;
+  MOZ_ASSERT(mImpl && mImpl->mMappedAttrs);
+  return static_cast<uint32_t>(mImpl->mMappedAttrs->Count());
 }
 
 nsresult
 AttrArray::ForceMapped(nsMappedAttributeElement* aContent, nsIDocument* aDocument)
 {
   nsHTMLStyleSheet* sheet = aDocument->GetAttributeStyleSheet();
   RefPtr<nsMappedAttributes> mapped = GetModifiableMapped(aContent, sheet, false, 0);
   return MakeMappedUnique(mapped);
@@ -543,137 +471,99 @@ AttrArray::GetMapped() const
 }
 
 nsresult
 AttrArray::EnsureCapacityToClone(const AttrArray& aOther)
 {
   MOZ_ASSERT(!mImpl, "AttrArray::EnsureCapacityToClone requires the array be empty when called");
 
   uint32_t attrCount = aOther.NonMappedAttrCount();
-
-  if (attrCount == 0) {
+  if (!attrCount) {
     return NS_OK;
   }
 
   // No need to use a CheckedUint32 because we are cloning. We know that we
   // have already allocated an AttrArray of this size.
-  uint32_t size = attrCount;
-  size *= ATTRSIZE;
-  uint32_t totalSize = size;
-  totalSize += NS_IMPL_EXTRA_SIZE;
-
-  mImpl = static_cast<Impl*>(malloc(totalSize * sizeof(void*)));
+  mImpl.reset(static_cast<Impl*>(malloc(
+    Impl::AllocationSizeForAttributes(attrCount))));
   NS_ENSURE_TRUE(mImpl, NS_ERROR_OUT_OF_MEMORY);
 
   mImpl->mMappedAttrs = nullptr;
-  mImpl->mBufferSize = size;
-
-  // The array is now the right size, but we should reserve the correct
-  // number of slots for attributes so that children don't get written into
-  // that part of the array (which will then need to be moved later).
-  memset(static_cast<void*>(mImpl->mBuffer), 0, sizeof(InternalAttr) * attrCount);
-  SetAttrSlotCount(attrCount);
+  mImpl->mCapacity = attrCount;
+  mImpl->mAttrCount = 0;
 
   return NS_OK;
 }
 
 bool
 AttrArray::GrowBy(uint32_t aGrowSize)
 {
-  CheckedUint32 size = 0;
-  if (mImpl) {
-    size += mImpl->mBufferSize;
-    size += NS_IMPL_EXTRA_SIZE;
-    if (!size.isValid()) {
-      return false;
-    }
-  }
+  const uint32_t kLinearThreshold = 16;
+  const uint32_t kLinearGrowSize = 4;
 
-  CheckedUint32 minSize = size.value();
-  minSize += aGrowSize;
-  if (!minSize.isValid()) {
+  CheckedUint32 capacity = mImpl ? mImpl->mCapacity : 0;
+  CheckedUint32 minCapacity = capacity;
+  minCapacity += aGrowSize;
+  if (!minCapacity.isValid()) {
     return false;
   }
 
-  if (minSize.value() <= ATTRCHILD_ARRAY_LINEAR_THRESHOLD) {
+  if (capacity.value() <= kLinearThreshold) {
     do {
-      size += ATTRCHILD_ARRAY_GROWSIZE;
-      if (!size.isValid()) {
+      capacity += kLinearGrowSize;
+      if (!capacity.isValid()) {
         return false;
       }
-    } while (size.value() < minSize.value());
-  }
-  else {
-    uint32_t shift = mozilla::CeilingLog2(minSize.value());
+    } while (capacity.value() < minCapacity.value());
+  } else {
+    uint32_t shift = mozilla::CeilingLog2(minCapacity.value());
     if (shift >= 32) {
       return false;
     }
-
-    size = 1u << shift;
+    capacity = 1u << shift;
   }
 
-  bool needToInitialize = !mImpl;
-  CheckedUint32 neededSize = size;
-  neededSize *= sizeof(void*);
-  if (!neededSize.isValid()) {
+  CheckedUint32 sizeInBytes = capacity.value();
+  sizeInBytes *= sizeof(InternalAttr);
+  if (!sizeInBytes.isValid()) {
     return false;
   }
 
-  Impl* newImpl = static_cast<Impl*>(realloc(mImpl, neededSize.value()));
+  sizeInBytes += sizeof(Impl);
+  if (!sizeInBytes.isValid()) {
+    return false;
+  }
+
+  MOZ_ASSERT(sizeInBytes.value() ==
+               Impl::AllocationSizeForAttributes(capacity.value()));
+
+  const bool needToInitialize = !mImpl;
+  Impl* newImpl = static_cast<Impl*>(realloc(mImpl.release(), sizeInBytes.value()));
   NS_ENSURE_TRUE(newImpl, false);
 
-  mImpl = newImpl;
+  mImpl.reset(newImpl);
 
   // Set initial counts if we didn't have a buffer before
   if (needToInitialize) {
     mImpl->mMappedAttrs = nullptr;
-    SetAttrSlotCount(0);
+    mImpl->mAttrCount = 0;
   }
 
-  mImpl->mBufferSize = size.value() - NS_IMPL_EXTRA_SIZE;
-
-  return true;
-}
-
-bool
-AttrArray::AddAttrSlot()
-{
-  uint32_t slotCount = AttrSlotCount();
-
-  CheckedUint32 size = slotCount;
-  size += 1;
-  size *= ATTRSIZE;
-  if (!size.isValid()) {
-    return false;
-  }
-
-  // Grow buffer if needed
-  if (!(mImpl && mImpl->mBufferSize >= size.value()) &&
-      !GrowBy(ATTRSIZE)) {
-    return false;
-  }
-
-  void** offset = mImpl->mBuffer + slotCount * ATTRSIZE;
-
-  SetAttrSlotCount(slotCount + 1);
-  memset(static_cast<void*>(offset), 0, sizeof(InternalAttr));
-
+  mImpl->mCapacity = capacity.value();
   return true;
 }
 
 size_t
 AttrArray::SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
 {
   size_t n = 0;
   if (mImpl) {
     // Don't add the size taken by *mMappedAttrs because it's shared.
 
-    n += aMallocSizeOf(mImpl);
+    n += aMallocSizeOf(mImpl.get());
 
-    uint32_t slotCount = AttrSlotCount();
-    for (uint32_t i = 0; i < slotCount && AttrSlotIsTaken(i); ++i) {
-      nsAttrValue* value = &ATTRS(mImpl)[i].mValue;
-      n += value->SizeOfExcludingThis(aMallocSizeOf);
+    for (const InternalAttr& attr : NonMappedAttrs()) {
+      n += attr.mValue.SizeOfExcludingThis(aMallocSizeOf);
     }
   }
 
   return n;
 }
--- a/dom/base/AttrArray.h
+++ b/dom/base/AttrArray.h
@@ -8,48 +8,49 @@
  * Storage of the attributes of a DOM node.
  */
 
 #ifndef AttrArray_h___
 #define AttrArray_h___
 
 #include "mozilla/Attributes.h"
 #include "mozilla/MemoryReporting.h"
+#include "mozilla/UniquePtr.h"
+#include "mozilla/Span.h"
 #include "mozilla/dom/BorrowedAttrInfo.h"
 
 #include "nscore.h"
 #include "nsAttrName.h"
 #include "nsAttrValue.h"
 #include "nsCaseTreatment.h"
 
 class nsINode;
 class nsIContent;
 class nsMappedAttributes;
 class nsHTMLStyleSheet;
 class nsRuleWalker;
 class nsMappedAttributeElement;
 
-#define ATTRCHILD_ARRAY_GROWSIZE 8
-#define ATTRCHILD_ARRAY_LINEAR_THRESHOLD 32
-
-#define ATTRSIZE (sizeof(InternalAttr) / sizeof(void*))
-
 class AttrArray
 {
   typedef mozilla::dom::BorrowedAttrInfo BorrowedAttrInfo;
 public:
-  AttrArray();
-  ~AttrArray();
+  AttrArray() = default;
+  ~AttrArray() = default;
 
   bool HasAttrs() const
   {
-    return MappedAttrCount() || (AttrSlotCount() && AttrSlotIsTaken(0));
+    return NonMappedAttrCount() || MappedAttrCount();
   }
 
-  uint32_t AttrCount() const;
+  uint32_t AttrCount() const
+  {
+    return NonMappedAttrCount() + MappedAttrCount();
+  }
+
   const nsAttrValue* GetAttr(nsAtom* aLocalName,
                              int32_t aNamespaceID = kNameSpaceID_None) const;
   // As above but using a string attr name and always using
   // kNameSpaceID_None.  This is always case-sensitive.
   const nsAttrValue* GetAttr(const nsAString& aName) const;
   // Get an nsAttrValue by qualified name.  Can optionally do
   // ASCII-case-insensitive name matching.
   const nsAttrValue* GetAttr(const nsAString& aName,
@@ -125,52 +126,49 @@ public:
   // Increases capacity (if necessary) to have enough space to accomodate the
   // unmapped attributes of |aOther|.
   nsresult EnsureCapacityToClone(const AttrArray& aOther);
 
 private:
   AttrArray(const AttrArray& aOther) = delete;
   AttrArray& operator=(const AttrArray& aOther) = delete;
 
-  void Clear();
+  uint32_t NonMappedAttrCount() const
+  {
+    return mImpl ? mImpl->mAttrCount : 0;
+  }
 
-  uint32_t NonMappedAttrCount() const;
-  uint32_t MappedAttrCount() const;
+  uint32_t MappedAttrCount() const
+  {
+    return mImpl && mImpl->mMappedAttrs ? DoGetMappedAttrCount() : 0;
+  }
+
+  uint32_t DoGetMappedAttrCount() const;
 
   // Returns a non-null zero-refcount object.
   nsMappedAttributes*
   GetModifiableMapped(nsMappedAttributeElement* aContent,
                       nsHTMLStyleSheet* aSheet,
                       bool aWillAddAttr,
                       int32_t aAttrCount = 1);
   nsresult MakeMappedUnique(nsMappedAttributes* aAttributes);
 
-  uint32_t AttrSlotsSize() const
-  {
-    return AttrSlotCount() * ATTRSIZE;
-  }
+  bool GrowBy(uint32_t aGrowSize);
 
-  uint32_t AttrSlotCount() const
-  {
-    return mImpl ? mImpl->mAttrCount : 0;
-  }
+  struct InternalAttr;
 
-  bool AttrSlotIsTaken(uint32_t aSlot) const
-  {
-    MOZ_ASSERT(aSlot < AttrSlotCount(), "out-of-bounds");
-    return mImpl->mBuffer[aSlot * ATTRSIZE];
-  }
-
-  void SetAttrSlotCount(uint32_t aCount)
-  {
-    mImpl->mAttrCount = aCount;
-  }
-
-  bool GrowBy(uint32_t aGrowSize);
-  bool AddAttrSlot();
+  // Tries to create an attribute, growing the buffer if needed, with the given
+  // name and value.
+  //
+  // The value is moved from the argument.
+  //
+  // `Name` can be anything you construct a `nsAttrName` with (either an atom or
+  // a NodeInfo pointer).
+  template<typename Name>
+  nsresult AddNewAttribute(Name*, nsAttrValue&);
 
   /**
    * Guts of SetMappedAttrStyleSheet for the rare case when we have mapped attrs
    */
   nsresult DoSetMappedAttrStyleSheet(nsHTMLStyleSheet* aSheet);
 
   /**
    * Guts of UpdateMappedAttrRuleMapper for the case  when we have mapped attrs.
@@ -178,19 +176,55 @@ private:
   nsresult DoUpdateMappedAttrRuleMapper(nsMappedAttributeElement& aElement);
 
   struct InternalAttr
   {
     nsAttrName mName;
     nsAttrValue mValue;
   };
 
-  struct Impl {
+  class Impl
+  {
+  public:
+
+    constexpr static size_t AllocationSizeForAttributes(uint32_t aAttrCount)
+    {
+      return sizeof(Impl) + aAttrCount * sizeof(InternalAttr);
+    }
+
+    mozilla::Span<const InternalAttr> NonMappedAttrs() const
+    {
+      return mozilla::MakeSpan(static_cast<const InternalAttr*>(mBuffer), mAttrCount);
+    }
+
+    mozilla::Span<InternalAttr> NonMappedAttrs()
+    {
+      return mozilla::MakeSpan(static_cast<InternalAttr*>(mBuffer), mAttrCount);
+    }
+
+    Impl(const Impl&) = delete;
+    Impl(Impl&&) = delete;
+    ~Impl();
+
     uint32_t mAttrCount;
-    uint32_t mBufferSize;
+    uint32_t mCapacity; // In number of InternalAttrs
+
+    // Manually refcounted.
     nsMappedAttributes* mMappedAttrs;
-    void* mBuffer[1];
+
+    // Allocated in the same buffer as `Impl`.
+    InternalAttr mBuffer[0];
   };
 
-  Impl* mImpl;
+  mozilla::Span<InternalAttr> NonMappedAttrs()
+  {
+    return mImpl ? mImpl->NonMappedAttrs() : mozilla::Span<InternalAttr>();
+  }
+
+  mozilla::Span<const InternalAttr> NonMappedAttrs() const
+  {
+    return mImpl ? mImpl->NonMappedAttrs() : mozilla::Span<const InternalAttr>();
+  }
+
+  mozilla::UniquePtr<Impl> mImpl;
 };
 
 #endif