Changeset 201584 in webkit

Timestamp:

Jun 1, 2016, 8:18:16 PM (10 years ago)

Author:

keith_miller@apple.com

Message:

canOptimizeStringObjectAccess should use ObjectPropertyConditions rather than structure watchpoints
​https://bugs.webkit.org/show_bug.cgi?id=158291

Reviewed by Benjamin Poulain.

The old StringObject primitive access code used structure watchpoints. This meant that
if you set a watchpoint on String.prototype prior to tiering up to the DFG then added
a new property to String.prototype then we would never use StringObject optimizations.
This made property caching in the LLInt bad because it meant we would watchpoint
String.prototype very early in the program, which hurt date-format-xpab.js since that
benchmark relies on the StringObject optimizations.

This patch also extends ObjectPropertyConditionSet to be able to handle a slotBase
equivalence condition. Since that makes the code for generating the DFG watchpoints
significantly cleaner.

• bytecode/ObjectPropertyCondition.cpp:

(JSC::ObjectPropertyCondition::structureEnsuresValidityAssumingImpurePropertyWatchpoint):

• bytecode/ObjectPropertyConditionSet.cpp:

(JSC::ObjectPropertyConditionSet::hasOneSlotBaseCondition):
(JSC::ObjectPropertyConditionSet::slotBaseCondition):
(JSC::generateConditionsForPrototypeEquivalenceConcurrently):

• bytecode/ObjectPropertyConditionSet.h:
• dfg/DFGGraph.cpp:

(JSC::DFG::Graph::isStringPrototypeMethodSane):
(JSC::DFG::Graph::canOptimizeStringObjectAccess):

• dfg/DFGGraph.h:

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• bytecode/ObjectPropertyCondition.cpp (modified) (1 diff)
• bytecode/ObjectPropertyConditionSet.cpp (modified) (5 diffs)
• bytecode/ObjectPropertyConditionSet.h (modified) (1 diff)
• dfg/DFGGraph.cpp (modified) (5 diffs)
• dfg/DFGGraph.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-06-01  Keith Miller  <keith_miller@apple.com>
+
+        canOptimizeStringObjectAccess should use ObjectPropertyConditions rather than structure watchpoints
+        https://bugs.webkit.org/show_bug.cgi?id=158291
+
+        Reviewed by Benjamin Poulain.
+
+        The old StringObject primitive access code used structure watchpoints. This meant that
+        if you set a watchpoint on String.prototype prior to tiering up to the DFG then added
+        a new property to String.prototype then we would never use StringObject optimizations.
+        This made property caching in the LLInt bad because it meant we would watchpoint
+        String.prototype very early in the program, which hurt date-format-xpab.js since that
+        benchmark relies on the StringObject optimizations.
+
+        This patch also extends ObjectPropertyConditionSet to be able to handle a slotBase
+        equivalence condition. Since that makes the code for generating the DFG watchpoints
+        significantly cleaner.
+
+        * bytecode/ObjectPropertyCondition.cpp:
+        (JSC::ObjectPropertyCondition::structureEnsuresValidityAssumingImpurePropertyWatchpoint):
+        * bytecode/ObjectPropertyConditionSet.cpp:
+        (JSC::ObjectPropertyConditionSet::hasOneSlotBaseCondition):
+        (JSC::ObjectPropertyConditionSet::slotBaseCondition):
+        (JSC::generateConditionsForPrototypeEquivalenceConcurrently):
+        * bytecode/ObjectPropertyConditionSet.h:
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::isStringPrototypeMethodSane):
+        (JSC::DFG::Graph::canOptimizeStringObjectAccess):
+        * dfg/DFGGraph.h:
+
 2016-06-01  Geoffrey Garen  <ggaren@apple.com>
 

trunk/Source/JavaScriptCore/bytecode/ObjectPropertyCondition.cpp

