Title: [211353] trunk/Source/WebCore
Revision
211353
Author
za...@apple.com
Date
2017-01-29 12:21:08 -0800 (Sun, 29 Jan 2017)

Log Message

Simple line layout: PerformanceTests/Layout/simple-line-layout-innertext.html regressed at r211108
https://bugs.webkit.org/show_bug.cgi?id=167562

Reviewed by Antti Koivisto.

Apparently RunResolver::Run::constructStringForHyphenIfNeeded() is in a superhot codepath.
The Run should not have any additional members anyway, so let's construct the hyphenated
string on demand.
Progression on simple-line-layout-innertext.html is from ~34runs/sec back to ~50runs/sec.

Covered by existing text.

* rendering/RenderTreeAsText.cpp:
(WebCore::writeSimpleLine):
* rendering/SimpleLineLayoutFunctions.cpp:
(WebCore::SimpleLineLayout::paintFlow):
* rendering/SimpleLineLayoutResolver.cpp:
(WebCore::SimpleLineLayout::RunResolver::Run::Run):
(WebCore::SimpleLineLayout::RunResolver::Run::textWithHyphen):
(WebCore::SimpleLineLayout::RunResolver::Run::text):
(WebCore::SimpleLineLayout::RunResolver::Run::constructStringForHyphenIfNeeded): Deleted.
* rendering/SimpleLineLayoutResolver.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (211352 => 211353)


--- trunk/Source/WebCore/ChangeLog	2017-01-29 15:13:44 UTC (rev 211352)
+++ trunk/Source/WebCore/ChangeLog	2017-01-29 20:21:08 UTC (rev 211353)
@@ -1,3 +1,28 @@
+2017-01-29  Zalan Bujtas  <za...@apple.com>
+
+        Simple line layout: PerformanceTests/Layout/simple-line-layout-innertext.html regressed at r211108
+        https://bugs.webkit.org/show_bug.cgi?id=167562
+
+        Reviewed by Antti Koivisto.
+
+        Apparently RunResolver::Run::constructStringForHyphenIfNeeded() is in a superhot codepath.
+        The Run should not have any additional members anyway, so let's construct the hyphenated
+        string on demand.
+        Progression on simple-line-layout-innertext.html is from ~34runs/sec back to ~50runs/sec.
+
+        Covered by existing text.
+
+        * rendering/RenderTreeAsText.cpp:
+        (WebCore::writeSimpleLine):
+        * rendering/SimpleLineLayoutFunctions.cpp:
+        (WebCore::SimpleLineLayout::paintFlow):
+        * rendering/SimpleLineLayoutResolver.cpp:
+        (WebCore::SimpleLineLayout::RunResolver::Run::Run):
+        (WebCore::SimpleLineLayout::RunResolver::Run::textWithHyphen):
+        (WebCore::SimpleLineLayout::RunResolver::Run::text):
+        (WebCore::SimpleLineLayout::RunResolver::Run::constructStringForHyphenIfNeeded): Deleted.
+        * rendering/SimpleLineLayoutResolver.h:
+
 2017-01-28  Tim Horton  <timothy_hor...@apple.com>
 
         Don't flash a tap highlight for the entirety of an editable WKWebView

Modified: trunk/Source/WebCore/rendering/RenderTreeAsText.cpp (211352 => 211353)


--- trunk/Source/WebCore/rendering/RenderTreeAsText.cpp	2017-01-29 15:13:44 UTC (rev 211352)
+++ trunk/Source/WebCore/rendering/RenderTreeAsText.cpp	2017-01-29 20:21:08 UTC (rev 211353)
@@ -504,12 +504,9 @@
         y -= floorToInt(downcast<RenderTableCell>(*renderText.containingBlock()).intrinsicPaddingBefore());
 
     ts << "text run at (" << x << "," << y << ") width " << logicalWidth;
