Make Length Calculation functions non-inline
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Mar 2012 18:10:55 +0000 (18:10 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Mar 2012 18:10:55 +0000 (18:10 +0000)
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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@111733 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/CMakeLists.txt
Source/WebCore/ChangeLog
Source/WebCore/GNUmakefile.list.am
Source/WebCore/Target.pri
Source/WebCore/WebCore.gypi
Source/WebCore/WebCore.vcproj/WebCore.vcproj
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/css/LengthFunctions.cpp [new file with mode: 0644]
Source/WebCore/css/LengthFunctions.h

index baf3112111540363a0190d18384c9323cbbdc87b..38ca544bcb5b96ad390dc75bd0e7c9704f7bbba8 100644 (file)
@@ -499,6 +499,7 @@ SET(WebCore_SOURCES
     css/CSSWrapShapes.cpp
     css/FontFeatureValue.cpp
     css/FontValue.cpp
+    css/LengthFunctions.cpp
     css/MediaFeatureNames.cpp
     css/MediaList.cpp
     css/MediaQuery.cpp
index 0e78a92eadbd5b42cb72e97e466b0909697d5286..59c8b58dc7cf31a235e2cae1d5b92483bba9a45c 100644 (file)
@@ -1,3 +1,32 @@
+2012-03-22  Joe Thomas  <joethomas@motorola.com>
+
+        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.
+
+        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):
+
 2012-03-22  Alexis Menard  <alexis.menard@openbossa.org>
 
         Increase code sharing between CSSParser and CSSPropertyLonghand.
index e2bcb109fc7b2552f6f833ab8a9e4036526fc170..cf6058e2ea2da319d931f45b19fd0a5f62cb46ad 100644 (file)
@@ -1647,6 +1647,7 @@ webcore_sources += \
        Source/WebCore/css/FontFeatureValue.h \
        Source/WebCore/css/FontValue.cpp \
        Source/WebCore/css/FontValue.h \
+       Source/WebCore/css/LengthFunctions.cpp \
        Source/WebCore/css/LengthFunctions.h \
        Source/WebCore/css/MediaFeatureNames.cpp \
        Source/WebCore/css/MediaFeatureNames.h \
index 0e2db6daaeb5dc354c33f111e3b54c299825f05d..b46301eddc6530aaf40a14300c8b776f6acecff3 100644 (file)
@@ -475,6 +475,7 @@ SOURCES += \
     css/CSSWrapShapes.cpp \
     css/FontFeatureValue.cpp \
     css/FontValue.cpp \
+    css/LengthFunctions.cpp \
     css/MediaFeatureNames.cpp \
     css/MediaList.cpp \
     css/MediaQuery.cpp \
index efba8a960369201f1da8ca1f2828f6b15a521ffc..05f027a274e7ae6fcb3aed429a3debdcd37c631f 100644 (file)
             'css/FontFeatureValue.h',
             'css/FontValue.cpp',
             'css/FontValue.h',
+            'css/LengthFunctions.cpp',
             'css/MediaFeatureNames.cpp',
             'css/MediaFeatureNames.h',
             'css/MediaList.cpp',
index 7e699062ac5d62f8cfbf0c95afc9091792884fcb..0f06e18582788778962eb4555aff724caf9feee1 100755 (executable)
                                RelativePath="..\css\html.css"
                                >
                        </File>
+                       <File
+                               RelativePath="..\css\LengthFunctions.cpp"
+                               >
+                       </File>
                        <File
                                RelativePath="..\css\LengthFunctions.h"
                                >
index ff2a1713ead37243556160731f92a4ee282743d6..9887f6620a43760f4181bc9d40d67c99a5d51678 100644 (file)
                E4C279590CF9741900E97B98 /* RenderMedia.h in Headers */ = {isa = PBXBuildFile; fileRef = E4C279570CF9741900E97B98 /* RenderMedia.h */; };
                E4D687770ED7AE3D006EA978 /* PurgeableBufferMac.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E4D687760ED7AE3D006EA978 /* PurgeableBufferMac.cpp */; };
                E4D687790ED7AE4F006EA978 /* PurgeableBuffer.h in Headers */ = {isa = PBXBuildFile; fileRef = E4D687780ED7AE4F006EA978 /* PurgeableBuffer.h */; };
