Attachment 342882 Details for Bug 186723 – Patch

[patch] Patch

bug-186723-20180617021211.patch (text/plain), 6.38 KB, created by Yusuke Suzuki on 2018-06-16 10:12:12 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-06-16 10:12:12 PDT

Size: 6.38 KB

patch

obsolete

>Subversion Revision: 232904
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 4713f1994e2e6c4dcca650d0bc88fbd8daed2463..676c95aadb3c58e82645b85eb12745840f9e2b5a 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2018-06-16  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        [JSC] Inline JSArray::pushInline and Structure::nonPropertyTransition
>+        https://bugs.webkit.org/show_bug.cgi?id=186723
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Now, CoW -> non-CoW transition is heavy path. We inline the part of Structure::nonPropertyTransition
>+        to catch the major path. And we also inline JSArray::pushInline well to spread this in operationArrayPushMultiple.
>+
>+        This patch improves SixSpeed/spread-literal.es5.
>+
>+                                     baseline                  patched
>+
>+        spread-literal.es5      114.4140+-4.5146     ^    104.5475+-3.6157        ^ definitely 1.0944x faster
>+
>+        * runtime/JSArrayInlines.h:
>+        (JSC::JSArray::pushInline):
>+        * runtime/Structure.cpp:
>+        (JSC::Structure::nonPropertyTransitionSlow):
>+        (JSC::Structure::nonPropertyTransition): Deleted.
>+        * runtime/Structure.h:
>+        * runtime/StructureInlines.h:
>+        (JSC::Structure::nonPropertyTransition):
>+
> 2018-06-16  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         [DFG] Reduce OSRExit for Kraken/crypto-aes due to CoW array
>diff --git a/Source/JavaScriptCore/runtime/JSArrayInlines.h b/Source/JavaScriptCore/runtime/JSArrayInlines.h
>index cfe73baf69bcc2fd08a4d810f1f43b5c31199e59..39338d047a473036bf9e1403cd86cfa968fbb71f 100644
>--- a/Source/JavaScriptCore/runtime/JSArrayInlines.h
>+++ b/Source/JavaScriptCore/runtime/JSArrayInlines.h
>@@ -83,11 +83,12 @@ ALWAYS_INLINE double toLength(ExecState* exec, JSObject* obj)
>     return lengthValue.toLength(exec);
> }
> 
>-inline void JSArray::pushInline(ExecState* exec, JSValue value)
>+ALWAYS_INLINE void JSArray::pushInline(ExecState* exec, JSValue value)
> {
>     VM& vm = exec->vm();
>     auto scope = DECLARE_THROW_SCOPE(vm);
> 
>+reloop:
>     Butterfly* butterfly = this->butterfly();
> 
>     switch (indexingMode()) {
>@@ -230,9 +231,7 @@ inline void JSArray::pushInline(ExecState* exec, JSValue value)
>     default: {
>         RELEASE_ASSERT(isCopyOnWrite(indexingMode()));
>         convertFromCopyOnWrite(vm);
>-        scope.release();
>-        // Reloop.
>-        return pushInline(exec, value);
>+        goto reloop;
>     }
>     }
> }
>diff --git a/Source/JavaScriptCore/runtime/Structure.cpp b/Source/JavaScriptCore/runtime/Structure.cpp
>index a7fef4542263f0fcacc2af786000d39babbac0f7..0590e5f80673f2c6f5a493ea2123b59ba356f430 100644
>--- a/Source/JavaScriptCore/runtime/Structure.cpp
>+++ b/Source/JavaScriptCore/runtime/Structure.cpp
>@@ -646,23 +646,11 @@ PropertyTable* Structure::takePropertyTableOrCloneIfPinned(VM& vm)
>     return materializePropertyTable(vm, setPropertyTable);
> }
> 
>-Structure* Structure::nonPropertyTransition(VM& vm, Structure* structure, NonPropertyTransition transitionKind)
>+Structure* Structure::nonPropertyTransitionSlow(VM& vm, Structure* structure, NonPropertyTransition transitionKind)
> {
>     unsigned attributes = toAttributes(transitionKind);
>     IndexingType indexingModeIncludingHistory = newIndexingType(structure->indexingModeIncludingHistory(), transitionKind);
>     
>-    if (changesIndexingType(transitionKind)) {
>-        if (JSGlobalObject* globalObject = structure->m_globalObject.get()) {
>-            if (globalObject->isOriginalArrayStructure(structure)) {
>-                Structure* result = globalObject->originalArrayStructureForIndexingType(indexingModeIncludingHistory);
>-                if (result->indexingModeIncludingHistory() == indexingModeIncludingHistory) {
>-                    structure->didTransitionFromThisStructure();
>-                    return result;
>-                }
>-            }
>-        }
>-    }
>-    
>     Structure* existingTransition;
>     if (!structure->isDictionary() && (existingTransition = structure->m_transitionTable.get(0, attributes))) {
>         ASSERT(existingTransition->attributesInPrevious() == attributes);
>diff --git a/Source/JavaScriptCore/runtime/Structure.h b/Source/JavaScriptCore/runtime/Structure.h
>index 503ce4626b1102c8fab484a1e42e15fc40f68313..a8bd4bb5db99e77d83b890da165093b1e3ff6ef8 100644
>--- a/Source/JavaScriptCore/runtime/Structure.h
>+++ b/Source/JavaScriptCore/runtime/Structure.h
>@@ -195,7 +195,8 @@ class Structure final : public JSCell {
>     JS_EXPORT_PRIVATE static Structure* sealTransition(VM&, Structure*);
>     JS_EXPORT_PRIVATE static Structure* freezeTransition(VM&, Structure*);
>     static Structure* preventExtensionsTransition(VM&, Structure*);
>-    JS_EXPORT_PRIVATE static Structure* nonPropertyTransition(VM&, Structure*, NonPropertyTransition);
>+    static Structure* nonPropertyTransition(VM&, Structure*, NonPropertyTransition);
>+    JS_EXPORT_PRIVATE static Structure* nonPropertyTransitionSlow(VM&, Structure*, NonPropertyTransition);
> 
>     JS_EXPORT_PRIVATE bool isSealed(VM&);
>     JS_EXPORT_PRIVATE bool isFrozen(VM&);
>diff --git a/Source/JavaScriptCore/runtime/StructureInlines.h b/Source/JavaScriptCore/runtime/StructureInlines.h
>index 6bd8ef4fe28820605724cb1334e78fef2606cbf6..1c94628ccab0e161c1353ea1224905736e6de8e6 100644
>--- a/Source/JavaScriptCore/runtime/StructureInlines.h
>+++ b/Source/JavaScriptCore/runtime/StructureInlines.h
>@@ -523,5 +523,24 @@ ALWAYS_INLINE bool Structure::shouldConvertToPolyProto(const Structure* a, const
> 
>     return !aObj && !bObj;
> }
>-    
>+
>+inline Structure* Structure::nonPropertyTransition(VM& vm, Structure* structure, NonPropertyTransition transitionKind)
>+{
>+    IndexingType indexingModeIncludingHistory = newIndexingType(structure->indexingModeIncludingHistory(), transitionKind);
>+
>+    if (changesIndexingType(transitionKind)) {
>+        if (JSGlobalObject* globalObject = structure->m_globalObject.get()) {
>+            if (globalObject->isOriginalArrayStructure(structure)) {
>+                Structure* result = globalObject->originalArrayStructureForIndexingType(indexingModeIncludingHistory);
>+                if (result->indexingModeIncludingHistory() == indexingModeIncludingHistory) {
>+                    structure->didTransitionFromThisStructure();
>+                    return result;
>+                }
>+            }
>+        }
>+    }
>+
>+    return nonPropertyTransitionSlow(vm, structure, transitionKind);
>+}
>+
> } // namespace JSC

Flags:

mark.lam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 186723: 342882