Ensure no intermediate WTF::Strings are created when concatenating with string literals
authoraroben@apple.com <aroben@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Jul 2011 13:35:57 +0000 (13:35 +0000)
committeraroben@apple.com <aroben@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Jul 2011 13:35:57 +0000 (13:35 +0000)
Fixes <http://webkit.org/b/63330> Concatenating string literals and WTF::Strings using
operator+ is suboptimal

Reviewed by Darin Adler.

Source/JavaScriptCore:

* wtf/text/StringConcatenate.h:
(WTF::StringTypeAdapter<String>::writeTo): Added a macro that can be used for testing how
many WTF::Strings get copied while evaluating an operator+ expression.

* wtf/text/StringOperators.h:
(WTF::operator+): Changed the overload that takes a StringAppend to take it on the left-hand
side, since operator+ is left-associative. Having the StringAppend on the right-hand side
was causing us to make intermediate WTF::Strings when evaluating expressions that contained
multiple calls to operator+. Added some more overloads for that take a left-hand side of
const char* to resolve overload ambiguity for certain expressions. Added overloads that take
a left-hand side of const UChar* (matching the const char* overloads) so that wide string
literals don't first have to be converted to a WTF::String in operator+ expressions.

Source/WebKit2:

Export some symbols needed by TestWebKitAPI

* win/WebKit2.def:

Tools:

Test that no intermediate WTF::Strings are created when concatenating with string literals

* TestWebKitAPI/Tests/WTF/StringOperators.cpp: Added.
(TestWebKitAPI::TEST): Test that a bunch of different string concatenation expressions don't
create any intermediate WTF::Strings while they're being evaluated.

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/win/TestWebKitAPI.vcproj:
Added new file.

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/wtf/text/StringConcatenate.h
Source/JavaScriptCore/wtf/text/StringOperators.h
Source/WebKit2/ChangeLog
Source/WebKit2/win/WebKit2.def
Tools/ChangeLog
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/WTF/StringOperators.cpp [new file with mode: 0644]
Tools/TestWebKitAPI/win/TestWebKitAPI.vcproj

index a922890..8389f0d 100644 (file)
@@ -1,5 +1,27 @@
 2011-07-12  Adam Roben  <aroben@apple.com>
 
+        Ensure no intermediate WTF::Strings are created when concatenating with string literals
+
+        Fixes <http://webkit.org/b/63330> Concatenating string literals and WTF::Strings using
+        operator+ is suboptimal
+
+        Reviewed by Darin Adler.
+
+        * wtf/text/StringConcatenate.h:
+        (WTF::StringTypeAdapter<String>::writeTo): Added a macro that can be used for testing how
+        many WTF::Strings get copied while evaluating an operator+ expression.
+
+        * wtf/text/StringOperators.h:
+        (WTF::operator+): Changed the overload that takes a StringAppend to take it on the left-hand
+        side, since operator+ is left-associative. Having the StringAppend on the right-hand side
+        was causing us to make intermediate WTF::Strings when evaluating expressions that contained
+        multiple calls to operator+. Added some more overloads for that take a left-hand side of
+        const char* to resolve overload ambiguity for certain expressions. Added overloads that take
+        a left-hand side of const UChar* (matching the const char* overloads) so that wide string
+        literals don't first have to be converted to a WTF::String in operator+ expressions.
+
+2011-07-12  Adam Roben  <aroben@apple.com>
+
         Unreviewed, rolling out r90811.
         http://trac.webkit.org/changeset/90811
         https://bugs.webkit.org/show_bug.cgi?id=61025
index e8c6331..d3f2376 100644 (file)
 #include "AtomicString.h"
 #endif
 
+// This macro is helpful for testing how many intermediate Strings are created while evaluating an
+// expression containing operator+.
+#ifndef WTF_STRINGTYPEADAPTER_COPIED_WTF_STRING
+#define WTF_STRINGTYPEADAPTER_COPIED_WTF_STRING() ((void)0)
+#endif
+
 namespace WTF {
 
 template<typename StringType>
@@ -180,6 +186,8 @@ public:
         unsigned length = m_buffer.length();
         for (unsigned i = 0; i < length; ++i)
             destination[i] = data[i];
+
+        WTF_STRINGTYPEADAPTER_COPIED_WTF_STRING();
     }
 
 private:
