From 2e00f64ed1b987aa3ddfbec51180f27c38b41787 Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Sat, 30 Jun 2018 01:30:37 +0300 Subject: [PATCH] Bug 1419661, if ExtendedDOMSlots are used before slots, use FatSlots to have fewer allocations, r=mrbkap --- dom/base/FragmentOrElement.cpp | 19 +++++---- dom/base/FragmentOrElement.h | 48 +++++++++++++++++---- dom/base/nsIContent.h | 55 ++++++++++++++++++++----- servo/components/style/gecko/wrapper.rs | 5 ++- 4 files changed, 101 insertions(+), 26 deletions(-) diff --git a/dom/base/FragmentOrElement.cpp b/dom/base/FragmentOrElement.cpp index 05ebbea420df..39d9e0406552 100644 --- a/dom/base/FragmentOrElement.cpp +++ b/dom/base/FragmentOrElement.cpp @@ -645,7 +645,7 @@ static_assert(sizeof(FragmentOrElement::nsDOMSlots) <= MaxDOMSlotSizeAllowed, "DOM slots cannot be grown without consideration"); void -nsIContent::nsExtendedContentSlots::Unlink() +nsIContent::nsExtendedContentSlots::UnlinkExtendedSlots() { mBindingParent = nullptr; mXBLInsertionPoint = nullptr; @@ -654,7 +654,7 @@ nsIContent::nsExtendedContentSlots::Unlink() } void -nsIContent::nsExtendedContentSlots::Traverse(nsCycleCollectionTraversalCallback& aCb) +nsIContent::nsExtendedContentSlots::TraverseExtendedSlots(nsCycleCollectionTraversalCallback& aCb) { NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(aCb, "mExtendedSlots->mBindingParent"); aCb.NoteXPCOMChild(NS_ISUPPORTS_CAST(nsIContent*, mBindingParent)); @@ -679,10 +679,13 @@ FragmentOrElement::nsDOMSlots::nsDOMSlots() : nsIContent::nsContentSlots(), mDataset(nullptr) { + MOZ_COUNT_CTOR(nsDOMSlots); } FragmentOrElement::nsDOMSlots::~nsDOMSlots() { + MOZ_COUNT_DTOR(nsDOMSlots); + if (mAttributeMap) { mAttributeMap->DropReference(); } @@ -723,8 +726,8 @@ size_t FragmentOrElement::nsDOMSlots::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const { size_t n = aMallocSizeOf(this); - if (mExtendedSlots) { - n += aMallocSizeOf(mExtendedSlots.get()); + if (OwnsExtendedSlots()) { + n += aMallocSizeOf(GetExtendedContentSlots()); } if (mAttributeMap) { @@ -758,9 +761,9 @@ FragmentOrElement::nsExtendedDOMSlots::~nsExtendedDOMSlots() } void -FragmentOrElement::nsExtendedDOMSlots::Unlink() +FragmentOrElement::nsExtendedDOMSlots::UnlinkExtendedSlots() { - nsIContent::nsExtendedContentSlots::Unlink(); + nsIContent::nsExtendedContentSlots::UnlinkExtendedSlots(); // Don't clear mXBLBinding, it'll be done in // BindingManager::RemovedFromDocument from FragmentOrElement::Unlink. @@ -780,9 +783,9 @@ FragmentOrElement::nsExtendedDOMSlots::Unlink() } void -FragmentOrElement::nsExtendedDOMSlots::Traverse(nsCycleCollectionTraversalCallback& aCb) +FragmentOrElement::nsExtendedDOMSlots::TraverseExtendedSlots(nsCycleCollectionTraversalCallback& aCb) { - nsIContent::nsExtendedContentSlots::Traverse(aCb); + nsIContent::nsExtendedContentSlots::TraverseExtendedSlots(aCb); NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(aCb, "mExtendedSlots->mSMILOverrideStyle"); aCb.NoteXPCOMChild(mSMILOverrideStyle.get()); diff --git a/dom/base/FragmentOrElement.h b/dom/base/FragmentOrElement.h index 7e4beeab2e10..2460122dc7d4 100644 --- a/dom/base/FragmentOrElement.h +++ b/dom/base/FragmentOrElement.h @@ -171,14 +171,14 @@ public: * accessed through the DOM. */ - class nsExtendedDOMSlots final : public nsIContent::nsExtendedContentSlots + class nsExtendedDOMSlots : public nsIContent::nsExtendedContentSlots { public: nsExtendedDOMSlots(); - ~nsExtendedDOMSlots() final; + ~nsExtendedDOMSlots(); - void Traverse(nsCycleCollectionTraversalCallback&) final; - void Unlink() final; + void TraverseExtendedSlots(nsCycleCollectionTraversalCallback&) final; + void UnlinkExtendedSlots() final; /** * SMIL Overridde style rules (for SMIL animation of CSS properties) @@ -223,11 +223,11 @@ public: }; - class nsDOMSlots final : public nsIContent::nsContentSlots + class nsDOMSlots : public nsIContent::nsContentSlots { public: nsDOMSlots(); - ~nsDOMSlots() final; + ~nsDOMSlots(); void Traverse(nsCycleCollectionTraversalCallback&) final; void Unlink() final; @@ -263,6 +263,29 @@ public: RefPtr mClassList; }; + /** + * In case ExtendedDOMSlots is needed before normal DOMSlots, an instance of + * FatSlots class, which combines those two slot types, is created. + * This way we can avoid extra allocation for ExtendedDOMSlots. + * FatSlots is useful for example when creating Custom Elements. + */ + class FatSlots final : public nsDOMSlots, public nsExtendedDOMSlots + { + public: + FatSlots() + : nsDOMSlots() + , nsExtendedDOMSlots() + { + MOZ_COUNT_CTOR(FatSlots); + SetExtendedContentSlots(this, false); + } + + ~FatSlots() final + { + MOZ_COUNT_DTOR(FatSlots); + } + }; + protected: void GetMarkup(bool aIncludeSelf, nsAString& aMarkup); void SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult& aError); @@ -290,7 +313,18 @@ protected: nsExtendedDOMSlots* ExtendedDOMSlots() { - return static_cast(ExtendedContentSlots()); + nsContentSlots* slots = GetExistingContentSlots(); + if (!slots) { + FatSlots* fatSlots = new FatSlots(); + mSlots = fatSlots; + return fatSlots; + } + + if (!slots->GetExtendedContentSlots()) { + slots->SetExtendedContentSlots(CreateExtendedSlots(), true); + } + + return static_cast(slots->GetExtendedContentSlots()); } const nsExtendedDOMSlots* GetExistingExtendedDOMSlots() const diff --git a/dom/base/nsIContent.h b/dom/base/nsIContent.h index cf821ee452bd..66dfb58db555 100644 --- a/dom/base/nsIContent.h +++ b/dom/base/nsIContent.h @@ -791,8 +791,8 @@ protected: nsExtendedContentSlots(); virtual ~nsExtendedContentSlots(); - virtual void Traverse(nsCycleCollectionTraversalCallback&); - virtual void Unlink(); + virtual void TraverseExtendedSlots(nsCycleCollectionTraversalCallback&); + virtual void UnlinkExtendedSlots(); /** * The nearest enclosing content node with a binding that created us. @@ -821,11 +821,24 @@ protected: class nsContentSlots : public nsINode::nsSlots { public: + nsContentSlots() + : nsINode::nsSlots() + , mExtendedSlots(0) + { + } + + ~nsContentSlots() + { + if (!(mExtendedSlots & sNonOwningExtendedSlotsFlag)) { + delete GetExtendedContentSlots(); + } + } + void Traverse(nsCycleCollectionTraversalCallback& aCb) override { nsINode::nsSlots::Traverse(aCb); if (mExtendedSlots) { - mExtendedSlots->Traverse(aCb); + GetExtendedContentSlots()->TraverseExtendedSlots(aCb); } } @@ -833,11 +846,33 @@ protected: { nsINode::nsSlots::Unlink(); if (mExtendedSlots) { - mExtendedSlots->Unlink(); + GetExtendedContentSlots()->UnlinkExtendedSlots(); } } - mozilla::UniquePtr mExtendedSlots; + void SetExtendedContentSlots(nsExtendedContentSlots* aSlots, bool aOwning) + { + mExtendedSlots = reinterpret_cast(aSlots); + if (!aOwning) { + mExtendedSlots |= sNonOwningExtendedSlotsFlag; + } + } + + bool OwnsExtendedSlots() const + { + return !(mExtendedSlots & sNonOwningExtendedSlotsFlag); + } + + nsExtendedContentSlots* GetExtendedContentSlots() const + { + return reinterpret_cast( + mExtendedSlots & ~sNonOwningExtendedSlotsFlag); + } + + private: + static const uintptr_t sNonOwningExtendedSlotsFlag = 1u; + + uintptr_t mExtendedSlots; }; // Override from nsINode @@ -869,22 +904,22 @@ protected: const nsExtendedContentSlots* GetExistingExtendedContentSlots() const { const nsContentSlots* slots = GetExistingContentSlots(); - return slots ? slots->mExtendedSlots.get() : nullptr; + return slots ? slots->GetExtendedContentSlots() : nullptr; } nsExtendedContentSlots* GetExistingExtendedContentSlots() { nsContentSlots* slots = GetExistingContentSlots(); - return slots ? slots->mExtendedSlots.get() : nullptr; + return slots ? slots->GetExtendedContentSlots() : nullptr; } nsExtendedContentSlots* ExtendedContentSlots() { nsContentSlots* slots = ContentSlots(); - if (!slots->mExtendedSlots) { - slots->mExtendedSlots.reset(CreateExtendedSlots()); + if (!slots->GetExtendedContentSlots()) { + slots->SetExtendedContentSlots(CreateExtendedSlots(), true); } - return slots->mExtendedSlots.get(); + return slots->GetExtendedContentSlots(); } /** diff --git a/servo/components/style/gecko/wrapper.rs b/servo/components/style/gecko/wrapper.rs index 47205f7378bb..ee0c134b269c 100644 --- a/servo/components/style/gecko/wrapper.rs +++ b/servo/components/style/gecko/wrapper.rs @@ -651,7 +651,10 @@ impl<'le> GeckoElement<'le> { #[inline] fn extended_slots(&self) -> Option<&structs::FragmentOrElement_nsExtendedDOMSlots> { self.dom_slots().and_then(|s| unsafe { - (s._base.mExtendedSlots.mPtr as *const structs::FragmentOrElement_nsExtendedDOMSlots) + // For the bit usage, see nsContentSlots::GetExtendedSlots. + let e_slots = s._base.mExtendedSlots & + !structs::nsIContent_nsContentSlots_sNonOwningExtendedSlotsFlag; + (e_slots as *const structs::FragmentOrElement_nsExtendedDOMSlots) .as_ref() }) }