Optimize JSRopeString for resolving directly to AtomicString.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 May 2014 06:24:44 +0000 (06:24 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 May 2014 06:24:44 +0000 (06:24 +0000)
<https://webkit.org/b/132548>

Source/JavaScriptCore:
If we know that the JSRopeString we are resolving is going to be used
as an AtomicString, we can try to avoid creating a new string.

We do this by first resolving the rope into a stack buffer, and using
that buffer as a key into the AtomicString table. If there is already
an AtomicString with the same characters, we reuse that instead of
constructing a new StringImpl.

JSString gains these two public functions:

- AtomicString toAtomicString()

    Returns an AtomicString, tries to avoid allocating a new string
    if possible.

- AtomicStringImpl* toExistingAtomicString()

    Returns a non-null AtomicStringImpl* if one already exists in the
    AtomicString table. If none is found, the rope is left unresolved.

Reviewed by Filip Pizlo.

* runtime/JSString.cpp:
(JSC::JSRopeString::resolveRopeInternal8):
(JSC::JSRopeString::resolveRopeInternal16):
(JSC::JSRopeString::resolveRopeToAtomicString):
(JSC::JSRopeString::clearFibers):
(JSC::JSRopeString::resolveRopeToExistingAtomicString):
(JSC::JSRopeString::resolveRope):
(JSC::JSRopeString::outOfMemory):
* runtime/JSString.h:
(JSC::JSString::toAtomicString):
(JSC::JSString::toExistingAtomicString):

Source/WebCore:
Add two bindings generator attributes for parameters to influence
the way that JS rope strings are resolved:

- AtomicString

    Generates code that avoids allocating a new StringImpl if there
    is already an existing AtomicString we can reuse.

- RequiresExistingAtomicString

    Generates code that fails immediately if the provided string
    is not found in the AtomicString table. This is now used for
    document.getElementById(), and works because any existing ID
    is guaranteed to be in the table.

Reviewed by Filip Pizlo.

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateParametersCheck):
(JSValueToNative):
* bindings/scripts/IDLAttributes.txt:
* dom/Document.idl:

Source/WTF:
Add AtomicString::find([LU]Char*, unsigned length) helpers for finding
an existing AtomicString without a StringImpl on hand.

Reviewed by Filip Pizlo.

* wtf/text/AtomicString.h:
* wtf/text/AtomicString.cpp:
(WTF::AtomicString::find):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSString.cpp
Source/JavaScriptCore/runtime/JSString.h
Source/WTF/ChangeLog
Source/WTF/wtf/text/AtomicString.cpp
Source/WTF/wtf/text/AtomicString.h
Source/WebCore/ChangeLog
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/bindings/scripts/IDLAttributes.txt
Source/WebCore/dom/Document.idl

index f4665a4..e1c3c42 100644 (file)
@@ -1,5 +1,44 @@
 2014-05-04  Andreas Kling  <akling@apple.com>
 
+        Optimize JSRopeString for resolving directly to AtomicString.
+        <https://webkit.org/b/132548>
+
+        If we know that the JSRopeString we are resolving is going to be used
+        as an AtomicString, we can try to avoid creating a new string.
+
+        We do this by first resolving the rope into a stack buffer, and using
+        that buffer as a key into the AtomicString table. If there is already
+        an AtomicString with the same characters, we reuse that instead of
+        constructing a new StringImpl.
+
+        JSString gains these two public functions:
+
+        - AtomicString toAtomicString()
+
+            Returns an AtomicString, tries to avoid allocating a new string
+            if possible.
+
+        - AtomicStringImpl* toExistingAtomicString()
+
+            Returns a non-null AtomicStringImpl* if one already exists in the
+            AtomicString table. If none is found, the rope is left unresolved.
+
+        Reviewed by Filip Pizlo.
+
+        * runtime/JSString.cpp:
+        (JSC::JSRopeString::resolveRopeInternal8):
+        (JSC::JSRopeString::resolveRopeInternal16):
+        (JSC::JSRopeString::resolveRopeToAtomicString):
+        (JSC::JSRopeString::clearFibers):
+        (JSC::JSRopeString::resolveRopeToExistingAtomicString):
+        (JSC::JSRopeString::resolveRope):
+        (JSC::JSRopeString::outOfMemory):
+        * runtime/JSString.h:
+        (JSC::JSString::toAtomicString):
+        (JSC::JSString::toExistingAtomicString):
+
+2014-05-04  Andreas Kling  <akling@apple.com>
+
         Unreviewed, rolling out r168254.
 
         Very crashy on debug JSC tests.