index e8c2181..cf7aa42 100644 (file)
@@ -91,6 +91,28 @@ inline StringAppend<const char*, AtomicString> operator+(const char* string1, co
     return StringAppend<const char*, AtomicString>(string1, string2);
 }
 
+template<typename U, typename V>
+StringAppend<const char*, StringAppend<U, V> > operator+(const char* string1, const StringAppend<U, V>& string2)
+{
+    return StringAppend<const char*, StringAppend<U, V> >(string1, string2);
+}
+
+inline StringAppend<const UChar*, String> operator+(const UChar* string1, const String& string2)
+{
+    return StringAppend<const UChar*, String>(string1, string2);
+}
+
+inline StringAppend<const UChar*, AtomicString> operator+(const UChar* string1, const AtomicString& string2)
+{
+    return StringAppend<const UChar*, AtomicString>(string1, string2);
+}
+
+template<typename U, typename V>
+StringAppend<const UChar*, StringAppend<U, V> > operator+(const UChar* string1, const StringAppend<U, V>& string2)
+{
+    return StringAppend<const UChar*, StringAppend<U, V> >(string1, string2);
+}
+
 template<typename T>
 StringAppend<String, T> operator+(const String& string1, T string2)
 {
@@ -98,9 +120,9 @@ StringAppend<String, T> operator+(const String& string1, T string2)
 }
 
 template<typename U, typename V, typename W>
-StringAppend<U, StringAppend<V, W> > operator+(U string1, const StringAppend<V, W>& string2)
+StringAppend<StringAppend<U, V>, W> operator+(const StringAppend<U, V>& string1, W string2)
 {
-    return StringAppend<U, StringAppend<V, W> >(string1, string2);
+    return StringAppend<StringAppend<U, V>, W>(string1, string2);
 }
 
 } // namespace WTF
index b76f260..823cc26 100644 (file)
@@ -1,3 +1,14 @@
+2011-07-12  Adam Roben  <aroben@apple.com>
+
+        Export some symbols needed by TestWebKitAPI
+
+        Part of <http://webkit.org/b/63330> Concatenating string literals and WTF::Strings using
+        operator+ is suboptimal
+
+        Reviewed by Darin Adler.
+
+        * win/WebKit2.def:
+
 2011-07-11  Hironori Bono  <hbono@chromium.org>
 
         Reviewed by Adam Roben.
index 1967fb0..6181dde 100644 (file)
@@ -134,7 +134,10 @@ EXPORTS
         ?waitForThreadCompletion@WTF@@YAHIPAPAX@Z
         ?createThread@WTF@@YAIP6APAXPAX@Z0@Z
 
-        ; Re-exports from WebCore for WebCoreTestSupport
+        ; Re-exports from WebCore for test harnesses
+        ??0String@WTF@@QAE@PBD@Z
+        ??0String@WTF@@QAE@PB_W@Z
+        ?add@AtomicString@WTF@@CA?AV?$PassRefPtr@VStringImpl@WTF@@@2@PBD@Z
         ?addSlowCase@AtomicString@WTF@@CA?AV?$PassRefPtr@VStringImpl@WTF@@@2@PAVStringImpl@2@@Z
         ?cacheDOMStructure@WebCore@@YAPAVStructure@JSC@@PAVJSDOMGlobalObject@1@PAV23@PBUClassInfo@3@@Z
         ?create@ShadowContentElement@WebCore@@SA?AV?$PassRefPtr@VShadowContentElement@WebCore@@@WTF@@PAVDocument@2@@Z
