Changeset 254634 in webkit

Timestamp:

Jan 15, 2020 1:49:37 PM (4 years ago)

Author:

Antti Koivisto

Message:

[LFC] Cache display box for the first LayoutState to Layout::Box
​https://bugs.webkit.org/show_bug.cgi?id=206288

Reviewed by Zalan Bujtas.

Add a single item cache for the common case to avoid using the hash.

• layout/FormattingState.cpp:

(WebCore::Layout::FormattingState::displayBox):

• layout/LayoutState.cpp:

(WebCore::Layout::LayoutState::displayBoxForRootLayoutBox):
(WebCore::Layout::LayoutState::ensureDisplayBoxForLayoutBoxSlow):
(WebCore::Layout::LayoutState::displayBoxForLayoutBox): Deleted.
(WebCore::Layout::LayoutState::displayBoxForLayoutBox const): Deleted.

• layout/LayoutState.h:

(WebCore::Layout::Box::cachedDisplayBoxForLayoutState const):
(WebCore::Layout::LayoutState::hasDisplayBox const):
(WebCore::Layout::LayoutState::ensureDisplayBoxForLayoutBox):
(WebCore::Layout::LayoutState::displayBoxForLayoutBox const):

• layout/layouttree/LayoutBox.cpp:

(WebCore::Layout::Box::setCachedDisplayBoxForLayoutState const):

• layout/layouttree/LayoutBox.h:

(WebCore::Layout::Box::hasCachedDisplayBox const):

• layout/layouttree/LayoutTreeBuilder.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• layout/FormattingState.cpp (modified) (1 diff)
• layout/LayoutState.cpp (modified) (1 diff)
• layout/LayoutState.h (modified) (5 diffs)
• layout/layouttree/LayoutBox.cpp (modified) (2 diffs)
• layout/layouttree/LayoutBox.h (modified) (3 diffs)
• layout/layouttree/LayoutTreeBuilder.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-01-15  Antti Koivisto  <antti@apple.com>
+
+        [LFC] Cache display box for the first LayoutState to Layout::Box
+        https://bugs.webkit.org/show_bug.cgi?id=206288
+
+        Reviewed by Zalan Bujtas.
+
+        Add a single item cache for the common case to avoid using the hash.
+
+        * layout/FormattingState.cpp:
+        (WebCore::Layout::FormattingState::displayBox):
+        * layout/LayoutState.cpp:
+        (WebCore::Layout::LayoutState::displayBoxForRootLayoutBox):
+        (WebCore::Layout::LayoutState::ensureDisplayBoxForLayoutBoxSlow):
+        (WebCore::Layout::LayoutState::displayBoxForLayoutBox): Deleted.
+        (WebCore::Layout::LayoutState::displayBoxForLayoutBox const): Deleted.
+        * layout/LayoutState.h:
+        (WebCore::Layout::Box::cachedDisplayBoxForLayoutState const):
+        (WebCore::Layout::LayoutState::hasDisplayBox const):
+        (WebCore::Layout::LayoutState::ensureDisplayBoxForLayoutBox):
+        (WebCore::Layout::LayoutState::displayBoxForLayoutBox const):
+        * layout/layouttree/LayoutBox.cpp:
+        (WebCore::Layout::Box::setCachedDisplayBoxForLayoutState const):
+        * layout/layouttree/LayoutBox.h:
+        (WebCore::Layout::Box::hasCachedDisplayBox const):
+        * layout/layouttree/LayoutTreeBuilder.h:
+
 2020-01-15  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/layout/FormattingState.cpp

@@ -52 +52 @@
     // Should never need to mutate a display box outside of the formatting context.
     ASSERT(&layoutState().establishedFormattingState(layoutBox.formattingContextRoot()) == this);
-    return layoutState().displayBoxForLayoutBox(layoutBox);
+    return layoutState().ensureDisplayBoxForLayoutBox(layoutBox);
 }
 

trunk/Source/WebCore/layout/LayoutState.cpp

@@ -61 +61 @@
 Display::Box& LayoutState::displayBoxForRootLayoutBox()
 {
-    return displayBoxForLayoutBox(m_layoutTreeContent->rootLayoutBox());
+    return ensureDisplayBoxForLayoutBox(m_layoutTreeContent->rootLayoutBox());
 }
 
-Display::Box& LayoutState::displayBoxForLayoutBox(const Box& layoutBox)
+Display::Box& LayoutState::ensureDisplayBoxForLayoutBoxSlow(const Box& layoutBox)
 {
+    if (layoutBox.canCacheForLayoutState(*this)) {
+        ASSERT(!layoutBox.cachedDisplayBoxForLayoutState(*this));
+        auto newBox = makeUnique<Display::Box>();
+        auto& newBoxPtr = *newBox;
+        layoutBox.setCachedDisplayBoxForLayoutState(*this, WTFMove(newBox));
+        return newBoxPtr;
+    }
+
     return *m_layoutToDisplayBox.ensure(&layoutBox, [] {
         return makeUnique<Display::Box>();
     }).iterator->value;
-}
-
-const Display::Box& LayoutState::displayBoxForLayoutBox(const Box& layoutBox) const
-{
-    ASSERT(hasDisplayBox(layoutBox));
-    return *m_layoutToDisplayBox.get(&layoutBox);
 }
 

