2010-01-16 Maciej Stachowiak <mjs@apple.com>
authormjs@apple.com <mjs@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 17 Jan 2010 07:18:38 +0000 (07:18 +0000)
committermjs@apple.com <mjs@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 17 Jan 2010 07:18:38 +0000 (07:18 +0000)
        Reviewed by Oliver Hunt.

        Cache JS string values made from DOM strings (Dromaeo speedup)
        https://bugs.webkit.org/show_bug.cgi?id=33768
        <rdar://problem/7353576>

        * runtime/JSString.h:
        (JSC::jsStringWithFinalizer): Added new mechanism for a string to have an optional
        finalizer callback, for the benefit of weak-referencing caches.
        (JSC::):
        (JSC::Fiber::JSString):
        (JSC::Fiber::~JSString):
        * runtime/JSString.cpp:
        (JSC::JSString::resolveRope): Clear fibers so this doesn't look like a string with a finalizer.
        * runtime/WeakGCMap.h: Include "Collector.h" to make this header includable by itself.
2010-01-16  Maciej Stachowiak  <mjs@apple.com>

        Reviewed by Oliver Hunt.

        Cache JS string values made from DOM strings (Dromaeo speedup)
        https://bugs.webkit.org/show_bug.cgi?id=33768
        <rdar://problem/7353576>

        * Plugins/Hosted/ProxyInstance.mm:
        (WebKit::ProxyInstance::stringValue): Explicitly make a String, since char*
        is now ambiguous.
2010-01-16  Maciej Stachowiak  <mjs@apple.com>

        Reviewed by Oliver Hunt.

        Cache JS string values made from DOM strings (Dromaeo speedup)
        https://bugs.webkit.org/show_bug.cgi?id=33768
        <rdar://problem/7353576>

        Added a new cache for JSString values that are created from Strings or AtomicStrings
        in the DOM. It's common for the same string to be retrieved from the DOM repeatedly,
        and it is wasteful to make a new JS-level string value every time.

        The string cache is per-world, and thus thread-safe and not a
        vector for accidental information exchange.

        ~30% speedup on Dromaeo Attributes test, also substantially helps other Dromaeo DOM tests.

        * bindings/js/JSDOMBinding.cpp:
        (WebCore::jsStringCache): Helper function to get the string cache for the current world.
        (WebCore::jsString): Some new overloads including the caching version.
        (WebCore::stringWrapperDestroyed): Finalizer callback - remove from relevant caches.
        * bindings/js/JSDOMBinding.h:
        (WebCore::jsString): Prototype new overloads (and define a few inline).
        * bindings/js/JSJavaScriptCallFrameCustom.cpp:
        (WebCore::JSJavaScriptCallFrame::type): Explicitly make a UString.
        * bindings/js/ScriptFunctionCall.cpp:
        (WebCore::ScriptFunctionCall::appendArgument): Ditto.
        * WebCore.base.exp: Add new JSString overloads that WebCore gets to see.

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

12 files changed:
JavaScriptCore/ChangeLog
JavaScriptCore/runtime/JSString.cpp
JavaScriptCore/runtime/JSString.h
JavaScriptCore/runtime/WeakGCMap.h
WebCore/ChangeLog
WebCore/WebCore.base.exp
WebCore/bindings/js/JSDOMBinding.cpp
WebCore/bindings/js/JSDOMBinding.h
WebCore/bindings/js/JSJavaScriptCallFrameCustom.cpp
WebCore/bindings/js/ScriptFunctionCall.cpp
WebKit/mac/ChangeLog
WebKit/mac/Plugins/Hosted/ProxyInstance.mm

index b9f7717dc792c00ee32b5a2d7dac7b98bc616f70..d744185058ec499f3a4c0c4a7451da1a98042562 100644 (file)
@@ -1,3 +1,21 @@
+2010-01-16  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        Cache JS string values made from DOM strings (Dromaeo speedup)
+        https://bugs.webkit.org/show_bug.cgi?id=33768
+        <rdar://problem/7353576>
+
+        * runtime/JSString.h:
+        (JSC::jsStringWithFinalizer): Added new mechanism for a string to have an optional
+        finalizer callback, for the benefit of weak-referencing caches.
+        (JSC::):
+        (JSC::Fiber::JSString):
+        (JSC::Fiber::~JSString):
+        * runtime/JSString.cpp:
+        (JSC::JSString::resolveRope): Clear fibers so this doesn't look like a string with a finalizer.
+        * runtime/WeakGCMap.h: Include "Collector.h" to make this header includable by itself.
+
 2010-01-15  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Maciej Stachowiak.
