Changeset 201436 in webkit

Timestamp:

May 26, 2016, 3:30:05 PM (10 years ago)

Author:

ggaren@apple.com

Message:

REGRESSION: JSBench spends a lot of time transitioning to/from dictionary
​https://bugs.webkit.org/show_bug.cgi?id=158045

Reviewed by Saam Barati.

15% speedup on jsbench-amazon-firefox, possibly 5% speedup overall on jsbench.

This regression seems to have two parts:

(1) Transitioning the window object to/from dictionary is more expensive
than it used to be to because the window object has lots more properties.
The window object has more properties because, for WebIDL compatibility,
we reify DOM APIs as properties when you delete.

(2) DOM prototypes transition to/from dictionary upon creation
because, once again for WebIDL compatibility, we reify their static
APIs eagerly.

The solution is to chill out a bit on dictionary transitions.

• bytecode/ObjectPropertyConditionSet.cpp: Don't flatten a dictionary

if we've already done so before. This avoids pathological churn, and it
is our idiom in other places.

• interpreter/Interpreter.cpp:

(JSC::Interpreter::execute): Do flatten the global object unconditionally
if it is an uncacheable dictionary because the global object is super
important.

• runtime/BatchedTransitionOptimizer.h:

(JSC::BatchedTransitionOptimizer::BatchedTransitionOptimizer):
(JSC::BatchedTransitionOptimizer::~BatchedTransitionOptimizer): Deleted.
Don't transition away from dictionary after a batched set of property
puts because normal dictionaries are cacheable and that's a perfectly
fine state to be in -- and the transition is expensive.

• runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::init): Do start the global object out as a cacheable
dictionary because it will inevitably have enough properties to become
a dictionary.

• runtime/Operations.h:

(JSC::normalizePrototypeChain): Same as ObjectPropertyConditionSet.cpp.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• bytecode/ObjectPropertyConditionSet.cpp (modified) (1 diff)
• interpreter/Interpreter.cpp (modified) (3 diffs)
• runtime/BatchedTransitionOptimizer.h (modified) (1 diff)
• runtime/JSGlobalObject.cpp (modified) (1 diff)
• runtime/Operations.cpp (modified) (1 diff)
• runtime/Operations.h (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-05-26  Geoffrey Garen  <ggaren@apple.com>
+
+        REGRESSION: JSBench spends a lot of time transitioning to/from dictionary
+        https://bugs.webkit.org/show_bug.cgi?id=158045
+
+        Reviewed by Saam Barati.
+
+        15% speedup on jsbench-amazon-firefox, possibly 5% speedup overall on jsbench.
+
+        This regression seems to have two parts:
+
+        (1) Transitioning the window object to/from dictionary is more expensive
+        than it used to be to because the window object has lots more properties.
+        The window object has more properties because, for WebIDL compatibility,
+        we reify DOM APIs as properties when you delete.
+
+        (2) DOM prototypes transition to/from dictionary upon creation
+        because, once again for WebIDL compatibility, we reify their static
+        APIs eagerly.
+
+        The solution is to chill out a bit on dictionary transitions.
+
+        * bytecode/ObjectPropertyConditionSet.cpp: Don't flatten a dictionary
+        if we've already done so before. This avoids pathological churn, and it
+        is our idiom in other places.
+
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::execute): Do flatten the global object unconditionally
+        if it is an uncacheable dictionary because the global object is super
+        important.
+
+        * runtime/BatchedTransitionOptimizer.h:
+        (JSC::BatchedTransitionOptimizer::BatchedTransitionOptimizer):
+        (JSC::BatchedTransitionOptimizer::~BatchedTransitionOptimizer): Deleted.
+        Don't transition away from dictionary after a batched set of property
+        puts because normal dictionaries are cacheable and that's a perfectly
+        fine state to be in -- and the transition is expensive.
+
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::init): Do start the global object out as a cacheable
+        dictionary because it will inevitably have enough properties to become
+        a dictionary.
+
+        * runtime/Operations.h:
+        (JSC::normalizePrototypeChain): Same as ObjectPropertyConditionSet.cpp.
+
 2016-05-25  Geoffrey Garen  <ggaren@apple.com>
 

trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp

@@ -264 +264 @@
         structure = object->structure(vm);
         