@@ -50 +50 @@
     Structure* structure) const
 {
-    return m_condition.isStillValidAssumingImpurePropertyWatchpoint(structure);
+    return m_condition.isStillValidAssumingImpurePropertyWatchpoint(structure, m_object);
 }
 

trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp

@@ -63 +63 @@
 bool ObjectPropertyConditionSet::hasOneSlotBaseCondition() const
 {
-    return numberOfConditionsWithKind(PropertyCondition::Presence) == 1;
+    return (numberOfConditionsWithKind(PropertyCondition::Presence) == 1) != (numberOfConditionsWithKind(PropertyCondition::Equivalence) == 1);
 }
 
@@ -71 +71 @@
     unsigned numFound = 0;
     for (const ObjectPropertyCondition& condition : *this) {
-        if (condition.kind() == PropertyCondition::Presence) {
+        if (condition.kind() == PropertyCondition::Presence
+            || condition.kind() == PropertyCondition::Equivalence) {
             result = condition;
             numFound++;
@@ -199 +200 @@
         break;
     }
+    case PropertyCondition::Equivalence: {
+        unsigned attributes;
+        PropertyOffset offset = structure->getConcurrently(uid, attributes);
+        if (offset == invalidOffset)
+            return ObjectPropertyCondition();
+        JSValue value = object->getDirect(offset);
+        result = ObjectPropertyCondition::equivalence(vm, owner, object, uid, value);
+        break;
+    }
     default:
         RELEASE_ASSERT_NOT_REACHED();
@@ -241 +251 @@
             if (!prototype) {
                 if (verbose)
-                    dataLog("Reached end up prototype chain as expected, done.\n");
+                    dataLog("Reached end of prototype chain as expected, done.\n");
                 break;
             }
@@ -356 +366 @@
 }
 
+ObjectPropertyConditionSet generateConditionsForPrototypeEquivalenceConcurrently(
+    VM& vm, JSGlobalObject* globalObject, Structure* headStructure, JSObject* prototype, UniquedStringImpl* uid)
+{
+    return generateConditions(vm, globalObject, headStructure, prototype,
+        [&] (Vector<ObjectPropertyCondition>& conditions, JSObject* object) -> bool {
+            PropertyCondition::Kind kind =
+                object == prototype ? PropertyCondition::Equivalence : PropertyCondition::Absence;
+            ObjectPropertyCondition result = generateCondition(vm, nullptr, object, uid, kind);
+            if (!result)
+                return false;
+            conditions.append(result);
+            return true;
+        }, Concurrent);
+}
+
 ObjectPropertyConditionSet generateConditionsForPropertyMissConcurrently(
     VM& vm, JSGlobalObject* globalObject, Structure* headStructure, UniquedStringImpl* uid)

trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.h

@@ -167 +167 @@
     UniquedStringImpl* uid);
 
-
+ObjectPropertyConditionSet generateConditionsForPrototypeEquivalenceConcurrently(
+    VM&, JSGlobalObject*, Structure* headStructure, JSObject* prototype,
+    UniquedStringImpl* uid);
 ObjectPropertyConditionSet generateConditionsForPropertyMissConcurrently(
     VM&, JSGlobalObject*, Structure* headStructure, UniquedStringImpl* uid);

trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp

@@ -1539 +1539 @@
 }
 
-bool Graph::isStringPrototypeMethodSane(JSObject* stringPrototype, Structure* stringPrototypeStructure, UniquedStringImpl* uid)
-{
-    unsigned attributesUnused;
-    PropertyOffset offset = stringPrototypeStructure->getConcurrently(uid, attributesUnused);
-    if (!isValidOffset(offset))
-        return false;
-
-    JSValue value = tryGetConstantProperty(stringPrototype, stringPrototypeStructure, offset);
-    if (!value)
-        return false;
-
-    JSFunction* function = jsDynamicCast<JSFunction*>(value);
-    if (!function)
-        return false;
-
-    if (function->executable()->intrinsicFor(CodeForCall) != StringPrototypeValueOfIntrinsic)
-        return false;
-    
-    return true;
-}
-
 bool Graph::getRegExpPrototypeProperty(JSObject* regExpPrototype, Structure* regExpPrototypeStructure, UniquedStringImpl* uid, JSValue& returnJSValue)
 {
@@ -1588 +1567 @@
 }
 
