Changeset 89142 in webkit


Ignore:
Timestamp:
Jun 17, 2011 10:36:43 AM (13 years ago)
Author:
commit-queue@webkit.org
Message:

2011-06-17 Julien Chaffraix <jchaffraix@google.com>

Reviewed by Darin Adler.

Avoid extra work in RenderStyle::visitedDependentColor
https://bugs.webkit.org/show_bug.cgi?id=62868

Refactoring only, no new test required.

The code used to cache the result of borderStyleForColorProperty. However
the value was either overwritten inside colorIncludingFallback or there was
not border. Thus I removed borderStyleForColorProperty and inlined the logic in
colorIncludingFallback.

This shows some nice performance improvements on the bug page (table of 22k rows with a link
for each row). Using pprof, the time spend in RenderStyle::visitedDependentColor is reduced
by ~10%, mostly due to removing the call to borderStyleForColorProperty.

  • rendering/style/RenderStyle.cpp: (WebCore::RenderStyle::colorIncludingFallback): We now calculate the borderStyle inside this function (which was already the case I just made it explicit). Also simplified the final 'if' as the border will be set only for the right CSS border properties.

(WebCore::RenderStyle::visitedDependentColor): Removed the |borderStyle| variable
as it was never read.

  • rendering/style/RenderStyle.h: Removed the parameter.
Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r89135 r89142  
     12011-06-17  Julien Chaffraix  <jchaffraix@google.com>
     2
     3        Reviewed by Darin Adler.
     4
     5        Avoid extra work in RenderStyle::visitedDependentColor
     6        https://bugs.webkit.org/show_bug.cgi?id=62868
     7
     8        Refactoring only, no new test required.
     9
     10        The code used to cache the result of borderStyleForColorProperty. However
     11        the value was either overwritten inside colorIncludingFallback or there was
     12        not border. Thus I removed borderStyleForColorProperty and inlined the logic in
     13        colorIncludingFallback.
     14
     15        This shows some nice performance improvements on the bug page (table of 22k rows with a link
     16        for each row). Using pprof, the time spend in RenderStyle::visitedDependentColor is reduced
     17        by ~10%, mostly due to removing the call to borderStyleForColorProperty.
     18
     19        * rendering/style/RenderStyle.cpp:
     20        (WebCore::RenderStyle::colorIncludingFallback): We now calculate the borderStyle
     21        inside this function (which was already the case I just made it explicit). Also
     22        simplified the final 'if' as the border will be set only for the right CSS border
     23        properties.
     24
     25        (WebCore::RenderStyle::visitedDependentColor): Removed the |borderStyle| variable
     26        as it was never read.
     27
     28        * rendering/style/RenderStyle.h: Removed the parameter.
     29
    1302011-06-16  Pavel Podivilov  <podivilov@chromium.org>
    231
  • trunk/Source/WebCore/rendering/style/RenderStyle.cpp

    r88526 r89142  
    10941094}
    10951095
    1096 static EBorderStyle borderStyleForColorProperty(const RenderStyle* style, int colorProperty)
    1097 {
    1098     EBorderStyle borderStyle;
    1099     switch (colorProperty) {
    1100     case CSSPropertyBorderLeftColor:
    1101         borderStyle = style->borderLeftStyle();
    1102         break;
    1103     case CSSPropertyBorderRightColor:
    1104         borderStyle = style->borderRightStyle();
    1105         break;
    1106     case CSSPropertyBorderTopColor:
    1107         borderStyle = style->borderTopStyle();
    1108         break;
    1109     case CSSPropertyBorderBottomColor:
    1110         borderStyle = style->borderBottomStyle();
    1111         break;
    1112     default:
    1113         borderStyle = BNONE;
    1114         break;
    1115     }
    1116     return borderStyle;
    1117 }
    1118 
    1119 const Color RenderStyle::colorIncludingFallback(int colorProperty, EBorderStyle borderStyle) const
     1096const Color RenderStyle::colorIncludingFallback(int colorProperty) const
    11201097{
    11211098    Color result;
     1099    EBorderStyle borderStyle = BNONE;
    11221100    switch (colorProperty) {
    11231101    case CSSPropertyBackgroundColor:
     
    11631141
    11641142    if (!result.isValid()) {
    1165         if ((colorProperty == CSSPropertyBorderLeftColor || colorProperty == CSSPropertyBorderRightColor
    1166             || colorProperty == CSSPropertyBorderTopColor || colorProperty == CSSPropertyBorderBottomColor)
    1167             && (borderStyle == INSET || borderStyle == OUTSET || borderStyle == RIDGE || borderStyle == GROOVE))
     1143        if (borderStyle == INSET || borderStyle == OUTSET || borderStyle == RIDGE || borderStyle == GROOVE)
    11681144            result.setRGB(238, 238, 238);
    11691145        else
     
    11761152const Color RenderStyle::visitedDependentColor(int colorProperty) const
    11771153{
    1178     EBorderStyle borderStyle = borderStyleForColorProperty(this, colorProperty);
    1179     Color unvisitedColor = colorIncludingFallback(colorProperty, borderStyle);
     1154    Color unvisitedColor = colorIncludingFallback(colorProperty);
    11801155    if (insideLink() != InsideVisitedLink)
    11811156        return unvisitedColor;
     
    11841159    if (!visitedStyle)
    11851160        return unvisitedColor;
    1186     Color visitedColor = visitedStyle->colorIncludingFallback(colorProperty, borderStyle);
     1161    Color visitedColor = visitedStyle->colorIncludingFallback(colorProperty);
    11871162
    11881163    // FIXME: Technically someone could explicitly specify the color transparent, but for now we'll just
  • trunk/Source/WebCore/rendering/style/RenderStyle.h

    r88804 r89142  
    13571357    const Color& textStrokeColor() const { return rareInheritedData->textStrokeColor; }
    13581358   
    1359     const Color colorIncludingFallback(int colorProperty, EBorderStyle) const;
     1359    const Color colorIncludingFallback(int colorProperty) const;
    13601360
    13611361    void appendContent(PassOwnPtr<ContentData>);
Note: See TracChangeset for help on using the changeset viewer.