index 00f05f5..b911a4a 100644 (file)
@@ -53,7 +53,7 @@ void JSString::destroy(JSCell* cell)
 void JSString::dumpToStream(const JSCell* cell, PrintStream& out)
 {
     const JSString* thisObject = jsCast<const JSString*>(cell);
-    out.printf("<%p, %s, [%u], ", thisObject, thisObject->className(), thisObject->length()); 
+    out.printf("<%p, %s, [%u], ", thisObject, thisObject->className(), thisObject->length());
     if (thisObject->isRope())
         out.printf("[rope]");
     else {
@@ -86,6 +86,113 @@ void JSRopeString::visitFibers(SlotVisitor& visitor)
         visitor.append(&m_fibers[i]);
 }
 
+static const unsigned maxLengthForOnStackResolve = 2048;
+
+void JSRopeString::resolveRopeInternal8(LChar* buffer) const
+{
+    for (size_t i = 0; i < s_maxInternalRopeLength && m_fibers[i]; ++i) {
+        if (m_fibers[i]->isRope()) {
+            resolveRopeSlowCase8(buffer);
+            return;
+        }
+    }
+
+    LChar* position = buffer;
+    for (size_t i = 0; i < s_maxInternalRopeLength && m_fibers[i]; ++i) {
+        const StringImpl& fiberString = *m_fibers[i]->m_value.impl();
+        unsigned length = fiberString.length();
+        StringImpl::copyChars(position, fiberString.characters8(), length);
+        position += length;
+    }
+    ASSERT((buffer + m_length) == position);
+}
+
+void JSRopeString::resolveRopeInternal16(UChar* buffer) const
+{
+    for (size_t i = 0; i < s_maxInternalRopeLength && m_fibers[i]; ++i) {
+        if (m_fibers[i]->isRope()) {
+            resolveRopeSlowCase(buffer);
+            return;
+        }
+    }
+
+    UChar* position = buffer;
+    for (size_t i = 0; i < s_maxInternalRopeLength && m_fibers[i]; ++i) {
+        const StringImpl& fiberString = *m_fibers[i]->m_value.impl();
+        unsigned length = fiberString.length();
+        if (fiberString.is8Bit())
+            StringImpl::copyChars(position, fiberString.characters8(), length);
+        else
+            StringImpl::copyChars(position, fiberString.characters16(), length);
+        position += length;
+    }
+    ASSERT((buffer + m_length) == position);
+}
+
+void JSRopeString::resolveRopeToAtomicString(ExecState* exec) const
+{
+    if (m_length > maxLengthForOnStackResolve) {
+        resolveRope(exec);
+        m_value = AtomicString(m_value);
+        return;
+    }
+
+    if (is8Bit()) {
+        LChar buffer[maxLengthForOnStackResolve];
+        resolveRopeInternal8(buffer);
+        m_value = AtomicString(buffer, m_length);
+    } else {
+        UChar buffer[maxLengthForOnStackResolve];
+        resolveRopeInternal16(buffer);
+        m_value = AtomicString(buffer, m_length);
+    }
+
+    clearFibers();
+
+    // If we resolved a string that didn't previously exist, notify the heap that we've grown.
+    if (m_value.impl()->hasOneRef())
+        Heap::heap(this)->reportExtraMemoryCost(m_value.impl()->cost());
+}
+
+void JSRopeString::clearFibers() const
+{
+    for (size_t i = 0; i < s_maxInternalRopeLength && m_fibers[i]; ++i)
+        m_fibers[i].clear();
+}
+
+AtomicStringImpl* JSRopeString::resolveRopeToExistingAtomicString(ExecState* exec) const
+{
+    if (m_length > maxLengthForOnStackResolve) {
+        resolveRope(exec);
+        if (AtomicStringImpl* existingAtomicString = AtomicString::find(m_value.impl())) {
+            m_value = *existingAtomicString;
+            clearFibers();
+            return existingAtomicString;
+        }
+        return nullptr;
+    }
+
+    if (is8Bit()) {
+        LChar buffer[maxLengthForOnStackResolve];
+        resolveRopeInternal8(buffer);
+        if (AtomicStringImpl* existingAtomicString = AtomicString::find(buffer, m_length)) {
+            m_value = *existingAtomicString;
+            clearFibers();
+            return existingAtomicString;
+        }
+    } else {
+        UChar buffer[maxLengthForOnStackResolve];
+        resolveRopeInternal16(buffer);
+        if (AtomicStringImpl* existingAtomicString = AtomicString::find(buffer, m_length)) {
+            m_value = *existingAtomicString;
+            clearFibers();
+            return existingAtomicString;
+        }
+    }
+
+    return nullptr;
+}
+
 void JSRopeString::resolveRope(ExecState* exec) const
 {
     ASSERT(isRope());
@@ -99,23 +206,9 @@ void JSRopeString::resolveRope(ExecState* exec) const
             outOfMemory(exec);
             return;
         }
-
-        for (size_t i = 0; i < s_maxInternalRopeLength && m_fibers[i]; ++i) {
-            if (m_fibers[i]->isRope())
-                return resolveRopeSlowCase8(buffer);
-        }
-
-        LChar* position = buffer;
-        for (size_t i = 0; i < s_maxInternalRopeLength && m_fibers[i]; ++i) {
-            StringImpl* string = m_fibers[i]->m_value.impl();
-            unsigned length = string->length();
-            StringImpl::copyChars(position, string->characters8(), length);
-            position += length;
-            m_fibers[i].clear();
-        }
-        ASSERT((buffer + m_length) == position);
+        resolveRopeInternal8(buffer);
+        clearFibers();
         ASSERT(!isRope());
-
         return;
     }
 