index 880c6ab..2c9e517 100644 (file)
@@ -1,3 +1,20 @@
+2011-07-12  Adam Roben  <aroben@apple.com>
+
+        Test that no intermediate WTF::Strings are created when concatenating with string literals
+
+        Test for <http://webkit.org/b/63330> Concatenating string literals and WTF::Strings using
+        operator+ is suboptimal
+
+        Reviewed by Darin Adler.
+
+        * TestWebKitAPI/Tests/WTF/StringOperators.cpp: Added.
+        (TestWebKitAPI::TEST): Test that a bunch of different string concatenation expressions don't
+        create any intermediate WTF::Strings while they're being evaluated.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/win/TestWebKitAPI.vcproj:
+        Added new file.
+
 2011-07-12  Eric Seidel  <eric@webkit.org>
 
         [Qt] NRWT should pick up the right httpd config file
index d612bae..d1bc87f 100644 (file)
@@ -53,6 +53,7 @@
                BCBD3737125ABBEB00D2C29F /* icon.png in Copy Resources */ = {isa = PBXBuildFile; fileRef = BCBD372E125ABBE600D2C29F /* icon.png */; };
                BCBD3761125ABCFE00D2C29F /* FrameMIMETypePNG.cpp in Sources */ = {isa = PBXBuildFile; fileRef = BCBD3760125ABCFE00D2C29F /* FrameMIMETypePNG.cpp */; };
                BCC8B95B12611F4700DE46A4 /* FailedLoad.cpp in Sources */ = {isa = PBXBuildFile; fileRef = BCC8B95A12611F4700DE46A4 /* FailedLoad.cpp */; };
+               C01363C813C3997300EF3964 /* StringOperators.cpp in Sources */ = {isa = PBXBuildFile; fileRef = C01363C713C3997300EF3964 /* StringOperators.cpp */; };
                C01A23F21266156700C9ED55 /* spacebar-scrolling.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = C02B7882126615410026BF0F /* spacebar-scrolling.html */; };
                C02B77F2126612140026BF0F /* SpacebarScrolling.cpp in Sources */ = {isa = PBXBuildFile; fileRef = C02B77F1126612140026BF0F /* SpacebarScrolling.cpp */; };
                C02B7854126613AE0026BF0F /* Carbon.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = C02B7853126613AE0026BF0F /* Carbon.framework */; };
                BCBD3760125ABCFE00D2C29F /* FrameMIMETypePNG.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = FrameMIMETypePNG.cpp; sourceTree = "<group>"; };
                BCC8B95A12611F4700DE46A4 /* FailedLoad.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = FailedLoad.cpp; sourceTree = "<group>"; };
                BCF0F9391381C25B008361AC /* CompilerVersion.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; path = CompilerVersion.xcconfig; sourceTree = "<group>"; };
+               C01363C713C3997300EF3964 /* StringOperators.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = StringOperators.cpp; path = WTF/StringOperators.cpp; sourceTree = "<group>"; };
                C02B77F1126612140026BF0F /* SpacebarScrolling.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SpacebarScrolling.cpp; sourceTree = "<group>"; };
                C02B7853126613AE0026BF0F /* Carbon.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; name = Carbon.framework; sourceTree = SDKROOT; };
                C02B7882126615410026BF0F /* spacebar-scrolling.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "spacebar-scrolling.html"; sourceTree = "<group>"; };
                BC9096461255618900083756 /* WTF */ = {
                        isa = PBXGroup;
                        children = (
+                               C01363C713C3997300EF3964 /* StringOperators.cpp */,
                                BC90964B125561BF00083756 /* VectorBasic.cpp */,
                                37200B9113A16230007A4FAD /* VectorReverse.cpp */,
                        );
                                33BE5AF5137B5A6C00705813 /* MouseMoveAfterCrash.cpp in Sources */,
                                C045F9451385C2EA00C0F3CD /* DownloadDecideDestinationCrash.cpp in Sources */,
                                37200B9213A16230007A4FAD /* VectorReverse.cpp in Sources */,
+                               C01363C813C3997300EF3964 /* StringOperators.cpp in Sources */,
                        );
                        runOnlyForDeploymentPostprocessing = 0;
                };
