Fast path for jsStringWithCache() when asked for the same string repeatedly.

<https://webkit.org/b/134635>

Source/JavaScriptCore:
Also moved the whole thing from WebCore to JavaScriptCore since it
makes more sense here, and inline the lightweight checks, leaving only
the hashmap stuff out of line.

Reviewed by Darin Adler.

* runtime/JSString.cpp:
(JSC::jsStringWithCacheSlowCase):
* runtime/JSString.h:
(JSC::jsStringWithCache):
* runtime/VM.h:

Source/WebCore:
Reviewed by Darin Adler.

* WebCore.exp.in:
* bindings/js/JSDOMBinding.cpp:
(WebCore::jsStringWithCache): Deleted.
* bindings/js/JSDOMBinding.h:
(WebCore::JSValueTraits<String>::arrayJSValue):
(WebCore::jsStringWithCache): Deleted.
* bridge/c/c_utility.cpp:
(JSC::Bindings::convertNPVariantToValue):
* loader/cache/CachedResourceHandle.h:

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

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 4d6ac6a..07dacd2 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,20 @@
+2014-07-04  Andreas Kling  <akling@apple.com>
+
+        Fast path for jsStringWithCache() when asked for the same string repeatedly.
+        <https://webkit.org/b/134635>
+
+        Also moved the whole thing from WebCore to JavaScriptCore since it
+        makes more sense here, and inline the lightweight checks, leaving only
+        the hashmap stuff out of line.
+
+        Reviewed by Darin Adler.
+
+        * runtime/JSString.cpp:
+        (JSC::jsStringWithCacheSlowCase):
+        * runtime/JSString.h:
+        (JSC::jsStringWithCache):
+        * runtime/VM.h:
+
 2014-07-03  Daniel Bates  <dabates@apple.com>
 
         Add WTF::move()

diff --git a/Source/JavaScriptCore/runtime/JSString.cpp b/Source/JavaScriptCore/runtime/JSString.cpp
index 1937e3f..641d8b6 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSString.cpp
+++ b/Source/JavaScriptCore/runtime/JSString.cpp
@@ -374,4 +374,13 @@ bool JSString::getStringPropertyDescriptor(ExecState* exec, PropertyName propert
     return false;
 }
 
+JSString* jsStringWithCacheSlowCase(VM& vm, StringImpl& stringImpl)
+{
+    auto addResult = vm.stringCache.add(&stringImpl, nullptr);
+    if (addResult.isNewEntry)
+        addResult.iterator->value = jsString(&vm, String(stringImpl));
+    vm.lastCachedString = addResult.iterator->value.get();
+    return vm.lastCachedString.get();
+}
+
 } // namespace JSC

