2011-05-12 Nikolas Zimmermann <nzimmermann@rim.com>
authorzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 May 2011 12:07:24 +0000 (12:07 +0000)
committerzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 May 2011 12:07:24 +0000 (12:07 +0000)
        Reviewed by Darin Adler.

        String operator+ reallocates unnecessarily when concatting > 2 strings
        https://bugs.webkit.org/show_bug.cgi?id=58420

        Provide a faster String append operator.
        Up until now, "String operator+(const String& a, const String& b)" copied String a into a temporary
        object, and used a.append(b), which reallocates a new buffer of aLength+bLength. When concatting
        N strings using operator+, this leads to N-1 reallocations.

        Replace this with a flexible operator+ implementation, that avoids these reallocations.
        When concatting a 'String' with any string type (char*, UChar, Vector<char>, String, AtomicString, etc..)
        a StringAppend<String, T> object is created, which holds the intermediate string objects, and delays
        creation of the final string, until operator String() is invoked.

        template<typename T>
        StringAppend<String, T> operator+(const String& string1, T string2)
        {
            return StringAppend<String, T>(string1, string2);
        }

        template<typename U, typename V, typename W>
        StringAppend<U, StringAppend<V, W> > operator+(U string1, const StringAppend<V, W>& string2)
        {
            return StringAppend<U, StringAppend<V, W> >(string1, string2);
        }

        When concatting three strings - "String a, b, c; String result = a + b + c;" following happens:
        first a StringAppend<String, String> object is created by operator+(const String& string1, String string2).
        Then operator+(String string1, const StringAppend<String, String>& string2) is invoked, which returns
        a StringAppend<String, StringAppend<String, String> > object.
        Then operator String() is invoked, which allocates a StringImpl object, once, large enough to hold the
        final string - it uses tryMakeString provided by StringConcatenate.h under the hoods, which guards us
        against too big string allocations, etc.

        Note that the second template, defines a recursive way to concat an arbitary number of strings
        into a single String with just one allocation.

        * GNUmakefile.list.am: Add StringOperators.h to build.
        * JavaScriptCore.exp: Export WTF::emptyString(). Remove no longer needed symbols.
        * JavaScriptCore.gypi: Add StringOperators.h to build.
        * JavaScriptCore.vcproj/WTF/WTF.vcproj: Ditto.
        * JavaScriptCore.xcodeproj/project.pbxproj: Ditto.
        * wtf/text/AtomicString.h: Pull in StringConcatenate.h at the end of the file.
        * wtf/text/StringConcatenate.h: Conditionally include AtomicString.h to avoid a cyclic dependency. Pull in StringOperators.h at the end of the file.
        * wtf/text/StringOperators.h: Added. This is never meant to be included directly, including either WTFString.h or AtomicString.h automatically pulls in this file.
        (WTF::StringAppend::StringAppend):
        (WTF::StringAppend::operator String):
        (WTF::StringAppend::operator AtomicString):
        (WTF::StringAppend::writeTo):
        (WTF::StringAppend::length):
        (WTF::operator+):
        * wtf/text/WTFString.cpp: Remove operator+ implementations that use String::append().
        (WTF::emptyString): Add new shared empty string free function.
        * wtf/text/WTFString.h: Replace operator+ implementations by StringAppend template solution. Pull in AtomicString.h at the end of the file.

2011-05-12  Nikolas Zimmermann  <nzimmermann@rim.com>

        Reviewed by Darin Adler.

        String operator+ reallocates unnecessary when concatting > 2 strings
        https://bugs.webkit.org/show_bug.cgi?id=58420

        Provide a faster String append operator. See Source/JavaScriptCore/ChangeLog for details.

        * dom/XMLDocumentParserLibxml2.cpp:
        (WebCore::handleElementAttributes):
        * editing/MarkupAccumulator.cpp:
        (WebCore::MarkupAccumulator::shouldAddNamespaceElement):
        * html/HTMLAnchorElement.cpp:
        (WebCore::HTMLAnchorElement::hash):
        (WebCore::HTMLAnchorElement::search):
        * html/ImageInputType.cpp:
        (WebCore::ImageInputType::appendFormData):
        * html/parser/HTMLTreeBuilder.cpp:
        * loader/CrossOriginAccessControl.cpp:
        (WebCore::passesAccessControlCheck):
        * page/Location.cpp:
        (WebCore::Location::search):
        (WebCore::Location::hash):
        * page/NavigatorBase.cpp:
        (WebCore::NavigatorBase::platform):
        * platform/chromium/ClipboardChromium.cpp:
        (WebCore::writeImageToDataObject):
        * platform/gtk/PasteboardHelper.cpp:
        (WebCore::PasteboardHelper::fillSelectionData):
        * platform/network/cf/ResourceHandleCFNet.cpp:
        (WebCore::encodeBasicAuthorization):
        * platform/network/cf/SocketStreamHandleCFNet.cpp:
        (WebCore::SocketStreamHandle::copyCFStreamDescription):
        * platform/network/mac/ResourceHandleMac.mm:
        (WebCore::encodeBasicAuthorization):
        * workers/WorkerLocation.cpp:
        (WebCore::WorkerLocation::search):
        (WebCore::WorkerLocation::hash):

