Make LOG_WITH_STREAM more efficient
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 May 2019 18:53:45 +0000 (18:53 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 May 2019 18:53:45 +0000 (18:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197905

Reviewed by Alex Christensen.
Source/WebCore:

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):

Source/WebCore/PAL:

Make the LOG_WITH_STREAM macro check that the log channel is enabled before
building the stream.

* pal/LogMacros.h:

Source/WTF:

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<<):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245336 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/text/TextStream.h
Source/WebCore/ChangeLog
Source/WebCore/PAL/ChangeLog
Source/WebCore/PAL/pal/LogMacros.h
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderLayerCompositor.cpp

index a421f4d..9b33d2f 100644 (file)
@@ -1,3 +1,17 @@
+2019-05-15  Simon Fraser  <simon.fraser@apple.com>
+
+        Make LOG_WITH_STREAM more efficient
+        https://bugs.webkit.org/show_bug.cgi?id=197905
+
+        Reviewed by Alex Christensen.
+
+        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-15  Víctor Manuel Jáquez Leal  <vjaquez@igalia.com>
 
         compilation failure with clang 9
index 24a370c..ec7f26c 100644 (file)
@@ -102,6 +102,22 @@ public:
         return (*func)(*this);
     }
 
+    struct Repeat {
+        Repeat(unsigned inWidth, char inCharacter)
+            : width(inWidth), character(inCharacter)
+        { }
+        unsigned width { 0 };
+        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)
index a608751..44db6ae 100644 (file)
 
 2019-05-15  Simon Fraser  <simon.fraser@apple.com>
 
+        Make LOG_WITH_STREAM more efficient
+        https://bugs.webkit.org/show_bug.cgi?id=197905
+
+        Reviewed by Alex Christensen.
+
+        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-15  Simon Fraser  <simon.fraser@apple.com>
+
         Move RenderLayerCompositor's OverlapMap to its own file
         https://bugs.webkit.org/show_bug.cgi?id=197915
 
index 5e75b25..94807af 100644 (file)
@@ -1,3 +1,15 @@
+2019-05-15  Simon Fraser  <simon.fraser@apple.com>
+
+        Make LOG_WITH_STREAM more efficient
+        https://bugs.webkit.org/show_bug.cgi?id=197905
+
+        Reviewed by Alex Christensen.
+        
+        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
index 418134a..7455836 100644 (file)
 #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
index 1f7c686..e5034c5 100644 (file)
@@ -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)
@@ -5629,10 +5625,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
@@ -5678,10 +5671,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;
 }
 
index 1f65c85..b633bf9 100644 (file)
@@ -138,7 +138,7 @@ struct RenderLayerCompositor::CompositingState {
 #if ENABLE(CSS_COMPOSITING)
         childState.hasNotIsolatedCompositedBlendingDescendants = false; // FIXME: should this only be reset for stacking contexts?
 #endif
-#if ENABLE(TREE_DEBUGGING)
+#if !LOG_DISABLED
         childState.depth = depth + 1;
 #endif
         return childState;
@@ -167,8 +167,8 @@ struct RenderLayerCompositor::CompositingState {
 #if ENABLE(CSS_COMPOSITING)
     bool hasNotIsolatedCompositedBlendingDescendants { false };
 #endif
-#if ENABLE(TREE_DEBUGGING)
-    int depth { 0 };
+#if !LOG_DISABLED
+    unsigned depth { 0 };
 #endif
 };
 
@@ -830,9 +830,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();
@@ -1060,13 +1058,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.
@@ -1076,9 +1071,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();