Changeset 111733 in webkit


Ignore:
Timestamp:
Mar 22, 2012 11:10:55 AM (12 years ago)
Author:
commit-queue@webkit.org
Message:

Make Length Calculation functions non-inline
​https://bugs.webkit.org/show_bug.cgi?id=81733

Currently length calculation functions in LengthFunctions.h are inline. These functions are pretty big to be inline.
And these functions are expected to grow again when new length units will be introduced in bug 27160.

A decent rule of thumb is to not inline a function if it is more than 10 lines long. Also it's typically not cost effective to inline
functions with loops or switch statements. (Reference: ​http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Inline_Functions).

Ran PerformanceTests/Parser/html5-full-render.html on Mac Snow-Leopard with and without the patch and did not see much performance difference.

Patch by Joe Thomas <​joethomas@motorola.com> on 2012-03-22
Reviewed by Antti Koivisto.

  • CMakeLists.txt:
  • GNUmakefile.list.am:
  • Target.pri:
  • WebCore.gypi:
  • WebCore.vcproj/WebCore.vcproj:
  • WebCore.xcodeproj/project.pbxproj:
  • css/LengthFunctions.cpp: Added.

(WebCore):
(WebCore::miminumValueForLength):
(WebCore::valueForLength):
(WebCore::floatValueForLength):

  • css/LengthFunctions.h:

(WebCore):