2011-05-12  Nikolas Zimmermann  <nzimmermann@rim.com>

        Reviewed by Darin Adler.

        String operator+ reallocates unnecessarily when concatting > 2 strings
        https://bugs.webkit.org/show_bug.cgi?id=58420

        Provide a faster String append operator. See Source/JavaScriptCore/ChangeLog for details.

        * src/WebAccessibilityObject.cpp:
        (WebKit::WebAccessibilityObject::keyboardShortcut): Cast to String first, before trying to convert to platform dependant type.
        * src/WebHTTPLoadInfo.cpp:
        (WebKit::addHeader): Don't pass WebString to makeString, explicit cast to String first.
        * tests/IDBLevelDBCodingTest.cpp: Cast to String first, to avoid conflicting with gtests global templatified operator+.
        (IDBLevelDBCoding::TEST):

2011-05-12  Nikolas Zimmermann  <nzimmermann@rim.com>

        Reviewed by Darin Adler.

        String operator+ reallocates unnecessarily when concatting > 2 strings
        https://bugs.webkit.org/show_bug.cgi?id=58420

        Provide a faster String append operator. See Source/JavaScriptCore/ChangeLog for details.

        * WebView/WebFrame.mm: Explicitely cast to Strings first, so operator NSString*() can be invoked.
        (-[WebFrame _stringWithDocumentTypeStringAndMarkupString:]):

2011-05-12  Nikolas Zimmermann  <nzimmermann@rim.com>

        Reviewed by Darin Adler.

        String operator+ reallocates unnecessarily when concatting > 2 strings
        https://bugs.webkit.org/show_bug.cgi?id=58420

        Provide a faster String append operator. See Source/JavaScriptCore/ChangeLog for details.

        * AccessibleBase.cpp:
        (AccessibleBase::get_accKeyboardShortcut): Explicitely cast to Strings first, so operator BString() can be invoked.

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

34 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/GNUmakefile.list.am
Source/JavaScriptCore/JavaScriptCore.exp
Source/JavaScriptCore/JavaScriptCore.gypi
Source/JavaScriptCore/JavaScriptCore.vcproj/WTF/WTF.vcproj
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/JavaScriptCore/wtf/text/AtomicString.h
Source/JavaScriptCore/wtf/text/StringConcatenate.h
Source/JavaScriptCore/wtf/text/StringOperators.h [new file with mode: 0644]
Source/JavaScriptCore/wtf/text/WTFString.cpp
Source/JavaScriptCore/wtf/text/WTFString.h
Source/WebCore/ChangeLog
Source/WebCore/dom/XMLDocumentParserLibxml2.cpp
Source/WebCore/editing/MarkupAccumulator.cpp
Source/WebCore/html/HTMLAnchorElement.cpp
Source/WebCore/html/ImageInputType.cpp
Source/WebCore/html/parser/HTMLTreeBuilder.cpp
Source/WebCore/loader/CrossOriginAccessControl.cpp
Source/WebCore/page/Location.cpp
Source/WebCore/page/NavigatorBase.cpp
Source/WebCore/platform/chromium/ClipboardChromium.cpp
Source/WebCore/platform/gtk/PasteboardHelper.cpp
Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp
Source/WebCore/platform/network/cf/SocketStreamHandleCFNet.cpp
Source/WebCore/platform/network/mac/ResourceHandleMac.mm
Source/WebCore/workers/WorkerLocation.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/WebAccessibilityObject.cpp
Source/WebKit/chromium/src/WebHTTPLoadInfo.cpp
Source/WebKit/chromium/tests/IDBLevelDBCodingTest.cpp
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebFrame.mm
Source/WebKit/win/AccessibleBase.cpp
Source/WebKit/win/ChangeLog

index f17d286..b5b6c5d 100644 (file)
@@ -1,3 +1,61 @@
+2011-05-12  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        Reviewed by Darin Adler.
+
+        String operator+ reallocates unnecessarily when concatting > 2 strings
+        https://bugs.webkit.org/show_bug.cgi?id=58420
+
+        Provide a faster String append operator.
+        Up until now, "String operator+(const String& a, const String& b)" copied String a into a temporary
+        object, and used a.append(b), which reallocates a new buffer of aLength+bLength. When concatting
+        N strings using operator+, this leads to N-1 reallocations.
+
+        Replace this with a flexible operator+ implementation, that avoids these reallocations.
+        When concatting a 'String' with any string type (char*, UChar, Vector<char>, String, AtomicString, etc..)
+        a StringAppend<String, T> object is created, which holds the intermediate string objects, and delays
+        creation of the final string, until operator String() is invoked.
+
+        template<typename T>
+        StringAppend<String, T> operator+(const String& string1, T string2)
+        {
+            return StringAppend<String, T>(string1, string2);
+        }
+
+        template<typename U, typename V, typename W>
+        StringAppend<U, StringAppend<V, W> > operator+(U string1, const StringAppend<V, W>& string2)
+        {
+            return StringAppend<U, StringAppend<V, W> >(string1, string2);
+        }
+
+        When concatting three strings - "String a, b, c; String result = a + b + c;" following happens:
+        first a StringAppend<String, String> object is created by operator+(const String& string1, String string2).
+        Then operator+(String string1, const StringAppend<String, String>& string2) is invoked, which returns
+        a StringAppend<String, StringAppend<String, String> > object.
+        Then operator String() is invoked, which allocates a StringImpl object, once, large enough to hold the
+        final string - it uses tryMakeString provided by StringConcatenate.h under the hoods, which guards us
+        against too big string allocations, etc.
+
+        Note that the second template, defines a recursive way to concat an arbitary number of strings
+        into a single String with just one allocation.
+
+        * GNUmakefile.list.am: Add StringOperators.h to build.
+        * JavaScriptCore.exp: Export WTF::emptyString(). Remove no longer needed symbols.
+        * JavaScriptCore.gypi: Add StringOperators.h to build.
+        * JavaScriptCore.vcproj/WTF/WTF.vcproj: Ditto.
+        * JavaScriptCore.xcodeproj/project.pbxproj: Ditto.
+        * wtf/text/AtomicString.h: Pull in StringConcatenate.h at the end of the file.
+        * wtf/text/StringConcatenate.h: Conditionally include AtomicString.h to avoid a cyclic dependency. Pull in StringOperators.h at the end of the file.
+        * wtf/text/StringOperators.h: Added. This is never meant to be included directly, including either WTFString.h or AtomicString.h automatically pulls in this file.
+        (WTF::StringAppend::StringAppend):
+        (WTF::StringAppend::operator String):
+        (WTF::StringAppend::operator AtomicString):
+        (WTF::StringAppend::writeTo):
+        (WTF::StringAppend::length):
+        (WTF::operator+):
+        * wtf/text/WTFString.cpp: Remove operator+ implementations that use String::append(). 
+        (WTF::emptyString): Add new shared empty string free function.
+        * wtf/text/WTFString.h: Replace operator+ implementations by StringAppend template solution. Pull in AtomicString.h at the end of the file.
+
 2011-05-12  Philippe Normand  <pnormand@igalia.com>
 
         Unreviewed, GTK build fix.