-    if (run.hasHyphen()) {
-        ts << ": " << quoteAndEscapeNonPrintables(run.text().substring(0, run.text().length() - 1));
+    ts << ": " << quoteAndEscapeNonPrintables(run.text());
+    if (run.hasHyphen())
         ts << " + hyphen string " << quoteAndEscapeNonPrintables(renderText.style().hyphenString().string());
-    } else
-        ts << ": " << quoteAndEscapeNonPrintables(run.text());
-
     ts << "\n";
 }
 

Modified: trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp (211352 => 211353)


--- trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp	2017-01-29 15:13:44 UTC (rev 211352)
+++ trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp	2017-01-29 20:21:08 UTC (rev 211353)
@@ -118,8 +118,11 @@
         if (paintRect.y() > visualOverflowRect.maxY() || paintRect.maxY() < visualOverflowRect.y())
             continue;
 
+        String textWithHyphen;
+        if (run.hasHyphen())
+            textWithHyphen = run.textWithHyphen();
         // x position indicates the line offset from the rootbox. It's always 0 in case of simple line layout.
-        TextRun textRun(run.text(), 0, run.expansion(), run.expansionBehavior());
+        TextRun textRun(run.hasHyphen() ? textWithHyphen : run.text(), 0, run.expansion(), run.expansionBehavior());
         textRun.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
         FloatPoint textOrigin = FloatPoint(rect.x() + paintOffset.x(), roundToDevicePixel(run.baselinePosition() + paintOffset.y(), deviceScaleFactor));
         textPainter.paintText(textRun, textRun.length(), rect, textOrigin);

Modified: trunk/Source/WebCore/rendering/SimpleLineLayoutResolver.cpp (211352 => 211353)


--- trunk/Source/WebCore/rendering/SimpleLineLayoutResolver.cpp	2017-01-29 15:13:44 UTC (rev 211352)
+++ trunk/Source/WebCore/rendering/SimpleLineLayoutResolver.cpp	2017-01-29 20:21:08 UTC (rev 211353)
@@ -47,19 +47,17 @@
 RunResolver::Run::Run(const Iterator& iterator)
     : m_iterator(iterator)
 {
-    constructStringForHyphenIfNeeded();
 }
 
-void RunResolver::Run::constructStringForHyphenIfNeeded()
+String RunResolver::Run::textWithHyphen() const
 {
     auto& run = m_iterator.simpleRun();
-    if (!run.hasHyphen)
-        return;
+    ASSERT(run.hasHyphen);
     // Empty runs should not have hyphen.
     ASSERT(run.start < run.end);
     auto& segment = m_iterator.resolver().m_flowContents.segmentForRun(run.start, run.end);
     auto text = StringView(segment.text).substring(segment.toSegmentPosition(run.start), run.end - run.start);
-    m_textWithHyphen = makeString(text, m_iterator.resolver().flow().style().hyphenString());
+    return makeString(text, m_iterator.resolver().flow().style().hyphenString());
 }
 
 FloatRect RunResolver::Run::rect() const
@@ -82,8 +80,6 @@
 
 StringView RunResolver::Run::text() const
 {
-    if (m_textWithHyphen)
-        return StringView(*m_textWithHyphen);
     auto& run = m_iterator.simpleRun();
     ASSERT(run.start < run.end);
     auto& segment = m_iterator.resolver().m_flowContents.segmentForRun(run.start, run.end);

Modified: trunk/Source/WebCore/rendering/SimpleLineLayoutResolver.h (211352 => 211353)


--- trunk/Source/WebCore/rendering/SimpleLineLayoutResolver.h	2017-01-29 15:13:44 UTC (rev 211352)
+++ trunk/Source/WebCore/rendering/SimpleLineLayoutResolver.h	2017-01-29 20:21:08 UTC (rev 211353)
@@ -64,6 +64,7 @@
         ExpansionBehavior expansionBehavior() const;
         int baselinePosition() const;
         StringView text() const;
+        String textWithHyphen() const;
         bool isEndOfLine() const;
         bool hasHyphen() const { return m_iterator.simpleRun().hasHyphen; }
 
@@ -74,7 +75,6 @@
         void constructStringForHyphenIfNeeded();
 
         const Iterator& m_iterator;
-        std::optional<String> m_textWithHyphen;
     };
 
     class Iterator {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to