+bool Graph::isStringPrototypeMethodSane(JSGlobalObject* globalObject, UniquedStringImpl* uid)
+{
+    ObjectPropertyConditionSet conditions = generateConditionsForPrototypeEquivalenceConcurrently(m_vm, globalObject, globalObject->stringObjectStructure(), globalObject->stringPrototype(), uid);
+
+    if (!conditions.isValid())
+        return false;
+
+    ObjectPropertyCondition equivalenceCondition = conditions.slotBaseCondition();
+    RELEASE_ASSERT(equivalenceCondition.hasRequiredValue());
+    JSFunction* function = jsDynamicCast<JSFunction*>(equivalenceCondition.condition().requiredValue());
+    if (!function)
+        return false;
+
+    if (function->executable()->intrinsicFor(CodeForCall) != StringPrototypeValueOfIntrinsic)
+        return false;
+    
+    return watchConditions(conditions);
+}
+
+
 bool Graph::canOptimizeStringObjectAccess(const CodeOrigin& codeOrigin)
 {
@@ -1593 +1592 @@
         return false;
 
+    JSGlobalObject* globalObject = globalObjectFor(codeOrigin);
     Structure* stringObjectStructure = globalObjectFor(codeOrigin)->stringObjectStructure();
     registerStructure(stringObjectStructure);
@@ -1598 +1598 @@
     ASSERT(stringObjectStructure->storedPrototype().asCell()->classInfo() == StringPrototype::info());
 
-    FrozenValue* stringPrototypeObjectValue = freeze(stringObjectStructure->storedPrototype());
-    StringPrototype* stringPrototypeObject = stringPrototypeObjectValue->dynamicCast<StringPrototype*>();
-    Structure* stringPrototypeStructure = stringPrototypeObjectValue->structure();
-    if (registerStructure(stringPrototypeStructure) != StructureRegisteredAndWatched)
-        return false;
-
-    if (stringPrototypeStructure->isDictionary())
-        return false;
-
-    if (!watchConditions(generateConditionsForPropertyMissConcurrently(m_vm, globalObjectFor(codeOrigin), stringObjectStructure, m_vm.propertyNames->toPrimitiveSymbol.impl())))
+    if (!watchConditions(generateConditionsForPropertyMissConcurrently(m_vm, globalObject, stringObjectStructure, m_vm.propertyNames->toPrimitiveSymbol.impl())))
         return false;
 
@@ -1615 +1606 @@
     // between the two, just because that seems like it would get confusing. So we
     // just require both methods to be sane.
-    if (!isStringPrototypeMethodSane(stringPrototypeObject, stringPrototypeStructure, m_vm.propertyNames->valueOf.impl()))
+    if (!isStringPrototypeMethodSane(globalObject, m_vm.propertyNames->valueOf.impl()))
         return false;
-    if (!isStringPrototypeMethodSane(stringPrototypeObject, stringPrototypeStructure, m_vm.propertyNames->toString.impl()))
-        return false;
-
-    return true;
+    return isStringPrototypeMethodSane(globalObject, m_vm.propertyNames->toString.impl());
 }
 

trunk/Source/JavaScriptCore/dfg/DFGGraph.h

@@ -911 +911 @@
 private:
 
-    bool isStringPrototypeMethodSane(JSObject* stringPrototype, Structure* stringPrototypeStructure, UniquedStringImpl*);
+    bool isStringPrototypeMethodSane(JSGlobalObject*, UniquedStringImpl*);
 
     void handleSuccessor(Vector<BasicBlock*, 16>& worklist, BasicBlock*, BasicBlock* successor);