Attachment 369929 Details for Bug 197905 – Patch

[patch] Patch

bug-197905-20190514231844.patch (text/plain), 8.83 KB, created by Simon Fraser (smfr) on 2019-05-14 23:18:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-05-14 23:18:44 PDT

Size: 8.83 KB

patch

obsolete

>Subversion Revision: 245221
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 3742b1866829cad2450ad37eb18fbef56b9b2bfc..a07c2f7c66014ed67080a6613228269d4ede2b68 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-05-14  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Make LOG_WITH_STREAM more efficient
>+        https://bugs.webkit.org/show_bug.cgi?id=197905
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a streamable repeat() class that can be used to output a series of characters.
>+        This is useful for indenting output.
>+
>+        * wtf/text/TextStream.h:
>+        (WTF::TextStream::repeat::repeat):
>+        (WTF::TextStream::operator<<):
>+
> 2019-05-12  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] Compress Watchpoint size by using enum type and Packed<> data structure
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e8d144433f1e105561f6d4832511776d9c5f2759..605781f22066124bb170d3de6613712b3ac76590 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-05-14  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Make LOG_WITH_STREAM more efficient
>+        https://bugs.webkit.org/show_bug.cgi?id=197905
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No longer need to conditionalize ClipRects logging on the channel being enabled
>+        since LOG_WITH_STREAM fix the performance problem.
>+
>+        Convert some RenderLayerCompositor logging to use LOG_WITH_STREAM.
>+
>+        * rendering/RenderLayer.cpp:
>+        (WebCore::RenderLayer::calculateClipRects const):
>+        (WebCore::clipRectsLogEnabled): Deleted.
>+        * rendering/RenderLayerCompositor.cpp:
>+        (WebCore::RenderLayerCompositor::computeCompositingRequirements):
>+        (WebCore::RenderLayerCompositor::traverseUnchangedSubtree):
>+
> 2019-05-12  Simon Fraser  <simon.fraser@apple.com>
> 
>         When the set of backing-sharing layers changes, we need to issue a repaint
>diff --git a/Source/WebCore/PAL/ChangeLog b/Source/WebCore/PAL/ChangeLog
>index 5e75b257c3c8ff3fce4f1d7c817f1f1bc48bbbe7..d11eb6e00464552a691414aefb8fc261e0fa7be7 100644
>--- a/Source/WebCore/PAL/ChangeLog
>+++ b/Source/WebCore/PAL/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-05-14  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Make LOG_WITH_STREAM more efficient
>+        https://bugs.webkit.org/show_bug.cgi?id=197905
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        Make the LOG_WITH_STREAM macro check that the log channel is enabled before
>+        building the stream.
>+
>+        * pal/LogMacros.h:
>+
> 2019-05-10  Chris Dumez  <cdumez@apple.com>
> 
>         Add WKWebViewConfiguration._canShowWhileLocked SPI
>diff --git a/Source/WTF/wtf/text/TextStream.h b/Source/WTF/wtf/text/TextStream.h
>index 24a370c129b3cee8bda48c34549553746a0844d4..e5ff3136cb757b7fc1eb725d2243eee1e84b2617 100644
>--- a/Source/WTF/wtf/text/TextStream.h
>+++ b/Source/WTF/wtf/text/TextStream.h
>@@ -102,6 +102,24 @@ public:
>         return (*func)(*this);
>     }
> 
>+    struct repeat {
>+        repeat(char inCharacter, unsigned inWidth)
>+            : width(inWidth)
>+            , character(inCharacter)
>+        { }
>+        
>+        unsigned width;
>+        char character;
>+    };
>+
>+    TextStream& operator<<(const repeat& repeated)
>+    {
>+        for (unsigned i = 0; i < repeated.width; ++i)
>+            m_text.append(repeated.character);
>+
>+        return *this;
>+    }
>+
>     class IndentScope {
>     public:
>         IndentScope(TextStream& ts, int amount = 1)
>diff --git a/Source/WebCore/PAL/pal/LogMacros.h b/Source/WebCore/PAL/pal/LogMacros.h
>index 418134a3575f891cf7875d0cf8ba92ea486d5804..7455836459dbd8f32e8ac1020c6d8796f784d4c3 100644
>--- a/Source/WebCore/PAL/pal/LogMacros.h
>+++ b/Source/WebCore/PAL/pal/LogMacros.h
>@@ -32,9 +32,11 @@
> #else
> 
> #define LOG_WITH_STREAM(channel, commands) do { \
>-        WTF::TextStream stream(WTF::TextStream::LineMode::SingleLine); \
>-        commands; \
>-        WTFLog(&JOIN_LOG_CHANNEL_WITH_PREFIX(LOG_CHANNEL_PREFIX, channel), "%s", stream.release().utf8().data()); \
>+        if (JOIN_LOG_CHANNEL_WITH_PREFIX(LOG_CHANNEL_PREFIX, channel).state == WTFLogChannelState::On) { \
>+            WTF::TextStream stream(WTF::TextStream::LineMode::SingleLine); \
>+            commands; \
>+            WTFLog(&JOIN_LOG_CHANNEL_WITH_PREFIX(LOG_CHANNEL_PREFIX, channel), "%s", stream.release().utf8().data()); \
>+        } \
>     } while (0)
> 
> #endif // !LOG_DISABLED
>diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
>index 7a1dc38fcf2de78a320e5fc58704156770114d93..e482031c4018b6ab185c6c15e62cfae4294377c4 100644
>--- a/Source/WebCore/rendering/RenderLayer.cpp
>+++ b/Source/WebCore/rendering/RenderLayer.cpp
>@@ -280,10 +280,6 @@ static TextStream& operator<<(TextStream& ts, const ClipRects& clipRects)
>     return ts;
> }
> 
>-static bool clipRectsLogEnabled()
>-{
>-    return LogClipRects.state == WTFLogChannelState::On;
>-}
> #endif
> 
> RenderLayer::RenderLayer(RenderLayerModelObject& rendererLayerModelObject)
>@@ -5621,10 +5617,7 @@ void RenderLayer::calculateClipRects(const ClipRectsContext& clipRectsContext, C
>         }
>     }
> 
>-#if !LOG_DISABLED
>-    if (clipRectsLogEnabled())
>-        LOG_WITH_STREAM(ClipRects, stream << "RenderLayer " << this << " calculateClipRects " << clipRects);
>-#endif
>+    LOG_WITH_STREAM(ClipRects, stream << "RenderLayer " << this << " calculateClipRects " << clipRects);
> }
> 
> Ref<ClipRects> RenderLayer::parentClipRects(const ClipRectsContext& clipRectsContext) const
>@@ -5670,10 +5663,7 @@ ClipRect RenderLayer::backgroundClipRect(const ClipRectsContext& clipRectsContex
>     if (parentRects->fixed() && &clipRectsContext.rootLayer->renderer() == &view && !backgroundClipRect.isInfinite())
>         backgroundClipRect.moveBy(view.frameView().scrollPositionForFixedPosition());
> 
>-#if !LOG_DISABLED
>-    if (clipRectsLogEnabled())
>-        LOG_WITH_STREAM(ClipRects, stream << "RenderLayer " << this << " backgroundClipRect with context " << clipRectsContext << " returning " << backgroundClipRect);
>-#endif
>+    LOG_WITH_STREAM(ClipRects, stream << "RenderLayer " << this << " backgroundClipRect with context " << clipRectsContext << " returning " << backgroundClipRect);
>     return backgroundClipRect;
> }
> 
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>index 07e8cc79b95e4f0c3d176d5ac429bdd651f04a5e..ff0f6c61fd14390515d063ee87c0ef9dc5984a20 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>@@ -942,9 +942,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>         return;
>     }
> 
>-#if ENABLE(TREE_DEBUGGING)
>-    LOG(Compositing, "%*p %s computeCompositingRequirements (backing provider candidate %p)", 12 + compositingState.depth * 2, &layer, layer.isNormalFlowOnly() ? "n" : "s", backingSharingState.backingProviderCandidate());
>-#endif
>+    LOG_WITH_STREAM(Compositing, stream << TextStream::repeat(' ', compositingState.depth * 2) << &layer << (layer.isNormalFlowOnly() ? " n" : " s") << " computeCompositingRequirements (backing provider candidate " << backingSharingState.backingProviderCandidate() << ")");
> 
>     // FIXME: maybe we can avoid updating all remaining layers in paint order.
>     compositingState.fullPaintOrderTraversalRequired |= layer.needsCompositingRequirementsTraversal();
>@@ -1172,13 +1170,10 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>         layer.setNeedsCompositingLayerConnection();
>     }
> 
>-#if ENABLE(TREE_DEBUGGING)
>-    LOG(Compositing, "%*p computeCompositingRequirements - willBeComposited %d (backing provider candidate %p)", 12 + compositingState.depth * 2, &layer, willBeComposited, backingSharingState.backingProviderCandidate());
>-#endif
>-
>     layer.clearCompositingRequirementsTraversalState();
>-    
>     overlapMap.geometryMap().popMappingsToAncestor(ancestorLayer);
>+
>+    LOG_WITH_STREAM(Compositing, stream << TextStream::repeat(' ', compositingState.depth * 2) << &layer << " computeCompositingRequirements - willBeComposited " << willBeComposited << " (backing provider candidate " << backingSharingState.backingProviderCandidate() << ")");
> }
> 
> // We have to traverse unchanged layers to fill in the overlap map.
>@@ -1188,9 +1183,7 @@ void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer,
>     ASSERT(!layer.hasDescendantNeedingCompositingRequirementsTraversal());
>     ASSERT(!layer.needsCompositingRequirementsTraversal());
> 
>-#if ENABLE(TREE_DEBUGGING)
>-    LOG(Compositing, "%*p traverseUnchangedSubtree", 12 + compositingState.depth * 2, &layer);
>-#endif
>+    LOG_WITH_STREAM(Compositing, stream << TextStream::repeat(' ', compositingState.depth * 2) << &layer << (layer.isNormalFlowOnly() ? " n" : " s") << " traverseUnchangedSubtree");
> 
>     layer.updateDescendantDependentFlags();
>     layer.updateLayerListsIfNeeded();

Flags:

achristensen: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 197905: 369929 | 369961