+               E55F497A151B888000BB67DB /* LengthFunctions.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E55F4979151B888000BB67DB /* LengthFunctions.cpp */; };
                E5BA7D63151437CA00FE1E3F /* LengthFunctions.h in Headers */ = {isa = PBXBuildFile; fileRef = E5BA7D62151437CA00FE1E3F /* LengthFunctions.h */; settings = {ATTRIBUTES = (Private, ); }; };
                ED2BA83C09A24B91006C0AC4 /* DocumentMarker.h in Headers */ = {isa = PBXBuildFile; fileRef = ED2BA83B09A24B91006C0AC4 /* DocumentMarker.h */; settings = {ATTRIBUTES = (Private, ); }; };
                ED501DC60B249F2900AE18D9 /* EditorMac.mm in Sources */ = {isa = PBXBuildFile; fileRef = ED501DC50B249F2900AE18D9 /* EditorMac.mm */; };
                E4C279570CF9741900E97B98 /* RenderMedia.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RenderMedia.h; sourceTree = "<group>"; };
                E4D687760ED7AE3D006EA978 /* PurgeableBufferMac.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PurgeableBufferMac.cpp; sourceTree = "<group>"; };
                E4D687780ED7AE4F006EA978 /* PurgeableBuffer.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PurgeableBuffer.h; sourceTree = "<group>"; };
+               E55F4979151B888000BB67DB /* LengthFunctions.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = LengthFunctions.cpp; sourceTree = "<group>"; };
                E5BA7D62151437CA00FE1E3F /* LengthFunctions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LengthFunctions.h; sourceTree = "<group>"; };
                ED2BA83B09A24B91006C0AC4 /* DocumentMarker.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DocumentMarker.h; sourceTree = "<group>"; };
                ED501DC50B249F2900AE18D9 /* EditorMac.mm */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.objcpp; name = EditorMac.mm; path = mac/EditorMac.mm; sourceTree = "<group>"; };
                                CD4E0AFA11F7BC27009D3811 /* fullscreen.css */,
                                CDBD93BA1333BD4B002570E3 /* fullscreenQuickTime.css */,
                                93CA4C9909DF93FA00DF8677 /* html.css */,
+                               E55F4979151B888000BB67DB /* LengthFunctions.cpp */,
                                E5BA7D62151437CA00FE1E3F /* LengthFunctions.h */,
                                93CA4C9A09DF93FA00DF8677 /* make-css-file-arrays.pl */,
                                93CA4C9B09DF93FA00DF8677 /* makeprop.pl */,
                                BCE93F471517C6D5008CCF74 /* RenderRegionSet.cpp in Sources */,
                                9393E5FF151A99F200066F06 /* CSSImageSetValue.cpp in Sources */,
                                9393E604151A9A1800066F06 /* StyleCachedImageSet.cpp in Sources */,
+                               E55F497A151B888000BB67DB /* LengthFunctions.cpp in Sources */,
                        );
                        runOnlyForDeploymentPostprocessing = 0;
                };
diff --git a/Source/WebCore/css/LengthFunctions.cpp b/Source/WebCore/css/LengthFunctions.cpp
new file mode 100644 (file)
index 0000000..fe4d3e9
--- /dev/null
@@ -0,0 +1,121 @@
+/*
+    Copyright (C) 1999 Lars Knoll (knoll@kde.org)
+    Copyright (C) 2006, 2008 Apple Inc. All rights reserved.
+    Copyright (C) 2011 Rik Cabanier (cabanier@adobe.com)
+    Copyright (C) 2011 Adobe Systems Incorporated. All rights reserved.
+    Copyright (C) 2012 Motorola Mobility, Inc. All rights reserved.
+
+    This library is free software; you can redistribute it and/or
+    modify it under the terms of the GNU Library General Public
+    License as published by the Free Software Foundation; either
+    version 2 of the License, or (at your option) any later version.
+
+    This library is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+    Library General Public License for more details.
+
+    You should have received a copy of the GNU Library General Public License
+    along with this library; see the file COPYING.LIB.  If not, write to
+    the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+    Boston, MA 02110-1301, USA.
+*/
+
+#include "config.h"
+#include "LengthFunctions.h"
+
+#include "Length.h"
+
+namespace WebCore {
+
+int miminumValueForLength(Length length, int maximumValue, bool roundPercentages)
+{
+    switch (length.type()) {
+    case Fixed:
+        return length.value();
+    case Percent:
+        if (roundPercentages)
+            return static_cast<int>(round(maximumValue * length.percent() / 100.0f));
+        // Don't remove the extra cast to float. It is needed for rounding on 32-bit Intel machines that use the FPU stack.
+        return static_cast<int>(static_cast<float>(maximumValue * length.percent() / 100.0f));
+    case Calculated:
+        return length.nonNanCalculatedValue(maximumValue);
+    case Auto:
+        return 0;
+    case Relative:
+    case Intrinsic:
+    case MinIntrinsic:
+    case Undefined:
+        ASSERT_NOT_REACHED();
+        return 0;
+    }
+    ASSERT_NOT_REACHED();
+    return 0;
+}
+
+int valueForLength(Length length, int maximumValue, bool roundPercentages)
+{
+    switch (length.type()) {
+    case Fixed:
+    case Percent:
+    case Calculated:
+        return miminumValueForLength(length, maximumValue, roundPercentages);
+    case Auto:
+        return maximumValue;
+    case Relative:
+    case Intrinsic:
+    case MinIntrinsic:
+    case Undefined:
+        ASSERT_NOT_REACHED();
+        return 0;
+    }
+    ASSERT_NOT_REACHED();
+    return 0;
+}
+
+// FIXME: when subpixel layout is supported this copy of floatValueForLength() can be removed. See bug 71143.
+float floatValueForLength(Length length, int maximumValue)
+{
+    switch (length.type()) {
+    case Fixed:
+        return length.getFloatValue();
+    case Percent:
+        return static_cast<float>(maximumValue * length.percent() / 100.0f);
+    case Auto:
+        return static_cast<float>(maximumValue);
+    case Calculated:
+        return length.nonNanCalculatedValue(maximumValue);                
+    case Relative:
+    case Intrinsic:
+    case MinIntrinsic:
+    case Undefined:
+        ASSERT_NOT_REACHED();
+        return 0;
+    }
+    ASSERT_NOT_REACHED();
+    return 0;
+}
+
+float floatValueForLength(Length length, float maximumValue)
+{
+    switch (length.type()) {
+    case Fixed:
+        return length.getFloatValue();
+    case Percent:
+        return static_cast<float>(maximumValue * length.percent() / 100.0f);
+    case Auto:
+        return static_cast<float>(maximumValue);
+    case Calculated:
+        return length.nonNanCalculatedValue(maximumValue);
+    case Relative:
+    case Intrinsic:
+    case MinIntrinsic:
+    case Undefined:
+        ASSERT_NOT_REACHED();
+        return 0;
+    }
+    ASSERT_NOT_REACHED();
+    return 0;
+}
+
+} // namespace WebCore
index 425e3c906d7cae18e5d44c3bbdf232e5ab42ef2f..25c3933b5d7ddf3b36a3e473f6f07c3f38c97711 100644 (file)
 #ifndef LengthFunctions_h
 #define LengthFunctions_h
 