diff --git a/Source/JavaScriptCore/runtime/JSString.h b/Source/JavaScriptCore/runtime/JSString.h
index aa869bf..1287b66 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSString.h
+++ b/Source/JavaScriptCore/runtime/JSString.h
@@ -516,6 +516,34 @@ namespace JSC {
     inline JSString* jsNontrivialString(ExecState* exec, const String& s) { return jsNontrivialString(&exec->vm(), s); }
     inline JSString* jsOwnedString(ExecState* exec, const String& s) { return jsOwnedString(&exec->vm(), s); }
 
+    JS_EXPORT_PRIVATE JSString* jsStringWithCacheSlowCase(VM&, StringImpl&);
+
+    ALWAYS_INLINE JSString* jsStringWithCache(ExecState* exec, const String& s)
+    {
+        VM& vm = exec->vm();
+        StringImpl* stringImpl = s.impl();
+        if (!stringImpl || !stringImpl->length())
+            return jsEmptyString(&vm);
+
+        if (stringImpl->length() == 1) {
+            UChar singleCharacter = (*stringImpl)[0u];
+            if (singleCharacter <= maxSingleCharacterString)
+                return vm.smallStrings.singleCharacterString(static_cast<unsigned char>(singleCharacter));
+        }
+
+        if (JSString* lastCachedString = vm.lastCachedString.get()) {
+            if (lastCachedString->tryGetValueImpl() == stringImpl)
+                return lastCachedString;
+        }
+
+        return jsStringWithCacheSlowCase(vm, *stringImpl);
+    }
+
+    ALWAYS_INLINE JSString* jsStringWithCache(ExecState* exec, const AtomicString& s)
+    {
+        return jsStringWithCache(exec, s.string());
+    }
+
     ALWAYS_INLINE bool JSString::getStringPropertySlot(ExecState* exec, PropertyName propertyName, PropertySlot& slot)
     {
         if (propertyName == exec->propertyNames().length) {

diff --git a/Source/JavaScriptCore/runtime/VM.h b/Source/JavaScriptCore/runtime/VM.h
index 5d648c6..3251809 100644 (file)
--- a/Source/JavaScriptCore/runtime/VM.h
+++ b/Source/JavaScriptCore/runtime/VM.h
@@ -297,6 +297,7 @@ namespace JSC {
         DateInstanceCache dateInstanceCache;
         WTF::SimpleStats machineCodeBytesPerBytecodeWordForBaselineJIT;
         WeakGCMap<StringImpl*, JSString, PtrHash<StringImpl*>> stringCache;
+        Weak<JSString> lastCachedString;
 
         AtomicStringTable* atomicStringTable() const { return m_atomicStringTable; }
 

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index ebfb5bc..b19955d 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,20 @@
+2014-07-04  Andreas Kling  <akling@apple.com>
+
+        Fast path for jsStringWithCache() when asked for the same string repeatedly.
+        <https://webkit.org/b/134635>
+
+        Reviewed by Darin Adler.
+
+        * WebCore.exp.in:
+        * bindings/js/JSDOMBinding.cpp:
+        (WebCore::jsStringWithCache): Deleted.
+        * bindings/js/JSDOMBinding.h:
+        (WebCore::JSValueTraits<String>::arrayJSValue):
+        (WebCore::jsStringWithCache): Deleted.
+        * bridge/c/c_utility.cpp:
+        (JSC::Bindings::convertNPVariantToValue):
+        * loader/cache/CachedResourceHandle.h:
+
 2014-07-04  Zalan Bujtas  <zalan@apple.com>
 
         Subpixel rendering: ebay.com rotating billboard on the main page has cut off buttons.

diff --git a/Source/WebCore/WebCore.exp.in b/Source/WebCore/WebCore.exp.in
index 6acc3a3..91b0424 100644 (file)
--- a/Source/WebCore/WebCore.exp.in
+++ b/Source/WebCore/WebCore.exp.in
@@ -766,7 +766,6 @@ __ZN7WebCore17SQLiteTransactionD1Ev
 __ZN7WebCore17SubresourceLoader6createEPNS_5FrameEPNS_14CachedResourceERKNS_15ResourceRequestERKNS_21ResourceLoaderOptionsE
 __ZN7WebCore17cacheDOMStructureEPNS_17JSDOMGlobalObjectEPN3JSC9StructureEPKNS2_9ClassInfoE
 __ZN7WebCore17encodeForFileNameERKN3WTF6StringE
-__ZN7WebCore17jsStringWithCacheEPN3JSC9ExecStateERKN3WTF6StringE
 __ZN7WebCore17languageDidChangeEv
 __ZN7WebCore17openTemporaryFileERKN3WTF6StringERi
 __ZN7WebCore17sRGBColorSpaceRefEv

diff --git a/Source/WebCore/bindings/js/JSDOMBinding.cpp b/Source/WebCore/bindings/js/JSDOMBinding.cpp
index d9c47e3..0567ecc 100644 (file)
--- a/Source/WebCore/bindings/js/JSDOMBinding.cpp
+++ b/Source/WebCore/bindings/js/JSDOMBinding.cpp
@@ -63,25 +63,6 @@ const JSC::HashTable& getHashTableForGlobalData(VM& vm, const JSC::HashTable& st
     return DOMObjectHashTableMap::mapFor(vm).get(staticTable);
 }
 
-JSC::JSValue jsStringWithCache(JSC::ExecState* exec, const String& s)
-{
-    JSC::VM& vm = exec->vm();
-    StringImpl* stringImpl = s.impl();
-    if (!stringImpl || !stringImpl->length())
-        return jsEmptyString(&vm);
-
-    if (stringImpl->length() == 1) {
-        UChar singleCharacter = (*stringImpl)[0u];
-        if (singleCharacter <= JSC::maxSingleCharacterString)
-            return vm.smallStrings.singleCharacterString(static_cast<unsigned char>(singleCharacter));
-    }
-
-    auto addResult = vm.stringCache.add(stringImpl, nullptr);
-    if (addResult.isNewEntry)
-        addResult.iterator->value = JSC::jsString(&vm, String(stringImpl));
-    return JSC::JSValue(addResult.iterator->value.get());
-}
-
 JSValue jsStringOrNull(ExecState* exec, const String& s)
 {
     if (s.isNull())

diff --git a/Source/WebCore/bindings/js/JSDOMBinding.h b/Source/WebCore/bindings/js/JSDOMBinding.h
index f2f19fc..ab5db0b 100644 (file)
--- a/Source/WebCore/bindings/js/JSDOMBinding.h
+++ b/Source/WebCore/bindings/js/JSDOMBinding.h
@@ -268,12 +268,7 @@ void reportCurrentException(JSC::ExecState*);
 // Convert a DOM implementation exception code into a JavaScript exception in the execution state.
 void setDOMException(JSC::ExecState*, ExceptionCode);
 
-JSC::JSValue jsStringWithCache(JSC::ExecState*, const String&);
 JSC::JSValue jsString(JSC::ExecState*, const URL&); // empty if the URL is null
-inline JSC::JSValue jsStringWithCache(JSC::ExecState* exec, const AtomicString& s)
-{
-    return jsStringWithCache(exec, s.string());
-}
 
 JSC::JSValue jsStringOrNull(JSC::ExecState*, const String&); // null if the string is null
 JSC::JSValue jsStringOrNull(JSC::ExecState*, const URL&); // null if the URL is null
@@ -423,7 +418,7 @@ template<typename T> struct JSValueTraits {
 template<> struct JSValueTraits<String> {
     static JSC::JSValue arrayJSValue(JSC::ExecState* exec, JSDOMGlobalObject*, const String& value)
     {
-        return jsStringWithCache(exec, value);
+        return JSC::jsStringWithCache(exec, value);
     }
 };
 

diff --git a/Source/WebCore/bridge/c/c_utility.cpp b/Source/WebCore/bridge/c/c_utility.cpp
index 3880fac..774fdfc 100644 (file)
--- a/Source/WebCore/bridge/c/c_utility.cpp
+++ b/Source/WebCore/bridge/c/c_utility.cpp
@@ -122,7 +122,7 @@ JSValue convertNPVariantToValue(ExecState* exec, const NPVariant* variant, RootO
     if (type == NPVariantType_Double)
         return jsNumber(NPVARIANT_TO_DOUBLE(*variant));
     if (type == NPVariantType_String)
-        return WebCore::jsStringWithCache(exec, convertNPStringToUTF16(&variant->value.stringValue));
+        return jsStringWithCache(exec, convertNPStringToUTF16(&variant->value.stringValue));
     if (type == NPVariantType_Object) {
         NPObject* obj = variant->value.objectValue;
         

diff --git a/Source/WebKit/WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in b/Source/WebKit/WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in
index 3677e92..33e4dd0 100644 (file)
--- a/Source/WebKit/WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in
+++ b/Source/WebKit/WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in
@@ -237,7 +237,6 @@ EXPORTS
         symbolWithPointer(?isActiveInsertionPoint@WebCore@@YA_NPBVNode@1@@Z, ?isActiveInsertionPoint@WebCore@@YA_NPEBVNode@1@@Z)
         symbolWithPointer(?isPreloaded@CachedResourceLoader@WebCore@@QBE_NABVString@WTF@@@Z, ?isPreloaded@CachedResourceLoader@WebCore@@QEBA_NAEBVString@WTF@@@Z)
         symbolWithPointer(?jsArray@WebCore@@YA?AVJSValue@JSC@@PAVExecState@3@PAVJSDOMGlobalObject@1@V?$PassRefPtr@VDOMStringList@WebCore@@@WTF@@@Z, ?jsArray@WebCore@@YA?AVJSValue@JSC@@PEAVExecState@3@PEAVJSDOMGlobalObject@1@V?$PassRefPtr@VDOMStringList@WebCore@@@WTF@@@Z)
-        symbolWithPointer(?jsStringWithCache@WebCore@@YA?AVJSValue@JSC@@PAVExecState@3@ABVString@WTF@@@Z, ?jsStringWithCache@WebCore@@YA?AVJSValue@JSC@@PEAVExecState@3@AEBVString@WTF@@@Z)
         symbolWithPointer(?lastChangeWasUserEdit@HTMLTextFormControlElement@WebCore@@QBE_NXZ, ?lastChangeWasUserEdit@HTMLTextFormControlElement@WebCore@@QEBA_NXZ)
         symbolWithPointer(?synchronousScrollingReasonsAsText@Page@WebCore@@QAE?AVString@WTF@@XZ, ?synchronousScrollingReasonsAsText@Page@WebCore@@QEAA?AVString@WTF@@XZ)
         symbolWithPointer(?markerTextForListItem@WebCore@@YA?AVString@WTF@@PAVElement@1@@Z, ?markerTextForListItem@WebCore@@YA?AVString@WTF@@PEAVElement@1@@Z)