Location:
trunk/Source/WebCore
Files:
8 edited
1 copied

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/CMakeLists.txt

    r111674 r111733  
    500500    css/FontFeatureValue.cpp
    501501    css/FontValue.cpp
     502    css/LengthFunctions.cpp
    502503    css/MediaFeatureNames.cpp
    503504    css/MediaList.cpp

  • trunk/Source/WebCore/ChangeLog

    r111731 r111733  
     12012-03-22  Joe Thomas  <joethomas@motorola.com>
     2
     3        Make Length Calculation functions non-inline
     4        https://bugs.webkit.org/show_bug.cgi?id=81733
     5
     6        Currently length calculation functions in LengthFunctions.h are inline. These functions are pretty big to be inline.
     7        And these functions are expected to grow again when new length units will be introduced in bug 27160.
     8
     9        A decent rule of thumb is to not inline a function if it is more than 10 lines long. Also it's typically not cost effective to inline
     10        functions with loops or switch statements. (Reference: http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Inline_Functions).
     11
     12        Ran PerformanceTests/Parser/html5-full-render.html on Mac Snow-Leopard with and without the patch and did not see much performance difference.
     13
     14        Reviewed by Antti Koivisto.
     15
     16        * CMakeLists.txt:
     17        * GNUmakefile.list.am:
     18        * Target.pri:
     19        * WebCore.gypi:
     20        * WebCore.vcproj/WebCore.vcproj:
     21        * WebCore.xcodeproj/project.pbxproj:
     22        * css/LengthFunctions.cpp: Added.
     23        (WebCore):
     24        (WebCore::miminumValueForLength):
     25        (WebCore::valueForLength):
     26        (WebCore::floatValueForLength):
     27        * css/LengthFunctions.h:
     28        (WebCore):
     29
    1302012-03-22  Alexis Menard  <alexis.menard@openbossa.org>
    231

  • trunk/Source/WebCore/GNUmakefile.list.am

    r111686 r111733  
    16481648        Source/WebCore/css/FontValue.cpp \
    16491649        Source/WebCore/css/FontValue.h \
     1650        Source/WebCore/css/LengthFunctions.cpp \
    16501651        Source/WebCore/css/LengthFunctions.h \
    16511652        Source/WebCore/css/MediaFeatureNames.cpp \

  • trunk/Source/WebCore/Target.pri

    r111674 r111733  
    476476    css/FontFeatureValue.cpp \
    477477    css/FontValue.cpp \
     478    css/LengthFunctions.cpp \
    478479    css/MediaFeatureNames.cpp \
    479480    css/MediaList.cpp \

  • trunk/Source/WebCore/WebCore.gypi

    r111686 r111733  
    24312431            'css/FontValue.cpp',
    24322432            'css/FontValue.h',
     2433            'css/LengthFunctions.cpp',
    24332434            'css/MediaFeatureNames.cpp',
    24342435            'css/MediaFeatureNames.h',

  • trunk/Source/WebCore/WebCore.vcproj/WebCore.vcproj

    r111655 r111733  
    3647536475                        </File>
    3647636476                        <File
     36477                                RelativePath="..\css\LengthFunctions.cpp"
     36478                                >
     36479                        </File>
     36480                        <File
    3647736481                                RelativePath="..\css\LengthFunctions.h"
    3647836482                                >

  • trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

    r111674 r111733  
    59665966                E4D687770ED7AE3D006EA978 /* PurgeableBufferMac.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E4D687760ED7AE3D006EA978 /* PurgeableBufferMac.cpp */; };
    59675967                E4D687790ED7AE4F006EA978 /* PurgeableBuffer.h in Headers */ = {isa = PBXBuildFile; fileRef = E4D687780ED7AE4F006EA978 /* PurgeableBuffer.h */; };
     5968                E55F497A151B888000BB67DB /* LengthFunctions.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E55F4979151B888000BB67DB /* LengthFunctions.cpp */; };
    59685969                E5BA7D63151437CA00FE1E3F /* LengthFunctions.h in Headers */ = {isa = PBXBuildFile; fileRef = E5BA7D62151437CA00FE1E3F /* LengthFunctions.h */; settings = {ATTRIBUTES = (Private, ); }; };
    59695970                ED2BA83C09A24B91006C0AC4 /* DocumentMarker.h in Headers */ = {isa = PBXBuildFile; fileRef = ED2BA83B09A24B91006C0AC4 /* DocumentMarker.h */; settings = {ATTRIBUTES = (Private, ); }; };
    … …  
    1308713088                E4D687760ED7AE3D006EA978 /* PurgeableBufferMac.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PurgeableBufferMac.cpp; sourceTree = "<group>"; };
    1308813089                E4D687780ED7AE4F006EA978 /* PurgeableBuffer.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PurgeableBuffer.h; sourceTree = "<group>"; };
     13090                E55F4979151B888000BB67DB /* LengthFunctions.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = LengthFunctions.cpp; sourceTree = "<group>"; };
    1308913091                E5BA7D62151437CA00FE1E3F /* LengthFunctions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LengthFunctions.h; sourceTree = "<group>"; };
    1309013092                ED2BA83B09A24B91006C0AC4 /* DocumentMarker.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DocumentMarker.h; sourceTree = "<group>"; };
    … …  
    2049120493                                CDBD93BA1333BD4B002570E3 /* fullscreenQuickTime.css */,
    2049220494                                93CA4C9909DF93FA00DF8677 /* html.css */,
     20495                                E55F4979151B888000BB67DB /* LengthFunctions.cpp */,
    2049320496                                E5BA7D62151437CA00FE1E3F /* LengthFunctions.h */,
    2049420497                                93CA4C9A09DF93FA00DF8677 /* make-css-file-arrays.pl */,
    … …  
    2761127614                                9393E5FF151A99F200066F06 /* CSSImageSetValue.cpp in Sources */,
    2761227615                                9393E604151A9A1800066F06 /* StyleCachedImageSet.cpp in Sources */,
     27616                                E55F497A151B888000BB67DB /* LengthFunctions.cpp in Sources */,
    2761327617                        );
    2761427618                        runOnlyForDeploymentPostprocessing = 0;

  • trunk/Source/WebCore/css/LengthFunctions.cpp

    r111732 r111733  
    2222*/
    2323
    24 #ifndef LengthFunctions_h
    25 #define LengthFunctions_h
     24#include "config.h"
     25#include "LengthFunctions.h"
    2626
    2727#include "Length.h"
    … …  
    2929namespace WebCore {
    3030
    31 inline int miminumValueForLength(Length length, int maximumValue, bool roundPercentages = false)
     31int miminumValueForLength(Length length, int maximumValue, bool roundPercentages)
    3232{
    3333    switch (length.type()) {
    … …  
    5454}
    5555
    56 inline int valueForLength(Length length, int maximumValue, bool roundPercentages = false)
     56int valueForLength(Length length, int maximumValue, bool roundPercentages)
    5757{
    5858    switch (length.type()) {
    … …  
    7575
    7676// FIXME: when subpixel layout is supported this copy of floatValueForLength() can be removed. See bug 71143.
    77 inline float floatValueForLength(Length length, int maximumValue)
     77float floatValueForLength(Length length, int maximumValue)
    7878{
    7979    switch (length.type()) {
    … …  
    9797}
    9898
    99 inline float floatValueForLength(Length length, float maximumValue)
     99float floatValueForLength(Length length, float maximumValue)
    100100{
    101101    switch (length.type()) {
    … …  
    120120
    121121} // namespace WebCore
    122 
    123 #endif // LengthFunctions_h

  • trunk/Source/WebCore/css/LengthFunctions.h

    r111126 r111733  
    2525#define LengthFunctions_h
    2626
    27 #include "Length.h"
    28 
    2927namespace WebCore {
    3028
    31 inline int miminumValueForLength(Length length, int maximumValue, bool roundPercentages = false)
    32 {
    33     switch (length.type()) {
    34     case Fixed:
    35         return length.value();
    36     case Percent:
    37         if (roundPercentages)
    38             return static_cast<int>(round(maximumValue * length.percent() / 100.0f));
    39         // Don't remove the extra cast to float. It is needed for rounding on 32-bit Intel machines that use the FPU stack.
    40         return static_cast<int>(static_cast<float>(maximumValue * length.percent() / 100.0f));
    41     case Calculated:
    42         return length.nonNanCalculatedValue(maximumValue);
    43     case Auto:
    44         return 0;
    45     case Relative:
    46     case Intrinsic:
    47     case MinIntrinsic:
    48     case Undefined:
    49         ASSERT_NOT_REACHED();
    50         return 0;
    51     }
    52     ASSERT_NOT_REACHED();
    53     return 0;
    54 }
     29struct Length;
    5530
    56 inline int valueForLength(Length length, int maximumValue, bool roundPercentages = false)
    57 {
    58     switch (length.type()) {
    59     case Fixed:
    60     case Percent:
    61     case Calculated:
    62         return miminumValueForLength(length, maximumValue, roundPercentages);
    63     case Auto:
    64         return maximumValue;
    65     case Relative:
    66     case Intrinsic:
    67     case MinIntrinsic:
    68     case Undefined:
    69         ASSERT_NOT_REACHED();
    70         return 0;
    71     }
    72     ASSERT_NOT_REACHED();
    73     return 0;
    74 }
    75 
    76 // FIXME: when subpixel layout is supported this copy of floatValueForLength() can be removed. See bug 71143.
    77 inline float floatValueForLength(Length length, int maximumValue)
    78 {
    79     switch (length.type()) {
    80     case Fixed:
    81         return length.getFloatValue();
    82     case Percent:
    83         return static_cast<float>(maximumValue * length.percent() / 100.0f);
    84     case Auto:
    85         return static_cast<float>(maximumValue);
    86     case Calculated:
    87         return length.nonNanCalculatedValue(maximumValue);               
    88     case Relative:
    89     case Intrinsic:
    90     case MinIntrinsic:
    91     case Undefined:
    92         ASSERT_NOT_REACHED();
    93         return 0;
    94     }
    95     ASSERT_NOT_REACHED();
    96     return 0;
    97 }
    98 
    99 inline float floatValueForLength(Length length, float maximumValue)
    100 {
    101     switch (length.type()) {
    102     case Fixed:
    103         return length.getFloatValue();
    104     case Percent:
    105         return static_cast<float>(maximumValue * length.percent() / 100.0f);
    106     case Auto:
    107         return static_cast<float>(maximumValue);
    108     case Calculated:
    109         return length.nonNanCalculatedValue(maximumValue);
    110     case Relative:
    111     case Intrinsic:
    112     case MinIntrinsic:
    113     case Undefined:
    114         ASSERT_NOT_REACHED();
    115         return 0;
    116     }
    117     ASSERT_NOT_REACHED();
    118     return 0;
    119 }
     31int miminumValueForLength(Length, int maximumValue, bool roundPercentages = false);
     32int valueForLength(Length, int maximumValue, bool roundPercentages = false);
     33float floatValueForLength(Length, int maximumValue);
     34float floatValueForLength(Length, float maximumValue);
    12035
    12136} // namespace WebCore
Note: See TracChangeset for help on using the changeset viewer.