index 35b2c42..4143cf4 100644 (file)
@@ -525,6 +525,7 @@ javascriptcore_sources += \
        Source/JavaScriptCore/wtf/text/StringImplBase.h \
        Source/JavaScriptCore/wtf/text/StringImpl.cpp \
        Source/JavaScriptCore/wtf/text/StringImpl.h \
+       Source/JavaScriptCore/wtf/text/StringOperators.h \
        Source/JavaScriptCore/wtf/text/StringStatics.cpp \
        Source/JavaScriptCore/wtf/text/TextPosition.h \
        Source/JavaScriptCore/wtf/text/WTFString.cpp \
index 9aceb94..ca7507e 100644 (file)
@@ -372,6 +372,7 @@ __ZN3WTF10fastStrDupEPKc
 __ZN3WTF11OSAllocator16reserveAndCommitEmNS0_5UsageEbb
 __ZN3WTF11OSAllocator18releaseDecommittedEPvm
 __ZN3WTF11commentAtomE
+__ZN3WTF11emptyStringEv
 __ZN3WTF11currentTimeEv
 __ZN3WTF11dtoaRoundDPEPcdiRbRiRj
 __ZN3WTF11dtoaRoundSFEPcdiRbRiRj
@@ -510,9 +511,6 @@ __ZN3WTF9xmlnsAtomE
 __ZN3WTFeqERKNS_12AtomicStringEPKc
 __ZN3WTFeqERKNS_12AtomicStringERKNS_6VectorItLm0EEE
 __ZN3WTFeqERKNS_7CStringES2_
-__ZN3WTFplEPKcRKNS_6StringE
-__ZN3WTFplERKNS_6StringEPKc
-__ZN3WTFplERKNS_6StringES2_
 __ZNK3JSC10JSFunction23isHostFunctionNonInlineEv
 __ZNK3JSC11Interpreter14retrieveCallerEPNS_9ExecStateEPNS_10JSFunctionE
 __ZNK3JSC11Interpreter18retrieveLastCallerEPNS_9ExecStateERiRlRNS_7UStringERNS_7JSValueE
index 9cbe9c7..bae0129 100644 (file)
             'wtf/text/StringHash.h',
             'wtf/text/StringImpl.h',
             'wtf/text/StringImplBase.h',
+            'wtf/text/StringOperators.h',
             'wtf/text/TextPosition.h',
             'wtf/text/WTFString.h',
             'wtf/unicode/CharacterNames.h',
index 5a96299..5a62dce 100644 (file)
                                >
                        </File>
                        <File
+                               RelativePath="..\..\wtf\text\StringOperators.h"
+                               >
+                       </File>
+                       <File
                                RelativePath="..\..\wtf\text\StringStatics.cpp"
                                >
                        </File>
index 91d66f3..06d1e8e 100644 (file)
                65E1A3DF122B894500B26097 /* NonCopyingSort.h in Headers */ = {isa = PBXBuildFile; fileRef = 65E1A2F4122B880D00B26097 /* NonCopyingSort.h */; settings = {ATTRIBUTES = (Private, ); }; };
                65FDE49C0BDD1D4A00E80111 /* Assertions.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 65E217B808E7EECC0023E5F6 /* Assertions.cpp */; };
                7186A6EC13100BA5004479E1 /* HexNumber.h in Headers */ = {isa = PBXBuildFile; fileRef = 7186A6E813100B57004479E1 /* HexNumber.h */; settings = {ATTRIBUTES = (Private, ); }; };