trunk/Source/WebCore/layout/LayoutState.h

@@ -43 +43 @@
 namespace Layout {
 
-class Box;
 class FormattingContext;
 class FormattingState;
 
-class LayoutState {
+class LayoutState : public CanMakeWeakPtr<LayoutState> {
     WTF_MAKE_ISO_ALLOCATED(LayoutState);
 public:
@@ -64 +63 @@
 
     Display::Box& displayBoxForRootLayoutBox();
-    Display::Box& displayBoxForLayoutBox(const Box& layoutBox);
-    const Display::Box& displayBoxForLayoutBox(const Box& layoutBox) const;
-    bool hasDisplayBox(const Box& layoutBox) const { return m_layoutToDisplayBox.contains(&layoutBox); }
+    Display::Box& ensureDisplayBoxForLayoutBox(const Box&);
+    const Display::Box& displayBoxForLayoutBox(const Box&) const;
+
+    bool hasDisplayBox(const Box&) const;
 
     enum class QuirksMode { No, Limited, Yes };
@@ -85 +85 @@
 private:
     void setQuirksMode(QuirksMode quirksMode) { m_quirksMode = quirksMode; }
+    Display::Box& ensureDisplayBoxForLayoutBoxSlow(const Box&);
 
     HashMap<const Container*, std::unique_ptr<FormattingState>> m_formattingStates;
@@ -98 +99 @@
 };
 
+inline bool LayoutState::hasDisplayBox(const Box& layoutBox) const
+{
+    if (layoutBox.cachedDisplayBoxForLayoutState(*this))
+        return true;
+    return m_layoutToDisplayBox.contains(&layoutBox);
+}
+
+inline Display::Box& LayoutState::ensureDisplayBoxForLayoutBox(const Box& layoutBox)
+{
+    if (auto* displayBox = layoutBox.cachedDisplayBoxForLayoutState(*this))
+        return *displayBox;
+    return ensureDisplayBoxForLayoutBoxSlow(layoutBox);
+}
+
+inline const Display::Box& LayoutState::displayBoxForLayoutBox(const Box& layoutBox) const
+{
+    if (auto* displayBox = layoutBox.cachedDisplayBoxForLayoutState(*this))
+        return *displayBox;
+    ASSERT(m_layoutToDisplayBox.contains(&layoutBox));
+    return *m_layoutToDisplayBox.get(&layoutBox);
+}
+
 #ifndef NDEBUG
 inline void LayoutState::registerFormattingContext(const FormattingContext& formattingContext)
@@ -107 +130 @@
 #endif
 
+// These Layout::Box function are here to allow inlining.
+inline bool Box::canCacheForLayoutState(const LayoutState& layoutState) const
+{
+    return !m_cachedLayoutState || m_cachedLayoutState.get() == &layoutState;
+}
+
+inline Display::Box* Box::cachedDisplayBoxForLayoutState(const LayoutState& layoutState) const
+{
+    if (m_cachedLayoutState.get() != &layoutState)
+        return nullptr;
+    return m_cachedDisplayBoxForLayoutState.get();
+}
+
 }
 }

trunk/Source/WebCore/layout/layouttree/LayoutBox.cpp

@@ -31 +31 @@
 #include "LayoutContainer.h"
 #include "LayoutPhase.h"
+#include "LayoutState.h"
 #include "RenderStyle.h"
 #include <wtf/IsoMallocInlines.h>
@@ -445 +446 @@
 }
 
+void Box::setCachedDisplayBoxForLayoutState(LayoutState& layoutState, std::unique_ptr<Display::Box> box) const
+{
+    ASSERT(!m_cachedLayoutState);
+    m_cachedLayoutState = makeWeakPtr(layoutState);
+    m_cachedDisplayBoxForLayoutState = WTFMove(box);
+}
+
 Box::RareDataMap& Box::rareDataMap()
 {

trunk/Source/WebCore/layout/layouttree/LayoutBox.h

@@ -35 +35 @@
 
 namespace WebCore {
+
+namespace Display {
+class Box;
+}
+
 namespace Layout {
 
 class Container;
+class LayoutState;
 class TreeBuilder;
 
@@ -168 +174 @@
     void setIsAnonymous() { m_isAnonymous = true; }
 
+    bool canCacheForLayoutState(const LayoutState&) const;
+    Display::Box* cachedDisplayBoxForLayoutState(const LayoutState&) const;
+    void setCachedDisplayBoxForLayoutState(LayoutState&, std::unique_ptr<Display::Box>) const;
+
 protected:
     Box(Optional<ElementAttributes>, Optional<TextContext>, RenderStyle&&, BaseTypeFlags);
@@ -202 +212 @@
     const Optional<TextContext> m_textContext;
 
+    // First LayoutState gets a direct cache.
+    mutable WeakPtr<LayoutState> m_cachedLayoutState;
+    mutable std::unique_ptr<Display::Box> m_cachedDisplayBoxForLayoutState;
+
     unsigned m_baseTypeFlags : 6;
     bool m_hasRareData : 1;

trunk/Source/WebCore/layout/layouttree/LayoutTreeBuilder.h

@@ -28 +28 @@
 #if ENABLE(LAYOUT_FORMATTING_CONTEXT)
 
-#include "LayoutBox.h"
 #include <wtf/IsoMalloc.h>
 #include <wtf/WeakPtr.h>