diff --git a/Tools/TestWebKitAPI/Tests/WTF/StringOperators.cpp b/Tools/TestWebKitAPI/Tests/WTF/StringOperators.cpp
new file mode 100644 (file)
index 0000000..2329d34
--- /dev/null
@@ -0,0 +1,126 @@
+/*
+ * Copyright (C) 2011 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "Test.h"
+
+#define JS_EXPORTDATA
+#define WTF_STRINGTYPEADAPTER_COPIED_WTF_STRING() (++wtfStringCopyCount)
+
+static int wtfStringCopyCount;
+
+#include <JavaScriptCore/WTFString.h>
+
+namespace TestWebKitAPI {
+
+#define EXPECT_N_WTF_STRING_COPIES(count, expr) \
+    do { \
+        wtfStringCopyCount = 0; \
+        String __testString = expr; \
+        (void)__testString; \
+        EXPECT_EQ(count, wtfStringCopyCount) << #expr; \
+    } while (false)
+
+TEST(WTF, StringOperators)
+{
+    String string("String");
+    AtomicString atomicString("AtomicString");
+
+    EXPECT_EQ(0, wtfStringCopyCount);
+
+    EXPECT_N_WTF_STRING_COPIES(2, string + string);
+    EXPECT_N_WTF_STRING_COPIES(2, string + atomicString);
+    EXPECT_N_WTF_STRING_COPIES(2, atomicString + string);
+    EXPECT_N_WTF_STRING_COPIES(2, atomicString + atomicString);
+
+    EXPECT_N_WTF_STRING_COPIES(1, "C string" + string);
+    EXPECT_N_WTF_STRING_COPIES(1, string + "C string");
+    EXPECT_N_WTF_STRING_COPIES(1, "C string" + atomicString);
+    EXPECT_N_WTF_STRING_COPIES(1, atomicString + "C string");
+
+    EXPECT_N_WTF_STRING_COPIES(2, "C string" + string + "C string" + string);
+    EXPECT_N_WTF_STRING_COPIES(2, "C string" + (string + "C string" + string));
+    EXPECT_N_WTF_STRING_COPIES(2, ("C string" + string) + ("C string" + string));
+    EXPECT_N_WTF_STRING_COPIES(2, string + "C string" + string + "C string");
+    EXPECT_N_WTF_STRING_COPIES(2, string + ("C string" + string + "C string"));
+    EXPECT_N_WTF_STRING_COPIES(2, (string + "C string") + (string + "C string"));
+
+    EXPECT_N_WTF_STRING_COPIES(2, "C string" + atomicString + "C string" + atomicString);
+    EXPECT_N_WTF_STRING_COPIES(2, "C string" + (atomicString + "C string" + atomicString));
+    EXPECT_N_WTF_STRING_COPIES(2, ("C string" + atomicString) + ("C string" + atomicString));
+    EXPECT_N_WTF_STRING_COPIES(2, atomicString + "C string" + atomicString + "C string");
+    EXPECT_N_WTF_STRING_COPIES(2, atomicString + ("C string" + atomicString + "C string"));
+    EXPECT_N_WTF_STRING_COPIES(2, (atomicString + "C string") + (atomicString + "C string"));
+
+    EXPECT_N_WTF_STRING_COPIES(2, "C string" + string + "C string" + atomicString);
+    EXPECT_N_WTF_STRING_COPIES(2, "C string" + (string + "C string" + atomicString));
+    EXPECT_N_WTF_STRING_COPIES(2, ("C string" + string) + ("C string" + atomicString));
+    EXPECT_N_WTF_STRING_COPIES(2, string + "C string" + atomicString + "C string");
+    EXPECT_N_WTF_STRING_COPIES(2, string + ("C string" + atomicString + "C string"));
+    EXPECT_N_WTF_STRING_COPIES(2, (string + "C string") + (atomicString + "C string"));
+
+    EXPECT_N_WTF_STRING_COPIES(2, "C string" + atomicString + "C string" + string);
+    EXPECT_N_WTF_STRING_COPIES(2, "C string" + (atomicString + "C string" + string));
+    EXPECT_N_WTF_STRING_COPIES(2, ("C string" + atomicString) + ("C string" + string));
+    EXPECT_N_WTF_STRING_COPIES(2, atomicString + "C string" + string + "C string");
+    EXPECT_N_WTF_STRING_COPIES(2, atomicString + ("C string" + string + "C string"));
+    EXPECT_N_WTF_STRING_COPIES(2, (atomicString + "C string") + (string + "C string"));
+
+#if COMPILER(MSVC)
+    EXPECT_N_WTF_STRING_COPIES(1, L"wide string" + string);
+    EXPECT_N_WTF_STRING_COPIES(1, string + L"wide string");
+    EXPECT_N_WTF_STRING_COPIES(1, L"wide string" + atomicString);
+    EXPECT_N_WTF_STRING_COPIES(1, atomicString + L"wide string");
+
+    EXPECT_N_WTF_STRING_COPIES(2, L"wide string" + string + L"wide string" + string);
+    EXPECT_N_WTF_STRING_COPIES(2, L"wide string" + (string + L"wide string" + string));
+    EXPECT_N_WTF_STRING_COPIES(2, (L"wide string" + string) + (L"wide string" + string));
+    EXPECT_N_WTF_STRING_COPIES(2, string + L"wide string" + string + L"wide string");
+    EXPECT_N_WTF_STRING_COPIES(2, string + (L"wide string" + string + L"wide string"));
+    EXPECT_N_WTF_STRING_COPIES(2, (string + L"wide string") + (string + L"wide string"));
+
+    EXPECT_N_WTF_STRING_COPIES(2, L"wide string" + atomicString + L"wide string" + atomicString);
+    EXPECT_N_WTF_STRING_COPIES(2, L"wide string" + (atomicString + L"wide string" + atomicString));
+    EXPECT_N_WTF_STRING_COPIES(2, (L"wide string" + atomicString) + (L"wide string" + atomicString));
+    EXPECT_N_WTF_STRING_COPIES(2, atomicString + L"wide string" + atomicString + L"wide string");
+    EXPECT_N_WTF_STRING_COPIES(2, atomicString + (L"wide string" + atomicString + L"wide string"));
+    EXPECT_N_WTF_STRING_COPIES(2, (atomicString + L"wide string") + (atomicString + L"wide string"));
+
+    EXPECT_N_WTF_STRING_COPIES(2, L"wide string" + string + L"wide string" + atomicString);
+    EXPECT_N_WTF_STRING_COPIES(2, L"wide string" + (string + L"wide string" + atomicString));
+    EXPECT_N_WTF_STRING_COPIES(2, (L"wide string" + string) + (L"wide string" + atomicString));
+    EXPECT_N_WTF_STRING_COPIES(2, string + L"wide string" + atomicString + L"wide string");
+    EXPECT_N_WTF_STRING_COPIES(2, string + (L"wide string" + atomicString + L"wide string"));
+    EXPECT_N_WTF_STRING_COPIES(2, (string + L"wide string") + (atomicString + L"wide string"));
+
+    EXPECT_N_WTF_STRING_COPIES(2, L"wide string" + atomicString + L"wide string" + string);
+    EXPECT_N_WTF_STRING_COPIES(2, L"wide string" + (atomicString + L"wide string" + string));
+    EXPECT_N_WTF_STRING_COPIES(2, (L"wide string" + atomicString) + (L"wide string" + string));
+    EXPECT_N_WTF_STRING_COPIES(2, atomicString + L"wide string" + string + L"wide string");
+    EXPECT_N_WTF_STRING_COPIES(2, atomicString + (L"wide string" + string + L"wide string"));
+    EXPECT_N_WTF_STRING_COPIES(2, (atomicString + L"wide string") + (string + L"wide string"));
+#endif
+}
+
+} // namespace TestWebKitAPI
index 2089b59..0a288b0 100644 (file)
                                Name="WTF"
                                >
                                <File
+                                       RelativePath="..\Tests\WTF\StringOperators.cpp"
+                                       >
+                               </File>
+                               <File
                                        RelativePath="..\Tests\WTF\VectorBasic.cpp"
                                        >
                                </File>