+               71DA9D82134F3F3D00B767E7 /* StringOperators.h in Headers */ = {isa = PBXBuildFile; fileRef = 718A8482134F3A1200B87529 /* StringOperators.h */; settings = {ATTRIBUTES = (Private, ); }; };
                76FB9F0F12E851860051A2EB /* SHA1.h in Headers */ = {isa = PBXBuildFile; fileRef = 76FB9F0E12E851860051A2EB /* SHA1.h */; settings = {ATTRIBUTES = (Private, ); }; };
                76FB9F1112E851960051A2EB /* SHA1.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 76FB9F1012E851960051A2EB /* SHA1.cpp */; };
                7934BB7B1361979300CB99A1 /* ParallelJobs.h in Headers */ = {isa = PBXBuildFile; fileRef = 7934BB761361979300CB99A1 /* ParallelJobs.h */; settings = {ATTRIBUTES = (Private, ); }; };
                65EA73630BAE35D1001BB560 /* CommonIdentifiers.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = CommonIdentifiers.h; sourceTree = "<group>"; };
                704FD35305697E6D003DBED9 /* BooleanObject.h */ = {isa = PBXFileReference; fileEncoding = 30; indentWidth = 4; lastKnownFileType = sourcecode.c.h; path = BooleanObject.h; sourceTree = "<group>"; tabWidth = 8; };
                7186A6E813100B57004479E1 /* HexNumber.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HexNumber.h; sourceTree = "<group>"; };
+               718A8482134F3A1200B87529 /* StringOperators.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = StringOperators.h; path = text/StringOperators.h; sourceTree = "<group>"; };
                76FB9F0E12E851860051A2EB /* SHA1.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SHA1.h; sourceTree = "<group>"; };
                76FB9F1012E851960051A2EB /* SHA1.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SHA1.cpp; sourceTree = "<group>"; };
                7934BB761361979300CB99A1 /* ParallelJobs.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ParallelJobs.h; sourceTree = "<group>"; };
                                868BFA06117CEFD100B908B1 /* StringImpl.cpp */,
                                868BFA07117CEFD100B908B1 /* StringImpl.h */,
                                86B99AE2117E578100DF5A90 /* StringImplBase.h */,
+                               718A8482134F3A1200B87529 /* StringOperators.h */,
                                8626BECE11928E3900782FAB /* StringStatics.cpp */,
                                F3BD31D0126730180065467F /* TextPosition.h */,
                                868BFA15117CF19900B908B1 /* WTFString.cpp */,
                                5D63E9AD10F2BD6E00FC8AE9 /* StringHasher.h in Headers */,
                                868BFA0F117CEFD100B908B1 /* StringImpl.h in Headers */,
                                86B99AE4117E578100DF5A90 /* StringImplBase.h in Headers */,
+                               71DA9D82134F3F3D00B767E7 /* StringOperators.h in Headers */,
                                BC18C4680E16F5CD00B34460 /* StringObject.h in Headers */,
                                BC18C4690E16F5CD00B34460 /* StringObjectThatMasqueradesAsUndefined.h in Headers */,
                                BC18C46A0E16F5CD00B34460 /* StringPrototype.h in Headers */,
index 440700c..45d332f 100644 (file)
@@ -201,4 +201,5 @@ using WTF::xmlAtom;
 using WTF::xmlnsAtom;
 #endif
 
+#include "StringConcatenate.h"
 #endif // AtomicString_h
index 8500200..e8c6331 100644 (file)
@@ -26,7 +26,9 @@
 #ifndef StringConcatenate_h
 #define StringConcatenate_h
 
-#include <wtf/text/WTFString.h>
+#ifndef WTFString_h
+#include "AtomicString.h"
+#endif
 
 namespace WTF {
 
@@ -184,6 +186,21 @@ private:
     const String& m_buffer;
 };
 
+template<>
+class StringTypeAdapter<AtomicString> {
+public:
+    StringTypeAdapter<AtomicString>(const AtomicString& string)
+        : m_adapter(string.string())
+    {
+    }
+
+    unsigned length() { return m_adapter.length(); }
+    void writeTo(UChar* destination) { m_adapter.writeTo(destination); }
+
+private:
+    StringTypeAdapter<String> m_adapter;
+};
+
 inline void sumWithOverflow(unsigned& total, unsigned addend, bool& overflow)
 {
     unsigned oldTotal = total;
@@ -580,4 +597,5 @@ String makeString(StringType1 string1, StringType2 string2, StringType3 string3,
 
 using WTF::makeString;
 
+#include "StringOperators.h"
 #endif
diff --git a/Source/JavaScriptCore/wtf/text/StringOperators.h b/Source/JavaScriptCore/wtf/text/StringOperators.h
new file mode 100644 (file)
index 0000000..e8c2181
--- /dev/null
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) Research In Motion Limited 2011. 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.
+ *
+ */
+
+#ifndef StringOperators_h
+#define StringOperators_h
+
+namespace WTF {
+
+template<typename StringType1, typename StringType2>
+class StringAppend {
+public:
+    StringAppend(StringType1 string1, StringType2 string2)
+        : m_string1(string1)
+        , m_string2(string2)
+    {
+    }
+
+    operator String() const
+    {
+        RefPtr<StringImpl> resultImpl = tryMakeString(m_string1, m_string2);
+        if (!resultImpl)
+            CRASH();
+        return resultImpl.release();
+    }
+
+    operator AtomicString() const
+    {
+        return operator String();
+    }
+
+    void writeTo(UChar* destination)
+    {
+        StringTypeAdapter<StringType1> adapter1(m_string1);
+        StringTypeAdapter<StringType2> adapter2(m_string2);
+        adapter1.writeTo(destination);
+        adapter2.writeTo(destination + adapter1.length());
+    }
+
+    unsigned length()
+    {
+        StringTypeAdapter<StringType1> adapter1(m_string1);
+        StringTypeAdapter<StringType2> adapter2(m_string2);
+        return adapter1.length() + adapter2.length();
+    }    
+
+private:
+    StringType1 m_string1;
+    StringType2 m_string2;
+};
+
+template<typename StringType1, typename StringType2>
+class StringTypeAdapter<StringAppend<StringType1, StringType2> > {
+public:
+    StringTypeAdapter<StringAppend<StringType1, StringType2> >(StringAppend<StringType1, StringType2>& buffer)
+        : m_buffer(buffer)
+    {
+    }
+
+    unsigned length() { return m_buffer.length(); }
+    void writeTo(UChar* destination) { m_buffer.writeTo(destination); }
+
+private:
+    StringAppend<StringType1, StringType2>& m_buffer;
+};
+
+inline StringAppend<const char*, String> operator+(const char* string1, const String& string2)
+{
+    return StringAppend<const char*, String>(string1, string2);
+}
+
+inline StringAppend<const char*, AtomicString> operator+(const char* string1, const AtomicString& string2)
+{
+    return StringAppend<const char*, AtomicString>(string1, string2);
+}
+
+template<typename T>
+StringAppend<String, T> operator+(const String& string1, T string2)
+{
+    return StringAppend<String, T>(string1, string2);
+}
+
+template<typename U, typename V, typename W>
+StringAppend<U, StringAppend<V, W> > operator+(U string1, const StringAppend<V, W>& string2)
+{
+    return StringAppend<U, StringAppend<V, W> >(string1, string2);
+}
+
+} // namespace WTF
+
+#endif // StringOperators_h
index d862f96..3ab4ff5 100644 (file)
@@ -131,27 +131,6 @@ void String::append(UChar c)
         m_impl = StringImpl::create(&c, 1);
 }
 