@@ -128,23 +221,8 @@ void JSRopeString::resolveRope(ExecState* exec) const
         return;
     }
 
-    for (size_t i = 0; i < s_maxInternalRopeLength && m_fibers[i]; ++i) {
-        if (m_fibers[i]->isRope())
-            return resolveRopeSlowCase(buffer);
-    }
-
-    UChar* position = buffer;
-    for (size_t i = 0; i < s_maxInternalRopeLength && m_fibers[i]; ++i) {
-        StringImpl* string = m_fibers[i]->m_value.impl();
-        unsigned length = string->length();
-        if (string->is8Bit())
-            StringImpl::copyChars(position, string->characters8(), length);
-        else
-            StringImpl::copyChars(position, string->characters16(), length);
-        position += length;
-        m_fibers[i].clear();
-    }
-    ASSERT((buffer + m_length) == position);
+    resolveRopeInternal16(buffer);
+    clearFibers();
     ASSERT(!isRope());
 }
 
@@ -224,8 +302,7 @@ void JSRopeString::resolveRopeSlowCase(UChar* buffer) const
 
 void JSRopeString::outOfMemory(ExecState* exec) const
 {
-    for (size_t i = 0; i < s_maxInternalRopeLength && m_fibers[i]; ++i)
-        m_fibers[i].clear();
+    clearFibers();
     ASSERT(isRope());
     ASSERT(m_value.isNull());
     if (exec)
index 4226f74..fc5170f 100644 (file)
@@ -139,6 +139,8 @@ namespace JSC {
             return newString;
         }
 
+        const AtomicString& toAtomicString(ExecState*) const;
+        AtomicStringImpl* toExistingAtomicString(ExecState*) const;
         const String& value(ExecState*) const;
         const String& tryGetValue() const;
         const StringImpl* tryGetValueImpl() const;
@@ -329,9 +331,14 @@ namespace JSC {
         friend JSValue jsStringFromArguments(ExecState*, JSValue);
 
         JS_EXPORT_PRIVATE void resolveRope(ExecState*) const;
+        JS_EXPORT_PRIVATE void resolveRopeToAtomicString(ExecState*) const;
+        JS_EXPORT_PRIVATE AtomicStringImpl* resolveRopeToExistingAtomicString(ExecState*) const;
         void resolveRopeSlowCase8(LChar*) const;
         void resolveRopeSlowCase(UChar*) const;
         void outOfMemory(ExecState*) const;
+        void resolveRopeInternal8(LChar*) const;
+        void resolveRopeInternal16(UChar*) const;
+        void clearFibers() const;
             
         JS_EXPORT_PRIVATE JSString* getIndexSlowCase(ExecState*, unsigned);
 
@@ -380,6 +387,28 @@ namespace JSC {
         return JSString::create(*vm, s.impl());
     }
 
+    ALWAYS_INLINE const AtomicString& JSString::toAtomicString(ExecState* exec) const
+    {
+        if (isRope())
+            static_cast<const JSRopeString*>(this)->resolveRopeToAtomicString(exec);
+        else if (!m_value.impl()->isAtomic())
+            m_value = AtomicString(m_value);
+        return *reinterpret_cast<const AtomicString*>(&m_value);
+    }
+
+    ALWAYS_INLINE AtomicStringImpl* JSString::toExistingAtomicString(ExecState* exec) const
+    {
+        if (isRope())
+            return static_cast<const JSRopeString*>(this)->resolveRopeToExistingAtomicString(exec);
+        if (m_value.impl()->isAtomic())
+            return static_cast<AtomicStringImpl*>(m_value.impl());
+        if (AtomicStringImpl* existingAtomicString = AtomicString::find(m_value.impl())) {
+            m_value = *existingAtomicString;
+            return existingAtomicString;
+        }
+        return nullptr;
+    }
+
     inline const String& JSString::value(ExecState* exec) const
     {
         if (isRope())
index 720ac60..98b4df9 100644 (file)
@@ -1,3 +1,17 @@
+2014-05-04  Andreas Kling  <akling@apple.com>
+
+        Optimize JSRopeString for resolving directly to AtomicString.
+        <https://webkit.org/b/132548>
+
+        Add AtomicString::find([LU]Char*, unsigned length) helpers for finding
+        an existing AtomicString without a StringImpl on hand.
+
+        Reviewed by Filip Pizlo.
+
+        * wtf/text/AtomicString.h:
+        * wtf/text/AtomicString.cpp:
+        (WTF::AtomicString::find):
+
 2014-05-01  Brent Fulgham  <bfulgham@apple.com>
 
         Fix handling of attributes prior to compiling shader
index 9d5b622..9b47839 100644 (file)
@@ -525,6 +525,30 @@ AtomicString AtomicString::number(double number)
     return String(numberToFixedPrecisionString(number, 6, buffer, true));
 }
 
+AtomicStringImpl* AtomicString::find(LChar* characters, unsigned length)
+{
+    AtomicStringTableLocker locker;
+    auto& table = stringTable();
+
+    LCharBuffer buffer = { characters, length };
+    auto iterator = table.find<LCharBufferTranslator>(buffer);
+    if (iterator != table.end())
+        return static_cast<AtomicStringImpl*>(*iterator);
+    return nullptr;
+}
+
+AtomicStringImpl* AtomicString::find(UChar* characters, unsigned length)
+{
+    AtomicStringTableLocker locker;
+    auto& table = stringTable();
+
+    UCharBuffer buffer = { characters, length };
+    auto iterator = table.find<UCharBufferTranslator>(buffer);
+    if (iterator != table.end())
+        return static_cast<AtomicStringImpl*>(*iterator);
+    return nullptr;
+}
+
 #if !ASSERT_DISABLED
 bool AtomicString::isInAtomicStringTable(StringImpl* string)
 {
index 2d7036e..146701b 100644 (file)
@@ -87,6 +87,8 @@ public:
     bool isHashTableDeletedValue() const { return m_string.isHashTableDeletedValue(); }
 
     WTF_EXPORT_STRING_API static AtomicStringImpl* findStringWithHash(const StringImpl&);
+    WTF_EXPORT_STRING_API static AtomicStringImpl* find(LChar*, unsigned length);
+    WTF_EXPORT_STRING_API static AtomicStringImpl* find(UChar*, unsigned length);
     static AtomicStringImpl* find(StringImpl* string)
     {
         if (!string || string->isAtomic())
index a14a57e..edeff3c 100644 (file)
@@ -1,3 +1,31 @@
+2014-05-04  Andreas Kling  <akling@apple.com>
+
+        Optimize JSRopeString for resolving directly to AtomicString.
+        <https://webkit.org/b/132548>
+
+        Add two bindings generator attributes for parameters to influence
+        the way that JS rope strings are resolved:
+
+        - AtomicString
+
+            Generates code that avoids allocating a new StringImpl if there
+            is already an existing AtomicString we can reuse.
+
+        - RequiresExistingAtomicString
+
+            Generates code that fails immediately if the provided string
+            is not found in the AtomicString table. This is now used for
+            document.getElementById(), and works because any existing ID
+            is guaranteed to be in the table.
+
+        Reviewed by Filip Pizlo.
+
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateParametersCheck):
+        (JSValueToNative):
+        * bindings/scripts/IDLAttributes.txt:
+        * dom/Document.idl:
+
 2014-05-04  Simon Fraser  <simon.fraser@apple.com>
 
         [iOS WK2] Compositing layers in iframes are misplaced
index 5c5d44d..44b0f34 100644 (file)
@@ -3283,7 +3283,14 @@ sub GenerateParametersCheck
                 }
             }
 
-            push(@$outputArray, "    " . GetNativeTypeFromSignature($parameter) . " $name(" . JSValueToNative($parameter, $optional && $defaultAttribute && $defaultAttribute eq "NullString" ? "argumentOrNull(exec, $argsIndex)" : "exec->argument($argsIndex)") . ");\n");
+            if ($parameter->extendedAttributes->{"RequiresExistingAtomicString"}) {
+                push(@$outputArray, "    AtomicStringImpl* existing_$name = exec->argument($argsIndex).isEmpty() ? nullptr : exec->argument($argsIndex).toString(exec)->toExistingAtomicString(exec);\n");
+                push(@$outputArray, "    if (!existing_$name)\n");
+                push(@$outputArray, "        return JSValue::encode(jsNull());\n");
+                push(@$outputArray, "    const AtomicString& $name(existing_$name);\n");
+            } else {
+                push(@$outputArray, "    " . GetNativeTypeFromSignature($parameter) . " $name(" . JSValueToNative($parameter, $optional && $defaultAttribute && $defaultAttribute eq "NullString" ? "argumentOrNull(exec, $argsIndex)" : "exec->argument($argsIndex)") . ");\n");
+            }
 
             # If a parameter is "an index" and it's negative it should throw an INDEX_SIZE_ERR exception.
             # But this needs to be done in the bindings, because the type is unsigned and the fact that it
@@ -3761,6 +3768,9 @@ sub JSValueToNative
         if (($signature->extendedAttributes->{"TreatNullAs"} and $signature->extendedAttributes->{"TreatNullAs"} eq "NullString") or $signature->extendedAttributes->{"Reflect"}) {
             return "valueToStringWithNullCheck(exec, $value)"
         }
+        if ($signature->extendedAttributes->{"AtomicString"}) {
+            return "$value.isEmpty() ? AtomicString() : $value.toString(exec)->toAtomicString(exec)";
+        }
         # FIXME: Add the case for 'if ($signature->extendedAttributes->{"TreatUndefinedAs"} and $signature->extendedAttributes->{"TreatUndefinedAs"} eq "NullString"))'.
         return "$value.isEmpty() ? String() : $value.toString(exec)->value(exec)";
     }
