Changeset 243037 in webkit
- Timestamp:
- Mar 16, 2019 12:50:24 AM (5 years ago)
- Location:
- trunk/Source/WebCore
- Files:
-
- 5 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebCore/ChangeLog
r243036 r243037 1 2019-03-16 Ryosuke Niwa <rniwa@webkit.org> 2 3 Reduce the size of Node::deref by eliminating an explicit parentNode check 4 https://bugs.webkit.org/show_bug.cgi?id=195776 5 6 Reviewed by Geoffrey Garen. 7 8 This patch eliminates the nullity check of m_parentNode in Node::deref as well as the store to 9 m_refCount in the case of invoking Node::removedLastRef() as done for RefCounted in r30042. 10 Together, this patch shrinks WebCore's size by 46KB or ~0.7%. 11 12 To do this, we take we take a similar approach as WTF::String by using the lowest bit of m_refCount 13 to indicate whether a node has a parent or not. Regular ref-counting is done on the upper 31 bits. 14 Node::setParentNode updates this flag, and Node::deref() would only `delete this` if m_refCount 15 is identically equal to 0. 16 17 For a Document, we set m_refCounted to 0 before in the case of non-zero m_referencingNodeCount 18 since decrementReferencingNodeCount needs to be able to tell if there is an outstanding Ref/RefPtr 19 or not when m_referencingNodeCount becomes 0. 20 21 No new tests since there should be no behavioral change. 22 23 * dom/Document.cpp: 24 (WebCore::Document::removedLastRef): 25 * dom/Document.h: 26 (WebCore::Document::decrementReferencingNodeCount): 27 * dom/Node.cpp: 28 (WebCore::Node::Node): Moved the initialization of m_refCount to the member variable declaration. 29 (WebCore::Node::~Node): 30 (WebCore::Node::removedLastRef): 31 * dom/Node.h: 32 (WebCore::Node): Changed the type of m_refCount from signed int to uint32_t. It was changed from 33 unsigned int to signed int back in r11492 but I don't think the signedness is needed. 34 (WebCore::Node::ref): Increment the ref count by 2 (upper 31-bit). 35 (WebCore::Node::deref): Implemented the optimization. This is what shrinks the WebCore binary size. 36 (WebCore::Node::hasOneRef const): 37 (WebCore::Node::refCount const): Ignore the lowest bit. Without this fix, the optimization in 38 replaceChildrenWithFragment to avoid replacing the text node is disabled whenever there is a parent. 39 (WebCore::Node::setParentNode): Sets the lowest bit to 1 if the node has a parent and 0 otherwise. 40 1 41 2019-03-16 Said Abou-Hallawa <sabouhallawa@apple.com> 2 42 -
trunk/Source/WebCore/dom/Document.cpp
r243035 r243037 670 670 ASSERT(!m_deletionHasBegun); 671 671 if (m_referencingNodeCount) { 672 // Node::removedLastRef doesn't set refCount() to zero because it's not observable. 673 // But we need to remember that our refCount reached zero in subsequent calls to decrementReferencingNodeCount() 674 m_refCountAndParentBit = 0; 675 672 676 // If removing a child removes the last node reference, we don't want the scope to be destroyed 673 677 // until after removeDetachedChildren returns, so we protect ourselves. … … 719 723 m_deletionHasBegun = true; 720 724 #endif 721 m_refCount = 1; // Avoid double destruction through use of RefPtr<T>. (This is a security mitigation in case of programmer error. It will ASSERT in debug builds.)722 725 delete this; 723 726 } -
trunk/Source/WebCore/dom/Document.h
r242964 r243037 377 377 m_deletionHasBegun = true; 378 378 #endif 379 m_refCount = 1; // Avoid double destruction through use ofRefPtr<T>. (This is a security mitigation in case of programmer error. It will ASSERT in debug builds.)379 m_refCountAndParentBit = s_refCountIncrement; // Avoid double destruction through use of Ref<T>/RefPtr<T>. (This is a security mitigation in case of programmer error. It will ASSERT in debug builds.) 380 380 delete this; 381 381 } -
trunk/Source/WebCore/dom/Node.cpp
r242964 r243037 317 317 318 318 Node::Node(Document& document, ConstructionType type) 319 : m_refCount(1) 320 , m_nodeFlags(type) 319 : m_nodeFlags(type) 321 320 , m_treeScope(&document) 322 321 { … … 333 332 { 334 333 ASSERT(isMainThread()); 335 // We set m_refCount to 1before calling delete to avoid double destruction through use of Ref<T>/RefPtr<T>.334 // We set m_refCount to 2 before calling delete to avoid double destruction through use of Ref<T>/RefPtr<T>. 336 335 // This is a security mitigation in case of programmer errorm (caught by a debug assertion). 337 ASSERT(m_refCount == 1);336 ASSERT(m_refCountAndParentBit == s_refCountIncrement); 338 337 ASSERT(m_deletionHasBegun); 339 338 ASSERT(!m_adoptionIsRequired); … … 2524 2523 void Node::removedLastRef() 2525 2524 { 2525 // This avoids double destruction even when there is a programming error to use Ref<T> / RefPtr<T> on this node. 2526 // There are debug assertions in Node::ref() / Node::deref() to catch such a programming error. 2527 ASSERT(m_refCountAndParentBit == s_refCountIncrement); 2528 2526 2529 // An explicit check for Document here is better than a virtual function since it is 2527 2530 // faster for non-Document nodes, and because the call to removedLastRef that is inlined … … 2535 2538 m_deletionHasBegun = true; 2536 2539 #endif 2537 m_refCount = 1; // Avoid double destruction through use of RefPtr<T>. (This is a security mitigation in case of programmer error. It will ASSERT in debug builds.)2538 2540 delete this; 2539 2541 } -
trunk/Source/WebCore/dom/Node.h
r242696 r243037 501 501 void deref(); 502 502 bool hasOneRef() const; 503 intrefCount() const;503 unsigned refCount() const; 504 504 505 505 #ifndef NDEBUG … … 619 619 Node(Document&, ConstructionType); 620 620 621 static constexpr uint32_t s_refCountIncrement = 2; 622 static constexpr uint32_t s_refCountMask = ~0x1; 623 621 624 virtual void addSubresourceAttributeURLs(ListHashSet<URL>&) const { } 622 625 … … 665 668 void moveNodeToNewDocument(Document& oldDocument, Document& newDocument); 666 669 667 int m_refCount;670 uint32_t m_refCountAndParentBit { s_refCountIncrement }; 668 671 mutable uint32_t m_nodeFlags; 669 672 … … 696 699 ASSERT(!m_inRemovedLastRefFunction); 697 700 ASSERT(!m_adoptionIsRequired); 698 ++m_refCount;701 m_refCountAndParentBit += s_refCountIncrement; 699 702 } 700 703 … … 702 705 { 703 706 ASSERT(isMainThread()); 704 ASSERT( m_refCount >= 0);707 ASSERT(refCount()); 705 708 ASSERT(!m_deletionHasBegun); 706 709 ASSERT(!m_inRemovedLastRefFunction); 707 710 ASSERT(!m_adoptionIsRequired); 708 if (--m_refCount <= 0 && !parentNode()) { 711 auto tempRefCount = m_refCountAndParentBit - s_refCountIncrement; 712 if (!tempRefCount) { 709 713 #ifndef NDEBUG 710 714 m_inRemovedLastRefFunction = true; 711 715 #endif 712 716 removedLastRef(); 717 return; 713 718 } 719 m_refCountAndParentBit = tempRefCount; 714 720 } 715 721 … … 718 724 ASSERT(!m_deletionHasBegun); 719 725 ASSERT(!m_inRemovedLastRefFunction); 720 return m_refCount== 1;721 } 722 723 ALWAYS_INLINE intNode::refCount() const724 { 725 return m_refCount ;726 return refCount() == 1; 727 } 728 729 ALWAYS_INLINE unsigned Node::refCount() const 730 { 731 return m_refCountAndParentBit / s_refCountIncrement; 726 732 } 727 733 … … 737 743 ASSERT(isMainThread()); 738 744 m_parentNode = parent; 745 auto refCountWithoutParentBit = m_refCountAndParentBit & s_refCountMask; 746 m_refCountAndParentBit = refCountWithoutParentBit | !!parent; 739 747 } 740 748
Note: See TracChangeset
for help on using the changeset viewer.