-String operator+(const String& a, const String& b)
-{
-    if (a.isEmpty())
-        return b;
-    if (b.isEmpty())
-        return a;
-    String c = a;
-    c += b;
-    return c;
-}
-
-String operator+(const String& s, const char* cs)
-{
-    return s + String(cs);
-}
-
-String operator+(const char* cs, const String& s)
-{
-    return String(cs) + s;
-}
-
 int codePointCompare(const String& a, const String& b)
 {
     return codePointCompare(a.impl(), b.impl());
@@ -971,6 +950,12 @@ float charactersToFloat(const UChar* data, size_t length, bool* ok, bool* didRea
     return static_cast<float>(charactersToDouble(data, length, ok, didReadNumber));
 }
 
+const String& emptyString()
+{
+    DEFINE_STATIC_LOCAL(String, emptyString, (StringImpl::empty()));
+    return emptyString;
+}
+
 } // namespace WTF
 
 #ifndef NDEBUG
index b593d20..1eb7164 100644 (file)
@@ -356,10 +356,6 @@ QDataStream& operator<<(QDataStream& stream, const String& str);
 QDataStream& operator>>(QDataStream& stream, String& str);
 #endif
 
-String operator+(const String&, const String&);
-String operator+(const String&, const char*);
-String operator+(const char*, const String&);
-
 inline String& operator+=(String& a, const String& b) { a.append(b); return a; }
 
 inline bool operator==(const String& a, const String& b) { return equal(a.impl(), b.impl()); }
@@ -501,10 +497,14 @@ template<> struct DefaultHash<String> {
 
 template <> struct VectorTraits<String> : SimpleClassVectorTraits { };
 
+// Shared global empty string.
+const String& emptyString();
+
 }
 
 using WTF::CString;
 using WTF::String;
+using WTF::emptyString;
 using WTF::append;
 using WTF::appendNumber;
 using WTF::charactersAreAllASCII;
@@ -528,4 +528,5 @@ using WTF::isAllSpecialCharacters;
 using WTF::isSpaceOrNewline;
 using WTF::reverseFind;
 
+#include "AtomicString.h"
 #endif
index b3da3d0..06835b5 100644 (file)
@@ -1,3 +1,43 @@
+2011-05-12  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        Reviewed by Darin Adler.
+
+        String operator+ reallocates unnecessary when concatting > 2 strings
+        https://bugs.webkit.org/show_bug.cgi?id=58420
+
+        Provide a faster String append operator. See Source/JavaScriptCore/ChangeLog for details.
+
+        * dom/XMLDocumentParserLibxml2.cpp:
+        (WebCore::handleElementAttributes):
+        * editing/MarkupAccumulator.cpp:
+        (WebCore::MarkupAccumulator::shouldAddNamespaceElement):
+        * html/HTMLAnchorElement.cpp:
+        (WebCore::HTMLAnchorElement::hash):
+        (WebCore::HTMLAnchorElement::search):
+        * html/ImageInputType.cpp:
+        (WebCore::ImageInputType::appendFormData):
+        * html/parser/HTMLTreeBuilder.cpp:
+        * loader/CrossOriginAccessControl.cpp:
+        (WebCore::passesAccessControlCheck):
+        * page/Location.cpp:
+        (WebCore::Location::search):
+        (WebCore::Location::hash):
+        * page/NavigatorBase.cpp:
+        (WebCore::NavigatorBase::platform):
+        * platform/chromium/ClipboardChromium.cpp:
+        (WebCore::writeImageToDataObject):
+        * platform/gtk/PasteboardHelper.cpp:
+        (WebCore::PasteboardHelper::fillSelectionData):
+        * platform/network/cf/ResourceHandleCFNet.cpp:
+        (WebCore::encodeBasicAuthorization):
+        * platform/network/cf/SocketStreamHandleCFNet.cpp:
+        (WebCore::SocketStreamHandle::copyCFStreamDescription):
+        * platform/network/mac/ResourceHandleMac.mm:
+        (WebCore::encodeBasicAuthorization):
+        * workers/WorkerLocation.cpp:
+        (WebCore::WorkerLocation::search):
+        (WebCore::WorkerLocation::hash):
+
 2011-05-06  Yury Semikhatsky  <yurys@chromium.org>
 
         Reviewed by Pavel Feldman.