index 5045fbf02fd31759a77a54643f5ae4682fd2bb49..1e23a15f7565288f202d7a3a2861c80dd7540f16 100644 (file)
@@ -85,8 +85,10 @@ void JSString::resolveRope(ExecState* exec) const
     if (PassRefPtr<UStringImpl> newImpl = UStringImpl::tryCreateUninitialized(m_stringLength, buffer))
         m_value = newImpl;
     else {
-        for (unsigned i = 0; i < m_ropeLength; ++i)
+        for (unsigned i = 0; i < m_ropeLength; ++i) {
             m_fibers[i].deref();
+            m_fibers[i] = static_cast<void*>(0);
+        }
         m_ropeLength = 0;
         ASSERT(!isRope());
         ASSERT(m_value == UString());
@@ -120,8 +122,10 @@ void JSString::resolveRope(ExecState* exec) const
             if (workQueue.isEmpty()) {
                 // Create a string from the UChar buffer, clear the rope RefPtr.
                 ASSERT(buffer == position);
-                for (unsigned i = 0; i < m_ropeLength; ++i)
+                for (unsigned i = 0; i < m_ropeLength; ++i) {
                     m_fibers[i].deref();
+                    m_fibers[i] = static_cast<void*>(0);
+                }
                 m_ropeLength = 0;
 
                 ASSERT(!isRope());
index 966ebefc5f16933d9b67ae12032557fdf98545a9..4a10df0d2ec666ea9c8a47aa81e7140b3b640c20 100644 (file)
@@ -59,6 +59,9 @@ namespace JSC {
     JSString* jsOwnedString(JSGlobalData*, const UString&); 
     JSString* jsOwnedString(ExecState*, const UString&); 
 
+    typedef void (*JSStringFinalizerCallback)(JSString*, void* context);
+    JSString* jsStringWithFinalizer(ExecState*, const UString&, JSStringFinalizerCallback callback, void* context);
+
     class JS_EXPORTCLASS JSString : public JSCell {
     public:
         friend class JIT;
@@ -71,10 +74,12 @@ namespace JSC {
             // Each Fiber in a rope is either UString::Rep or another Rope.
             class Fiber {
             public:
-                Fiber() {}
+                Fiber() : m_value(0) {}
                 Fiber(UString::Rep* string) : m_value(reinterpret_cast<intptr_t>(string)) {}
                 Fiber(Rope* rope) : m_value(reinterpret_cast<intptr_t>(rope) | 1) {}
 
+                Fiber(void* nonFiber) : m_value(reinterpret_cast<intptr_t>(nonFiber)) {}
+
                 void deref()
                 {
                     if (isRope())
@@ -109,6 +114,7 @@ namespace JSC {
                 bool isString() { return !isRope(); }
                 UString::Rep* string() { return reinterpret_cast<UString::Rep*>(m_value); }
 
+                void* nonFiber() { return reinterpret_cast<void*>(m_value); }
             private:
                 intptr_t m_value;
             };
@@ -245,11 +251,28 @@ namespace JSC {
             ASSERT(index == s_maxInternalRopeLength);
         }
 
+        JSString(JSGlobalData* globalData, const UString& value, JSStringFinalizerCallback finalizer, void* context)
+            : JSCell(globalData->stringStructure.get())
+            , m_stringLength(value.size())
+            , m_value(value)
+            , m_ropeLength(0)
+        {
+            // nasty hack because we can't union non-POD types
+            m_fibers[0] = reinterpret_cast<void*>(finalizer);
+            m_fibers[1] = context;
+            Heap::heap(this)->reportExtraMemoryCost(value.cost());
+        }
+
         ~JSString()
         {
             ASSERT(vptr() == JSGlobalData::jsStringVPtr);
             for (unsigned i = 0; i < m_ropeLength; ++i)
                 m_fibers[i].deref();
+
+            if (!m_ropeLength && m_fibers[0].nonFiber()) {
+                JSStringFinalizerCallback finalizer = reinterpret_cast<JSStringFinalizerCallback>(m_fibers[0].nonFiber());
+                finalizer(this, m_fibers[1].nonFiber());
+            }
         }
 
         const UString& value(ExecState* exec) const
@@ -347,6 +370,7 @@ namespace JSC {
         friend JSValue jsString(ExecState* exec, JSString* s1, const UString& u2);
         friend JSValue jsString(ExecState* exec, Register* strings, unsigned count);
         friend JSValue jsString(ExecState* exec, JSValue thisValue, const ArgList& args);
+        friend JSString* jsStringWithFinalizer(ExecState*, const UString&, JSStringFinalizerCallback callback, void* context);
     };
 
     JSString* asString(JSValue);
@@ -421,6 +445,13 @@ namespace JSC {
         return fixupVPtr(globalData, new (globalData) JSString(globalData, s));
     }
 
+    inline JSString* jsStringWithFinalizer(ExecState* exec, const UString& s, JSStringFinalizerCallback callback, void* context)
+    {
+        ASSERT(s.size() && (s.size() > 1 || s.data()[0] > 0xFF));
+        JSGlobalData* globalData = &exec->globalData();
+        return fixupVPtr(globalData, new (globalData) JSString(globalData, s, callback, context));
+    }
+
     inline JSString* jsSubstring(JSGlobalData* globalData, const UString& s, unsigned offset, unsigned length)
     {
         ASSERT(offset <= static_cast<unsigned>(s.size()));
index abf1d79759d153860a34e25d68f57985a614f478..465cb56749c09acedb1778b76e0eea94b6363073 100644 (file)
@@ -26,6 +26,7 @@
 #ifndef WeakGCMap_h
 #define WeakGCMap_h
 
+#include "Collector.h"
 #include <wtf/HashMap.h>
 
 namespace JSC {
index 9f8522c83d079ee417249f14d29d4111bc15008c..a109fad5beb5acb2cfe9f6c7f226657eb972e019 100644 (file)
@@ -1,3 +1,32 @@
+2010-01-16  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        Cache JS string values made from DOM strings (Dromaeo speedup)
+        https://bugs.webkit.org/show_bug.cgi?id=33768
+        <rdar://problem/7353576>
+
+        Added a new cache for JSString values that are created from Strings or AtomicStrings
+        in the DOM. It's common for the same string to be retrieved from the DOM repeatedly,
+        and it is wasteful to make a new JS-level string value every time.
+        
+        The string cache is per-world, and thus thread-safe and not a
+        vector for accidental information exchange.
+        
+        ~30% speedup on Dromaeo Attributes test, also substantially helps other Dromaeo DOM tests.
+
+        * bindings/js/JSDOMBinding.cpp:
+        (WebCore::jsStringCache): Helper function to get the string cache for the current world.
+        (WebCore::jsString): Some new overloads including the caching version.
+        (WebCore::stringWrapperDestroyed): Finalizer callback - remove from relevant caches.
+        * bindings/js/JSDOMBinding.h:
+        (WebCore::jsString): Prototype new overloads (and define a few inline).
+        * bindings/js/JSJavaScriptCallFrameCustom.cpp:
+        (WebCore::JSJavaScriptCallFrame::type): Explicitly make a UString.
+        * bindings/js/ScriptFunctionCall.cpp:
+        (WebCore::ScriptFunctionCall::appendArgument): Ditto.
+        * WebCore.base.exp: Add new JSString overloads that WebCore gets to see.
+
 2010-01-16  Oliver Hunt  <oliver@apple.com>
 
         Reviewed by Nikolas Zimmermann.
index b26ddd974fab3407d9daf1632723586a6cef9af6..7aeb6d0d900669a1094571223d01737ef6d2ee4e 100644 (file)
@@ -478,6 +478,7 @@ __ZN7WebCore4KURLC1ENS_18ParsedURLStringTagERKNS_6StringE
 __ZN7WebCore4KURLC1EP5NSURL
 __ZN7WebCore4Node17stopIgnoringLeaksEv
 __ZN7WebCore4Node18startIgnoringLeaksEv
+__ZN7WebCore4Node19setNeedsStyleRecalcENS_15StyleChangeTypeE
 __ZN7WebCore4Page12setGroupNameERKNS_6StringE
 __ZN7WebCore4Page13didStopPluginEPNS_14HaltablePluginE
 __ZN7WebCore4Page14didStartPluginEPNS_14HaltablePluginE
@@ -673,6 +674,7 @@ __ZN7WebCore8Settings40setJavaScriptCanOpenWindowsAutomaticallyEb
 __ZN7WebCore8Settings40setTextDirectionSubmenuInclusionBehaviorENS_37TextDirectionSubmenuInclusionBehaviorE
 __ZN7WebCore8Settings41setNeedsKeyboardEventDisambiguationQuirksEb
 __ZN7WebCore8blankURLEv
+__ZN7WebCore8jsStringEPN3JSC9ExecStateERKNS_6StringE
 __ZN7WebCore8makeRGBAEiiii
 __ZN7WebCore9DOMWindow30dispatchAllPendingUnloadEventsEv
 __ZN7WebCore9DOMWindow36dispatchAllPendingBeforeUnloadEventsEv
@@ -883,7 +885,6 @@ __ZNK7WebCore4KURLcvP5NSURLEv
 __ZNK7WebCore4Node14isDescendantOfEPKS0_
 __ZNK7WebCore4Node18getSubresourceURLsERN3WTF11ListHashSetINS_4KURLENS_8KURLHashEEE
 __ZNK7WebCore4Node9nodeIndexEv
-__ZN7WebCore4Node19setNeedsStyleRecalcENS_15StyleChangeTypeE
 __ZNK7WebCore4Page10pluginDataEv
 __ZNK7WebCore4Page34inLowQualityImageInterpolationModeEv
 __ZNK7WebCore4Page9groupNameEv
@@ -1019,6 +1020,7 @@ _wkGetNSURLResponseCalculatedExpiration
 _wkGetNSURLResponseLastModifiedDate
 _wkGetNSURLResponseMustRevalidate
 _wkGetPreferredExtensionForMIMEType
+_wkGetUserToBaseCTM
 _wkGetWheelEventDeltas
 _wkHitTestMediaUIPart
 _wkInitializeMaximumHTTPConnectionCountPerHost
@@ -1044,7 +1046,6 @@ _wkSetNSURLConnectionDefersCallbacks
 _wkSetNSURLRequestShouldContentSniff
 _wkSetPatternBaseCTM
 _wkSetPatternPhaseInUserSpace
-_wkGetUserToBaseCTM
 _wkSetUpFontCache
 _wkSignalCFReadStreamEnd
 _wkSignalCFReadStreamError
index e98af019ab9082c2aac70fa6dd96f857abd1fa75..1c934c74348400cedf2b06113291a9ce87dc85d4 100644 (file)
@@ -556,6 +556,59 @@ void markDOMNodeWrapper(MarkStack& markStack, Document* document, Node* node)
     }
 }
 
+static inline JSStringCache& jsStringCache(ExecState* exec)
+{
+    return currentWorld(exec)->m_stringCache;
+}
+
+static void stringWrapperDestroyed(JSString* str, void* context)
+{
+    StringImpl* cacheKey = static_cast<StringImpl*>(context);
+    JSC::JSGlobalData* globalData = Heap::heap(str)->globalData();
+
+    // Check the normal world first!
+    JSGlobalData::ClientData* clientData = globalData->clientData;
+    ASSERT(clientData);
+    JSStringCache& cache = static_cast<WebCoreJSClientData*>(clientData)->normalWorld()->m_stringCache;
+    if (cache.uncheckedRemove(cacheKey, str)) {
+        cacheKey->deref();
+        return;
+    }
+
+    for (JSGlobalDataWorldIterator worldIter(globalData); worldIter; ++worldIter) {
+        if (worldIter->m_stringCache.uncheckedRemove(cacheKey, str))
+            break;
+    }
+
+    cacheKey->deref();
+}
+
+JSValue jsString(ExecState* exec, const String& s)
+{
+    StringImpl* stringImpl = s.impl();
+    if (!stringImpl || !stringImpl->length())
+        return jsEmptyString(exec);
+
+    if (stringImpl->length() == 1 && stringImpl->characters()[0] <= 0xFF)
+        return jsString(exec, stringImpl->ustring());
+
+    JSStringCache& stringCache = jsStringCache(exec);
+    if (JSString* wrapper = stringCache.get(stringImpl))
+        return wrapper;
+
+    // If there is a stale entry, we have to explicitly remove it to avoid
+    // problems down the line.
+    if (JSString* wrapper = stringCache.uncheckedGet(stringImpl))
+        stringCache.uncheckedRemove(stringImpl, wrapper);
+
+    JSString* wrapper = jsStringWithFinalizer(exec, stringImpl->ustring(), stringWrapperDestroyed, stringImpl);
+    stringCache.set(stringImpl, wrapper);
+    // ref explicitly instead of using a RefPtr-keyed hashtable because the wrapper can
+    // outlive the cache, so the stringImpl has to match the wrapper's lifetime.
+    stringImpl->ref();
+    return wrapper;
+}
+
 JSValue jsStringOrNull(ExecState* exec, const String& s)
 {
     if (s.isNull())
@@ -584,6 +637,11 @@ JSValue jsStringOrFalse(ExecState* exec, const String& s)
     return jsString(exec, s);
 }
 
+JSValue jsString(ExecState* exec, const KURL& url)
+{
+    return jsString(exec, url.string());
+}
+
 JSValue jsStringOrNull(ExecState* exec, const KURL& url)
 {
     if (url.isNull())
index c84756052f9e2133d3d14396ba9f7ac4d4afce7d..e20a2320a559d364b29ece01abda6479705fdaed 100644 (file)
@@ -141,6 +141,7 @@ namespace WebCore {
     };
 
     typedef JSC::WeakGCMap<void*, DOMObject*> DOMObjectWrapperMap;
+    typedef JSC::WeakGCMap<StringImpl*, JSC::JSString*> JSStringCache; 
 
     class DOMWrapperWorld : public RefCounted<DOMWrapperWorld> {
     public:
@@ -152,6 +153,7 @@ namespace WebCore {
 
         // FIXME: can we make this private?
         DOMObjectWrapperMap m_wrappers;
+        JSStringCache m_stringCache;
 
         bool isNormal() const { return m_isNormal; }
 
@@ -343,6 +345,13 @@ namespace WebCore {
     // Convert a DOM implementation exception code into a JavaScript exception in the execution state.
     void setDOMException(JSC::ExecState*, ExceptionCode);
 
+    JSC::JSValue jsString(JSC::ExecState*, const String&); // empty if the string is null
+    JSC::JSValue jsString(JSC::ExecState*, const KURL&); // empty if the URL is null
+    inline JSC::JSValue jsString(JSC::ExecState* exec, const AtomicString& s)
+    { 
+        return jsString(exec, s.string());
+    }
+        
     JSC::JSValue jsStringOrNull(JSC::ExecState*, const String&); // null if the string is null
     JSC::JSValue jsStringOrNull(JSC::ExecState*, const KURL&); // null if the URL is null
 
index 08ecf2b1263cefa2070a1a2d0ed361d7753e3565..afbdf5d1d358117bc6c9a16f732be5438e729699 100644 (file)
@@ -55,9 +55,9 @@ JSValue JSJavaScriptCallFrame::type(ExecState* exec) const
 {
     switch (impl()->type()) {
         case DebuggerCallFrame::FunctionType:
-            return jsString(exec, "function");
+            return jsString(exec, UString("function"));
         case DebuggerCallFrame::ProgramType:
-            return jsString(exec, "program");
+            return jsString(exec, UString("program"));
     }
 
     ASSERT_NOT_REACHED();
index e38acb910cc433077d242205d0e6101aa87c7b13..a2284bc60493c6b133d6e00bd78e72080be750f7 100644 (file)
@@ -79,7 +79,7 @@ void ScriptFunctionCall::appendArgument(const JSC::UString& argument)
 void ScriptFunctionCall::appendArgument(const char* argument)
 {
     JSLock lock(SilenceAssertionsOnly);
-    m_arguments.append(jsString(m_exec, argument));
+    m_arguments.append(jsString(m_exec, UString(argument)));
 }
 
 void ScriptFunctionCall::appendArgument(JSC::JSValue argument)
index 85c58c9112d0010aa89768f1680a933d90e8cf5a..7676d413bc07d95c0985448b0b796a0a499712fb 100644 (file)
@@ -1,3 +1,15 @@
+2010-01-16  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        Cache JS string values made from DOM strings (Dromaeo speedup)
+        https://bugs.webkit.org/show_bug.cgi?id=33768
+        <rdar://problem/7353576>
+        
+        * Plugins/Hosted/ProxyInstance.mm:
+        (WebKit::ProxyInstance::stringValue): Explicitly make a String, since char*
+        is now ambiguous.
+
 2010-01-13  Simon Fraser  <simon.fraser@apple.com>
 
         Reviewed by Darin Adler.
index 6be3953735ef4e035e7c19279cc9581507a7a0d7..92ef8ba8a84b29a6b9cbfbe16ec4e30ad46ef35a 100644 (file)
@@ -223,7 +223,7 @@ JSValue ProxyInstance::defaultValue(ExecState* exec, PreferredPrimitiveType hint
 JSValue ProxyInstance::stringValue(ExecState* exec) const
 {
     // FIXME: Implement something sensible.
-    return jsString(exec, "");
+    return jsEmptyString(exec);
 }
 
 JSValue ProxyInstance::numberValue(ExecState* exec) const