Changeset 245336 in webkit

Timestamp:

May 15, 2019, 11:53:45 AM (7 years ago)

Author:

Simon Fraser

Message:

Make LOG_WITH_STREAM more efficient
​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<<):

Location:

trunk/Source

Files:

• WTF/ChangeLog (modified) (1 diff)
• WTF/wtf/text/TextStream.h (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/PAL/ChangeLog (modified) (1 diff)
• WebCore/PAL/pal/LogMacros.h (modified) (1 diff)
• WebCore/rendering/RenderLayer.cpp (modified) (3 diffs)
• WebCore/rendering/RenderLayerCompositor.cpp (modified) (5 diffs)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WTF/wtf/text/TextStream.h

@@ -103 +103 @@
     }
 
+    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:

trunk/Source/WebCore/ChangeLog

@@ -23 +23 @@
         (WebCore::MockRealtimeMediaSourceCenter::mockRealtimeMediaSourceCenterEnabled):
         * platform/mock/MockRealtimeMediaSourceCenter.h:
+
+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>

trunk/Source/WebCore/PAL/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/PAL/pal/LogMacros.h

@@ -33 +33 @@
 
 #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)
 

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -281 +281 @@
 }
 
-static bool clipRectsLogEnabled()
-{
-    return LogClipRects.state == WTFLogChannelState::On;
-}
 #endif
 
@@ -5630 +5626 @@
     }
 
-#if !LOG_DISABLED
-    if (clipRectsLogEnabled())
-        LOG_WITH_STREAM(ClipRects, stream << "RenderLayer " << this << " calculateClipRects " << clipRects);
-#endif
+    LOG_WITH_STREAM(ClipRects, stream << "RenderLayer " << this << " calculateClipRects " << clipRects);
 }
 
@@ -5679 +5672 @@
         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;
 }

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -139 +139 @@
         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
@@ -168 +168 @@
     bool hasNotIsolatedCompositedBlendingDescendants { false };
 #endif
-#if ENABLE(TREE_DEBUGGING)
-    int depth { 0 };
+#if !LOG_DISABLED
+    unsigned depth { 0 };
 #endif
 };
@@ -831 +831 @@
     }
 
-#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.
@@ -1061 +1059 @@
     }
 
-#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() << ")");
 }
 
@@ -1077 +1072 @@
     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();