index 148c081..f2e6a04 100644 (file)
@@ -739,7 +739,7 @@ static inline void handleElementAttributes(Element* newElement, const xmlChar**
         AtomicString attrValue = toAtomicString(attributes[i].value, valueLength);
         String attrPrefix = toString(attributes[i].prefix);
         AtomicString attrURI = attrPrefix.isEmpty() ? AtomicString() : toAtomicString(attributes[i].uri);
-        AtomicString attrQName = attrPrefix.isEmpty() ? toAtomicString(attributes[i].localname) : AtomicString(attrPrefix + ":" + toString(attributes[i].localname));
+        AtomicString attrQName = attrPrefix.isEmpty() ? toAtomicString(attributes[i].localname) : attrPrefix + ":" + toString(attributes[i].localname);
 
         newElement->setAttributeNS(attrURI, attrQName, attrValue, ec, scriptingPermission);
         if (ec) // exception setting attributes
index 9939d6e..baa799b 100644 (file)
@@ -210,8 +210,11 @@ bool MarkupAccumulator::shouldAddNamespaceElement(const Element* element)
 {
     // Don't add namespace attribute if it is already defined for this elem.
     const AtomicString& prefix = element->prefix();
-    AtomicString attr = !prefix.isEmpty() ? "xmlns:" + prefix : "xmlns";
-    return !element->hasAttribute(attr);
+    if (prefix.isEmpty())
+        return !element->hasAttribute(xmlnsAtom);
+
+    DEFINE_STATIC_LOCAL(String, xmlnsWithColon, ("xmlns:"));
+    return !element->hasAttribute(xmlnsWithColon + prefix);
 }
 
 bool MarkupAccumulator::shouldAddNamespaceAttribute(const Attribute& attribute, Namespaces& namespaces)
index 31801df..e3ecf0f 100644 (file)
@@ -305,7 +305,7 @@ String HTMLAnchorElement::target() const
 String HTMLAnchorElement::hash() const
 {
     String fragmentIdentifier = href().fragmentIdentifier();
-    return fragmentIdentifier.isEmpty() ? "" : "#" + fragmentIdentifier;
+    return fragmentIdentifier.isEmpty() ? emptyString() : "#" + fragmentIdentifier;
 }
 
 void HTMLAnchorElement::setHash(const String& value)
@@ -442,7 +442,7 @@ void HTMLAnchorElement::setProtocol(const String& value)
 String HTMLAnchorElement::search() const
 {
     String query = href().query();
-    return query.isEmpty() ? "" : "?" + query;
+    return query.isEmpty() ? emptyString() : "?" + query;
 }
 
 String HTMLAnchorElement::origin() const
index 85b3329..e80b767 100644 (file)
@@ -57,9 +57,18 @@ bool ImageInputType::appendFormData(FormDataList& encoding, bool) const
     if (!element()->isActivatedSubmit())
         return false;
     const AtomicString& name = element()->name();
-    encoding.appendData(name.isEmpty() ? "x" : (name + ".x"), m_clickLocation.x());
-    encoding.appendData(name.isEmpty() ? "y" : (name + ".y"), m_clickLocation.y());
-    if (!name.isEmpty() && !element()->value().isEmpty())
+    if (name.isEmpty()) {
+        encoding.appendData("x", m_clickLocation.x());
+        encoding.appendData("y", m_clickLocation.y());
+        return true;
+    }
+
+    DEFINE_STATIC_LOCAL(String, dotXString, (".x"));
+    DEFINE_STATIC_LOCAL(String, dotYString, (".y"));
+    encoding.appendData(name + dotXString, m_clickLocation.x());
+    encoding.appendData(name + dotYString, m_clickLocation.y());
+
+    if (!!element()->value().isEmpty())
         encoding.appendData(name, element()->value());
     return true;
 }
