Optimize line layout to avoid overflow computation and allocation. RenderOverflows are allocated in quirks mode when they don't need to be. We also do a whole crawl of the line to compute overflow when none is going to exist.
Created attachment 87772 [details] Patch
Attachment 87772 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/rendering/InlineFlowBox.cpp:744: Extra space for operator ! [whitespace/operators] [4] Total errors found: 1 in 30 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 87772 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=87772&action=review > Source/WebCore/ChangeLog:8 > + Optimize overflow computations on lines to avoid allocating RenderOverflows in nearly all cases and to avoid even having > + to check the line for overflow in the first place. Would be nice to quantify the gain here.
Comment on attachment 87772 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=87772&action=review I suspect you could get even more speedup by tuning the inlining so that the rare case was out of line more often. > Source/WebCore/rendering/EllipsisBox.cpp:32 > -void EllipsisBox::paint(PaintInfo& paintInfo, int tx, int ty) > +void EllipsisBox::paint(PaintInfo& paintInfo, int tx, int ty, int lineTop, int lineBottom) As these get more and more arguments I wonder if we might want to use a struct in the future. > Source/WebCore/rendering/InlineBox.cpp:162 > +void InlineBox::paint(PaintInfo& paintInfo, int tx, int ty, int /* lineTop */, int /*lineBottom*/) I think you could just leave these names out instead of including comments. > Source/WebCore/rendering/InlineBox.cpp:290 > + m_knownToHaveNoOverflow = false; > + if (parent() && parent()->knownToHaveNoOverflow()) > + parent()->clearKnownToHaveNoOverflow(); This might be better as a loop instead of recursion. > Source/WebCore/rendering/InlineFlowBox.cpp:153 > + if (child->isText()) { > + RenderStyle* childStyle = child->renderer()->style(m_firstLine); > + if (childStyle->letterSpacing() < 0 || childStyle->textShadow() || childStyle->textEmphasisMark() != TextEmphasisMarkNone || childStyle->textStrokeWidth()) > + child->clearKnownToHaveNoOverflow(); > + } else if (child->renderer()->isReplaced()) { > + RenderBox* box = toRenderBox(child->renderer()); > + if (box->hasRenderOverflow() || box->hasSelfPaintingLayer()) > + child->clearKnownToHaveNoOverflow(); > + } else if (!child->renderer()->isBR() && (child->renderer()->style(m_firstLine)->boxShadow() || child->boxModelObject()->hasSelfPaintingLayer() > + || (child->renderer()->isListMarker() && !toRenderListMarker(child->renderer())->isInside()))) > + child->clearKnownToHaveNoOverflow(); Why not just skip all this work if child->knownToHaveNoOverflow is already false? You are doing that elsewhere. > Source/WebCore/rendering/InlineFlowBox.cpp:854 > - IntRect frameBox = enclosingIntRect(FloatRect(x(), y(), width(), height())); > - if (frameBox == rect || rect.isEmpty()) > + IntRect frameBox = enclosingIntRect(frameRectIncludingLineHeight(lineTop, lineBottom)); > + if (frameBox.contains(rect) || rect.isEmpty()) > return; Maybe the rect.isEmpty() check should come first. Why compute frameBox in that case? > Source/WebCore/rendering/InlineFlowBox.cpp:866 > - IntRect frameBox = enclosingIntRect(FloatRect(x(), y(), width(), height())); > - if (frameBox == rect || rect.isEmpty()) > + IntRect frameBox = enclosingIntRect(frameRectIncludingLineHeight(lineTop, lineBottom)); > + if (frameBox.contains(rect) || rect.isEmpty()) > return; Maybe the rect.isEmpty() check should come first. Why compute frameBox in that case? > Source/WebCore/rendering/RenderBox.h:401 > + RenderOverflow* hasRenderOverflow() const { return m_overflow.get(); } This is named “has” but it is not a boolean. You should change the name or the return type.
Fixed in r82611.
http://trac.webkit.org/changeset/82611 might have broken Qt Linux Release The following tests are not passing: fast/multicol/layers-in-multicol.html
This broke repaint (repaint rect is offset horizontally) in the following case: <style> span:hover { color: blue; } </style> <div style="width: 250px;"> Lorem <span>ipsum<br>dolor sit amet</span> </div>
Filed bug 57735.
We show this change as a ~4.5% speedup in the moz page cycler: http://build.chromium.org/f/chromium/perf/xp-release-dual-core/moz/report.html?history=150&rev=-1
Comment on attachment 87772 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=87772&action=review > Source/WebCore/rendering/InlineFlowBox.h:198 > + int logicalLeftLayoutOverflow() const { return m_overflow ? (isHorizontal() ? m_overflow->minXLayoutOverflow() : m_overflow->minYLayoutOverflow()) : logicalLeft(); } This generates on VS2008: warning C4244: 'return' : conversion from 'float' to 'int', possible loss of data It happens because logicalLeft() returns a float. A static_cast<int>() should be used to clarify the intent. Lines 200, 201, 226, 227, 255, 256, 261. I'm at r84919 so the lines may be offset.