-#include "Length.h"
-
 namespace WebCore {
 
-inline int miminumValueForLength(Length length, int maximumValue, bool roundPercentages = false)
-{
-    switch (length.type()) {
-    case Fixed:
-        return length.value();
-    case Percent:
-        if (roundPercentages)
-            return static_cast<int>(round(maximumValue * length.percent() / 100.0f));
-        // Don't remove the extra cast to float. It is needed for rounding on 32-bit Intel machines that use the FPU stack.
-        return static_cast<int>(static_cast<float>(maximumValue * length.percent() / 100.0f));
-    case Calculated:
-        return length.nonNanCalculatedValue(maximumValue);
-    case Auto:
-        return 0;
-    case Relative:
-    case Intrinsic:
-    case MinIntrinsic:
-    case Undefined:
-        ASSERT_NOT_REACHED();
-        return 0;
-    }
-    ASSERT_NOT_REACHED();
-    return 0;
-}
-
-inline int valueForLength(Length length, int maximumValue, bool roundPercentages = false)
-{
-    switch (length.type()) {
-    case Fixed:
-    case Percent:
-    case Calculated:
-        return miminumValueForLength(length, maximumValue, roundPercentages);
-    case Auto:
-        return maximumValue;
-    case Relative:
-    case Intrinsic:
-    case MinIntrinsic:
-    case Undefined:
-        ASSERT_NOT_REACHED();
-        return 0;
-    }
-    ASSERT_NOT_REACHED();
-    return 0;
-}
-
-// FIXME: when subpixel layout is supported this copy of floatValueForLength() can be removed. See bug 71143.
-inline float floatValueForLength(Length length, int maximumValue)
-{
-    switch (length.type()) {
-    case Fixed:
-        return length.getFloatValue();
-    case Percent:
-        return static_cast<float>(maximumValue * length.percent() / 100.0f);
-    case Auto:
-        return static_cast<float>(maximumValue);
-    case Calculated:
-        return length.nonNanCalculatedValue(maximumValue);                
-    case Relative:
-    case Intrinsic:
-    case MinIntrinsic:
-    case Undefined:
-        ASSERT_NOT_REACHED();
-        return 0;
-    }
-    ASSERT_NOT_REACHED();
-    return 0;
-}
+struct Length;
 
-inline float floatValueForLength(Length length, float maximumValue)
-{
-    switch (length.type()) {
-    case Fixed:
-        return length.getFloatValue();
-    case Percent:
-        return static_cast<float>(maximumValue * length.percent() / 100.0f);
-    case Auto:
-        return static_cast<float>(maximumValue);
-    case Calculated:
-        return length.nonNanCalculatedValue(maximumValue);
-    case Relative:
-    case Intrinsic:
-    case MinIntrinsic:
-    case Undefined:
-        ASSERT_NOT_REACHED();
-        return 0;
-    }
-    ASSERT_NOT_REACHED();
-    return 0;
-}
+int miminumValueForLength(Length, int maximumValue, bool roundPercentages = false);
+int valueForLength(Length, int maximumValue, bool roundPercentages = false);
+float floatValueForLength(Length, int maximumValue);
+float floatValueForLength(Length, float maximumValue);
 
 } // namespace WebCore