index 5161608..f83026b 100644 (file)
@@ -669,7 +669,7 @@ void addNamesWithPrefix(PrefixedNameToQualifiedNameMap* map, const AtomicString&
     for (size_t i = 0; i < length; ++i) {
         QualifiedName* name = names[i];
         const AtomicString& localName = name->localName();
-        AtomicString prefixColonLocalName(prefix + ":" + localName);
+        AtomicString prefixColonLocalName = prefix + ':' + localName;
         QualifiedName nameWithPrefix(prefix, localName, name->namespaceURI());
         map->add(prefixColonLocalName, nameWithPrefix);
     }
index 8ca821e..1666ae1 100644 (file)
@@ -109,8 +109,10 @@ bool passesAccessControlCheck(const ResourceResponse& response, bool includeCred
     // FIXME: Access-Control-Allow-Origin can contain a list of origins.
     RefPtr<SecurityOrigin> accessControlOrigin = SecurityOrigin::createFromString(accessControlOriginString);
     if (!accessControlOrigin->isSameSchemeHostPort(securityOrigin)) {
-        errorDescription = (accessControlOriginString == "*") ? "Cannot use wildcard in Access-Control-Allow-Origin when credentials flag is true."
-            : "Origin " + securityOrigin->toString() + " is not allowed by Access-Control-Allow-Origin.";
+        if (accessControlOriginString == "*")
+            errorDescription = "Cannot use wildcard in Access-Control-Allow-Origin when credentials flag is true.";
+        else
+            errorDescription =  "Origin " + securityOrigin->toString() + " is not allowed by Access-Control-Allow-Origin.";
         return false;
     }
 
index 2a33de3..6781103 100644 (file)
@@ -120,7 +120,7 @@ String Location::search() const
         return String();
 
     const KURL& url = this->url();
-    return url.query().isEmpty() ? "" : "?" + url.query();
+    return url.query().isEmpty() ? emptyString() : "?" + url.query();
 }
 
 String Location::origin() const
@@ -136,7 +136,7 @@ String Location::hash() const
         return String();
 
     const String& fragmentIdentifier = url().fragmentIdentifier();
-    return fragmentIdentifier.isEmpty() ? "" : "#" + fragmentIdentifier;
+    return fragmentIdentifier.isEmpty() ? emptyString() : "#" + fragmentIdentifier;
 }
 
 String Location::getParameter(const String& name) const