index d13b30e..ea0ef70 100644 (file)
@@ -19,6 +19,7 @@
 #
 
 ActiveDOMObject
+AtomicString
 CPPPureInterface
 CachedAttribute
 CallbackNeedsOperatorEqual
@@ -101,6 +102,7 @@ Reflect=*
 Replaceable
 ReplaceableConstructor
 ReportExtraMemoryCost
+RequiresExistingAtomicString
 ReturnNewObject
 SetterRaisesException
 SkipVTableValidation
index 8dd51ff..6dfe52f 100644 (file)
@@ -51,7 +51,7 @@
                                                                           [TreatNullAs=NullString,Default=Undefined] optional DOMString qualifiedName);
     [ObjCLegacyUnnamedParameters] NodeList getElementsByTagNameNS([TreatNullAs=NullString,Default=Undefined] optional DOMString namespaceURI,
                                                    [Default=Undefined] optional DOMString localName);
-    Element getElementById([Default=Undefined,ObjCExplicitAtomicString] optional DOMString elementId);
+    Element getElementById([Default=Undefined,ObjCExplicitAtomicString,RequiresExistingAtomicString] optional DOMString elementId);
 
     // DOM Level 3 Core
 
     readonly attribute HTMLCollection anchors;
     readonly attribute DOMString lastModified;
 
-    NodeList getElementsByName([Default=Undefined] optional DOMString elementName);
+    NodeList getElementsByName([Default=Undefined,AtomicString] optional DOMString elementName);
 
 #if defined(LANGUAGE_JAVASCRIPT) && LANGUAGE_JAVASCRIPT
              [Custom] attribute Location location;