-        // Since we're accessing a prototype repeatedly, it's a good bet that it should not be
-        // treated as a dictionary.
         if (structure->isDictionary()) {
             if (concurrency == MainThread) {
+                if (structure->hasBeenFlattenedBefore()) {
+                    if (verbose)
+                        dataLog("Dictionary has been flattened before, so invalid.\n");
+                    return ObjectPropertyConditionSet::invalid();
+                }
                 if (verbose)
                     dataLog("Flattening ", pointerDump(structure));

trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

@@ -939 +939 @@
         return throwTerminatedExecutionException(callFrame);
 
+    if (scope->structure()->isUncacheableDictionary())
+        scope->flattenDictionaryObject(vm);
+
     ASSERT(codeBlock->numParameters() == 1); // 1 parameter for 'this'.
 
@@ -1187 +1190 @@
     }
 
+    if (variableObject->structure()->isUncacheableDictionary())
+        variableObject->flattenDictionaryObject(vm);
+
     if (numVariables || numFunctions) {
         BatchedTransitionOptimizer optimizer(vm, variableObject);
@@ -1243 +1249 @@
     if (UNLIKELY(vm.shouldTriggerTermination(callFrame)))
         return throwTerminatedExecutionException(callFrame);
+
+    if (scope->structure()->isUncacheableDictionary())
+        scope->flattenDictionaryObject(vm);
 
     ASSERT(codeBlock->numParameters() == 1); // 1 parameter for 'this'.

trunk/Source/JavaScriptCore/runtime/BatchedTransitionOptimizer.h

@@ -36 +36 @@
 public:
     BatchedTransitionOptimizer(VM& vm, JSObject* object)
-        : m_vm(&vm)
-        , m_object(object)
     {
-        if (!m_object->structure(vm)->isDictionary())
-            m_object->convertToDictionary(vm);
+        if (!object->structure(vm)->isDictionary())
+            object->convertToDictionary(vm);
     }
-
-    ~BatchedTransitionOptimizer()
-    {
-        if (m_object->structure()->isDictionary())
-            m_object->flattenDictionaryObject(*m_vm);
-    }
-
-private:
-    VM* m_vm;
-    JSObject* m_object;
 };
 

trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

@@ -320 +320 @@
     ASSERT(vm.currentThreadIsHoldingAPILock());
 
+    Base::setStructure(vm, Structure::toCacheableDictionaryTransition(vm, structure()));
+
     JSGlobalObject::globalExec()->init(0, 0, CallFrame::noCaller(), 0, 0);
 

trunk/Source/JavaScriptCore/runtime/Operations.cpp

@@ -121 +121 @@
 }
 
+size_t normalizePrototypeChain(CallFrame* callFrame, Structure* structure)
+{
+    VM& vm = callFrame->vm();
+    size_t count = 0;
+    while (1) {
+        if (structure->isProxy())
+            return InvalidPrototypeChain;
+        JSValue v = structure->prototypeForLookup(callFrame);
+        if (v.isNull())
+            return count;
+
+        JSCell* base = v.asCell();
+        structure = base->structure(vm);
+        if (structure->isDictionary()) {
+            if (structure->hasBeenFlattenedBefore())
+                return InvalidPrototypeChain;
+            structure->flattenDictionaryStructure(vm, asObject(base));
+        }
+
+        ++count;
+    }
+}
+
 } // namespace JSC

trunk/Source/JavaScriptCore/runtime/Operations.h

@@ -28 +28 @@
 
 namespace JSC {
+
+#define InvalidPrototypeChain (std::numeric_limits<size_t>::max())
 
 NEVER_INLINE JSValue jsAddSlowCase(CallFrame*, JSValue, JSValue);
@@ -34 +36 @@
 bool jsIsObjectTypeOrNull(CallFrame*, JSValue);
 bool jsIsFunctionType(JSValue);
+size_t normalizePrototypeChain(CallFrame*, Structure*);
 
 ALWAYS_INLINE JSValue jsString(ExecState* exec, JSString* s1, JSString* s2)
@@ -193 +196 @@
 }
 
-#define InvalidPrototypeChain (std::numeric_limits<size_t>::max())
-
-inline size_t normalizePrototypeChain(CallFrame* callFrame, Structure* structure)
-{
-    VM& vm = callFrame->vm();
-    size_t count = 0;
-    while (1) {
-        if (structure->isProxy())
-            return InvalidPrototypeChain;
-        JSValue v = structure->prototypeForLookup(callFrame);
-        if (v.isNull())
-            return count;
-
-        JSCell* base = v.asCell();
-        structure = base->structure(vm);
-        // Since we're accessing a prototype in a loop, it's a good bet that it
-        // should not be treated as a dictionary.
-        if (structure->isDictionary())
-            structure->flattenDictionaryStructure(vm, asObject(base));
-
-        ++count;
-    }
-}
-
 } // namespace JSC