index ca51a29..e31fe11 100644 (file)
@@ -86,10 +86,10 @@ String NavigatorBase::appVersion() const
 String NavigatorBase::platform() const
 {
 #if OS(LINUX)
-    if (String("") != WEBCORE_NAVIGATOR_PLATFORM)
+    if (!String(WEBCORE_NAVIGATOR_PLATFORM).isEmpty())
         return WEBCORE_NAVIGATOR_PLATFORM;
     struct utsname osname;
-    DEFINE_STATIC_LOCAL(String, platformName, (uname(&osname) >= 0 ? String(osname.sysname) + String(" ") + String(osname.machine) : ""));
+    DEFINE_STATIC_LOCAL(String, platformName, (uname(&osname) >= 0 ? String(osname.sysname) + String(" ") + String(osname.machine) : emptyString()));
     return platformName;
 #else
     return WEBCORE_NAVIGATOR_PLATFORM;
index 64ddd04..5bc26f7 100644 (file)
@@ -230,7 +230,7 @@ static void writeImageToDataObject(ChromiumDataObject* dataObject, Element* elem
     // in the URL.
     String extension = MIMETypeRegistry::getPreferredExtensionForMIMEType(
         cachedImage->response().mimeType());
-    dataObject->setFileExtension(extension.isEmpty() ? "" : "." + extension);
+    dataObject->setFileExtension(extension.isEmpty() ? emptyString() : "." + extension);
     String title = element->getAttribute(altAttr);
     if (title.isEmpty())
         title = cachedImage->response().suggestedFilename();
index 013789f..fc3ede4 100644 (file)
@@ -162,7 +162,7 @@ void PasteboardHelper::fillSelectionData(GtkSelectionData* selectionData, guint
     else if (info == getIdForTargetType(TargetTypeMarkup)) {
         // Some Linux applications refuse to accept pasted markup unless it is
         // prefixed by a content-type meta tag.
-        CString markup = (gMarkupPrefix + dataObject->markup()).utf8();
+        CString markup = String(gMarkupPrefix + dataObject->markup()).utf8();
         gtk_selection_data_set(selectionData, markupAtom, 8,
             reinterpret_cast<const guchar*>(markup.data()), markup.length() + 1);
 
index 3f590d1..5c47e5e 100644 (file)
@@ -126,7 +126,7 @@ static void setDefaultMIMEType(CFURLResponseRef response)
 
 static String encodeBasicAuthorization(const String& user, const String& password)
 {
-    return base64Encode((user + ":" + password).utf8());
+    return base64Encode(String(user + ":" + password).utf8());
 }
 
 CFURLRequestRef willSendRequest(CFURLConnectionRef conn, CFURLRequestRef cfRequest, CFURLResponseRef cfRedirectResponse, const void* clientInfo)
index 0eba3b8..56e33a8 100644 (file)
@@ -372,7 +372,7 @@ void SocketStreamHandle::addCONNECTCredentials(CFHTTPMessageRef proxyResponse)
 CFStringRef SocketStreamHandle::copyCFStreamDescription(void* info)
 {
     SocketStreamHandle* handle = static_cast<SocketStreamHandle*>(info);
-    return ("WebKit socket stream, " + handle->m_url.string()).createCFString();
+    return String("WebKit socket stream, " + handle->m_url.string()).createCFString();
 }
 
 struct MainThreadEventCallbackInfo {
index c8f0fda..5945643 100644 (file)
@@ -139,7 +139,7 @@ static bool isInitializingConnection;
     
 static String encodeBasicAuthorization(const String& user, const String& password)
 {
-    return base64Encode((user + ":" + password).utf8());
+    return base64Encode(String(user + ":" + password).utf8());
 }
 
 ResourceHandleInternal::~ResourceHandleInternal()
index b934abd..d227a31 100644 (file)
@@ -66,12 +66,12 @@ String WorkerLocation::pathname() const
 
 String WorkerLocation::search() const
 {
-    return m_url.query().isEmpty() ? "" : "?" + m_url.query();
+    return m_url.query().isEmpty() ? emptyString() : "?" + m_url.query();
 }
 
 String WorkerLocation::hash() const
 {
-    return m_url.fragmentIdentifier().isEmpty() ? "" : "#" + m_url.fragmentIdentifier();
+    return m_url.fragmentIdentifier().isEmpty() ? emptyString() : "#" + m_url.fragmentIdentifier();
 }
 
 String WorkerLocation::toString() const
index 145137c..f357cd6 100644 (file)
@@ -1,3 +1,19 @@
+2011-05-12  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        Reviewed by Darin Adler.
+
+        String operator+ reallocates unnecessarily when concatting > 2 strings
+        https://bugs.webkit.org/show_bug.cgi?id=58420
+
+        Provide a faster String append operator. See Source/JavaScriptCore/ChangeLog for details.
+
+        * src/WebAccessibilityObject.cpp:
+        (WebKit::WebAccessibilityObject::keyboardShortcut): Cast to String first, before trying to convert to platform dependant type.
+        * src/WebHTTPLoadInfo.cpp:
+        (WebKit::addHeader): Don't pass WebString to makeString, explicit cast to String first.
+        * tests/IDBLevelDBCodingTest.cpp: Cast to String first, to avoid conflicting with gtests global templatified operator+.
+        (IDBLevelDBCoding::TEST):
+
 2011-05-10  Tony Gentilcore  <tonyg@chromium.org>
 
         Reviewed by Darin Adler.
index 8df112a..efb762b 100644 (file)
@@ -423,7 +423,7 @@ WebString WebAccessibilityObject::keyboardShortcut() const
             modifierString += "Win+";
     }
 
-    return modifierString + accessKey;
+    return String(modifierString + accessKey);
 }
 
 bool WebAccessibilityObject::performDefaultAction() const
index 9cb4aaa..7a07dde 100644 (file)
@@ -105,7 +105,7 @@ static void addHeader(HTTPHeaderMap* map, const WebString& name, const WebString
 {
     pair<HTTPHeaderMap::iterator, bool> result = map->add(name, value);
     if (!result.second)
-        result.first->second += String("\n") + value;
+        result.first->second += "\n" + String(value);
 }
 
 void WebHTTPLoadInfo::addRequestHeader(const WebString& name, const WebString& value)
index 40cc2c3..cbb8c02 100644 (file)
@@ -180,10 +180,10 @@ TEST(IDBLevelDBCodingTest, DecodeString)
     EXPECT_EQ(String("foo"), decodeString(v.data(), v.data() + v.size()));
 
     v = encodeString(String(testStringA));
-    EXPECT_EQ(testStringA, decodeString(v.data(), v.data() + v.size()));
+    EXPECT_EQ(String(testStringA), decodeString(v.data(), v.data() + v.size()));
 
     v = encodeString(String(testStringB));
-    EXPECT_EQ(testStringB, decodeString(v.data(), v.data() + v.size()));
+    EXPECT_EQ(String(testStringB), decodeString(v.data(), v.data() + v.size()));
 }
 
 TEST(IDBLevelDBCodingTest, EncodeStringWithLength)
index 5db05e2..f93b870 100644 (file)
@@ -1,3 +1,15 @@
+2011-05-12  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        Reviewed by Darin Adler.
+
+        String operator+ reallocates unnecessarily when concatting > 2 strings
+        https://bugs.webkit.org/show_bug.cgi?id=58420
+
+        Provide a faster String append operator. See Source/JavaScriptCore/ChangeLog for details.
+
+        * WebView/WebFrame.mm: Explicitely cast to Strings first, so operator NSString*() can be invoked.
+        (-[WebFrame _stringWithDocumentTypeStringAndMarkupString:]):
+
 2011-05-10  Tony Gentilcore  <tonyg@chromium.org>
 
         Reviewed by Darin Adler.
index 1c3cb1f..9f193c5 100644 (file)
@@ -493,7 +493,7 @@ static inline WebDataSource *dataSource(DocumentLoader* loader)
 
 - (NSString *)_stringWithDocumentTypeStringAndMarkupString:(NSString *)markupString
 {
-    return _private->coreFrame->documentTypeString() + markupString;
+    return String(_private->coreFrame->documentTypeString() + String(markupString));
 }
 
 - (NSArray *)_nodesFromList:(Vector<Node*> *)nodesVector
index 1ebd45c..3f0bfd4 100644 (file)
@@ -375,7 +375,7 @@ HRESULT STDMETHODCALLTYPE AccessibleBase::get_accKeyboardShortcut(VARIANT vChild
         if (modifiers & PlatformKeyboardEvent::MetaKey)
             accessKeyModifiers += "Win+";
     }
-    *shortcut = BString(accessKeyModifiers + accessKey).release();
+    *shortcut = BString(String(accessKeyModifiers + accessKey)).release();
     return S_OK;
 }
 
index 3b0c8e1..cddfc78 100644 (file)
@@ -1,3 +1,15 @@
+2011-05-12  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        Reviewed by Darin Adler.
+
+        String operator+ reallocates unnecessarily when concatting > 2 strings
+        https://bugs.webkit.org/show_bug.cgi?id=58420
+
+        Provide a faster String append operator. See Source/JavaScriptCore/ChangeLog for details.
+
+        * AccessibleBase.cpp:
+        (AccessibleBase::get_accKeyboardShortcut): Explicitely cast to Strings first, so operator BString() can be invoked.
+
 2011-05-11  Dmitry Lomov  <dslomov@google.com>
 
         Reviewed by David Levin.