Title: [242902] trunk/Source/_javascript_Core
Revision
242902
Author
ysuz...@apple.com
Date
2019-03-13 13:27:54 -0700 (Wed, 13 Mar 2019)

Log Message

[JSC] Move species watchpoint installation from ArrayPrototype to JSGlobalObject
https://bugs.webkit.org/show_bug.cgi?id=195593

Reviewed by Keith Miller.

This patch moves watchpoints installation and watchpoints themselves from ArrayPrototype to JSGlobalObject because of the following two reasons.

1. ArrayPrototype configures finalizer because of std::unique_ptr<> for watchpoints. If we move them from ArrayPrototype to JSGlobalObject, we do
   not need to set finalizer. And we can avoid unnecessary WeakBlock allocation.

2. This code lazily configures watchpoints instead of setting watchpoints eagerly in JSGlobalObject::init. We would like to expand this mechanism
   to other watchpoints which are eagerly configured in JSGlobalObject::init. Putting these code in JSGlobalObject instead of scattering them in
   each XXXPrototype / XXXConstructor can encourage the reuse of the code.

* runtime/ArrayPrototype.cpp:
(JSC::ArrayPrototype::create):
(JSC::speciesWatchpointIsValid):
(JSC::ArrayPrototype::destroy): Deleted.
(JSC::ArrayPrototype::tryInitializeSpeciesWatchpoint): Deleted.
(JSC::ArrayPrototypeAdaptiveInferredPropertyWatchpoint::ArrayPrototypeAdaptiveInferredPropertyWatchpoint): Deleted.
(JSC::ArrayPrototypeAdaptiveInferredPropertyWatchpoint::handleFire): Deleted.
* runtime/ArrayPrototype.h:
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::tryInstallArraySpeciesWatchpoint): Instead of using ArrayPrototypeAdaptiveInferredPropertyWatchpoint,
we use ObjectPropertyChangeAdaptiveWatchpoint. We create watchpoints after touching WatchpointSet since ObjectPropertyChangeAdaptiveWatchpoint
requires WatchpointSet is IsWatched state.
* runtime/JSGlobalObject.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (242901 => 242902)


--- trunk/Source/_javascript_Core/ChangeLog	2019-03-13 20:18:08 UTC (rev 242901)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-03-13 20:27:54 UTC (rev 242902)
@@ -1,3 +1,33 @@
+2019-03-11  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] Move species watchpoint installation from ArrayPrototype to JSGlobalObject
+        https://bugs.webkit.org/show_bug.cgi?id=195593
+
+        Reviewed by Keith Miller.
+
+        This patch moves watchpoints installation and watchpoints themselves from ArrayPrototype to JSGlobalObject because of the following two reasons.
+
+        1. ArrayPrototype configures finalizer because of std::unique_ptr<> for watchpoints. If we move them from ArrayPrototype to JSGlobalObject, we do
+           not need to set finalizer. And we can avoid unnecessary WeakBlock allocation.
+
+        2. This code lazily configures watchpoints instead of setting watchpoints eagerly in JSGlobalObject::init. We would like to expand this mechanism
+           to other watchpoints which are eagerly configured in JSGlobalObject::init. Putting these code in JSGlobalObject instead of scattering them in
+           each XXXPrototype / XXXConstructor can encourage the reuse of the code.
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::ArrayPrototype::create):
+        (JSC::speciesWatchpointIsValid):
+        (JSC::ArrayPrototype::destroy): Deleted.
+        (JSC::ArrayPrototype::tryInitializeSpeciesWatchpoint): Deleted.
+        (JSC::ArrayPrototypeAdaptiveInferredPropertyWatchpoint::ArrayPrototypeAdaptiveInferredPropertyWatchpoint): Deleted.
+        (JSC::ArrayPrototypeAdaptiveInferredPropertyWatchpoint::handleFire): Deleted.
+        * runtime/ArrayPrototype.h:
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::tryInstallArraySpeciesWatchpoint): Instead of using ArrayPrototypeAdaptiveInferredPropertyWatchpoint,
+        we use ObjectPropertyChangeAdaptiveWatchpoint. We create watchpoints after touching WatchpointSet since ObjectPropertyChangeAdaptiveWatchpoint
+        requires WatchpointSet is IsWatched state.
+        * runtime/JSGlobalObject.h:
+
 2019-03-12  Yusuke Suzuki  <ysuz...@apple.com>
 
         [JSC] OSR entry should respect abstract values in addition to flush formats

Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (242901 => 242902)


--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2019-03-13 20:18:08 UTC (rev 242901)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2019-03-13 20:27:54 UTC (rev 242902)
@@ -68,7 +68,6 @@
 {
     ArrayPrototype* prototype = new (NotNull, allocateCell<ArrayPrototype>(vm.heap)) ArrayPrototype(vm, structure);
     prototype->finishCreation(vm, globalObject);
-    vm.heap.addFinalizer(prototype, destroy);
     return prototype;
 }
 
@@ -142,12 +141,6 @@
     putDirectWithoutTransition(vm, vm.propertyNames->unscopablesSymbol, unscopables, PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly);
 }
 
-void ArrayPrototype::destroy(JSC::JSCell* cell)
-{
-    ArrayPrototype* thisObject = static_cast<ArrayPrototype*>(cell);
-    thisObject->ArrayPrototype::~ArrayPrototype();
-}
-
 // ------------------------------ Array Functions ----------------------------
 
 static ALWAYS_INLINE JSValue getProperty(ExecState* exec, JSObject* object, unsigned index)
@@ -192,6 +185,10 @@
         throwTypeError(exec, scope, ReadonlyPropertyWriteError);
 }
 
+namespace ArrayPrototypeInternal {
+static bool verbose = false;
+}
+
 ALWAYS_INLINE bool speciesWatchpointIsValid(ExecState* exec, JSObject* thisObject)
 {
     VM& vm = exec->vm();
@@ -199,7 +196,8 @@
     ArrayPrototype* arrayPrototype = globalObject->arrayPrototype();
 
     if (globalObject->arraySpeciesWatchpoint().stateOnJSThread() == ClearWatchpoint) {
-        arrayPrototype->tryInitializeSpeciesWatchpoint(exec);
+        dataLogLnIf(ArrayPrototypeInternal::verbose, "Initializing Array species watchpoints for Array.prototype: ", pointerDump(arrayPrototype), " with structure: ", pointerDump(arrayPrototype->structure(vm)), "\nand Array: ", pointerDump(globalObject->arrayConstructor()), " with structure: ", pointerDump(globalObject->arrayConstructor()->structure(vm)));
+        globalObject->tryInstallArraySpeciesWatchpoint(exec);
         ASSERT(globalObject->arraySpeciesWatchpoint().stateOnJSThread() != ClearWatchpoint);
     }
 
@@ -1544,113 +1542,4 @@
     return JSValue::encode(jsUndefined());
 }
 
-
-// -------------------- ArrayPrototype.constructor Watchpoint ------------------
-
-namespace ArrayPrototypeInternal {
-static bool verbose = false;
-}
-
-class ArrayPrototypeAdaptiveInferredPropertyWatchpoint : public AdaptiveInferredPropertyValueWatchpointBase {
-public:
-    typedef AdaptiveInferredPropertyValueWatchpointBase Base;
-    ArrayPrototypeAdaptiveInferredPropertyWatchpoint(const ObjectPropertyCondition&, ArrayPrototype*);
-
-private:
-    void handleFire(VM&, const FireDetail&) override;
-
-    ArrayPrototype* m_arrayPrototype;
-};
-
-void ArrayPrototype::tryInitializeSpeciesWatchpoint(ExecState* exec)
-{
-    VM& vm = exec->vm();
-
-    RELEASE_ASSERT(!m_constructorWatchpoint);
-    RELEASE_ASSERT(!m_constructorSpeciesWatchpoint);
-
-    auto scope = DECLARE_THROW_SCOPE(vm);
-
-    if (ArrayPrototypeInternal::verbose)
-        dataLog("Initializing Array species watchpoints for Array.prototype: ", pointerDump(this), " with structure: ", pointerDump(this->structure(vm)), "\nand Array: ", pointerDump(this->globalObject(vm)->arrayConstructor()), " with structure: ", pointerDump(this->globalObject(vm)->arrayConstructor()->structure(vm)), "\n");
-    // First we need to make sure that the Array.prototype.constructor property points to Array
-    // and that Array[Symbol.species] is the primordial GetterSetter.
-
-    // We only initialize once so flattening the structures does not have any real cost.
-    Structure* prototypeStructure = this->structure(vm);
-    if (prototypeStructure->isDictionary())
-        prototypeStructure = prototypeStructure->flattenDictionaryStructure(vm, this);
-    RELEASE_ASSERT(!prototypeStructure->isDictionary());
-
-    JSGlobalObject* globalObject = this->globalObject(vm);
-    ArrayConstructor* arrayConstructor = globalObject->arrayConstructor();
-
-    auto invalidateWatchpoint = [&] {
-        globalObject->arraySpeciesWatchpoint().invalidate(vm, StringFireDetail("Was not able to set up array species watchpoint."));
-    };
-
-    PropertySlot constructorSlot(this, PropertySlot::InternalMethodType::VMInquiry);
-    this->getOwnPropertySlot(this, exec, vm.propertyNames->constructor, constructorSlot);
-    scope.assertNoException();
-    if (constructorSlot.slotBase() != this
-        || !constructorSlot.isCacheableValue()
-        || constructorSlot.getValue(exec, vm.propertyNames->constructor) != arrayConstructor) {
-        invalidateWatchpoint();
-        return;
-    }
-
-    Structure* constructorStructure = arrayConstructor->structure(vm);
-    if (constructorStructure->isDictionary())
-        constructorStructure = constructorStructure->flattenDictionaryStructure(vm, arrayConstructor);
-
-    PropertySlot speciesSlot(arrayConstructor, PropertySlot::InternalMethodType::VMInquiry);
-    arrayConstructor->getOwnPropertySlot(arrayConstructor, exec, vm.propertyNames->speciesSymbol, speciesSlot);
-    scope.assertNoException();
-    if (speciesSlot.slotBase() != arrayConstructor
-        || !speciesSlot.isCacheableGetter()
-        || speciesSlot.getterSetter() != globalObject->speciesGetterSetter()) {
-        invalidateWatchpoint();
-        return;
-    }
-
-    // Now we need to setup the watchpoints to make sure these conditions remain valid.
-    prototypeStructure->startWatchingPropertyForReplacements(vm, constructorSlot.cachedOffset());
-    constructorStructure->startWatchingPropertyForReplacements(vm, speciesSlot.cachedOffset());
-
-    ObjectPropertyCondition constructorCondition = ObjectPropertyCondition::equivalence(vm, this, this, vm.propertyNames->constructor.impl(), arrayConstructor);
-    ObjectPropertyCondition speciesCondition = ObjectPropertyCondition::equivalence(vm, this, arrayConstructor, vm.propertyNames->speciesSymbol.impl(), globalObject->speciesGetterSetter());
-
-    if (!constructorCondition.isWatchable() || !speciesCondition.isWatchable()) {
-        invalidateWatchpoint();
-        return;
-    }
-
-    m_constructorWatchpoint = std::make_unique<ArrayPrototypeAdaptiveInferredPropertyWatchpoint>(constructorCondition, this);
-    m_constructorWatchpoint->install(vm);
-
-    m_constructorSpeciesWatchpoint = std::make_unique<ArrayPrototypeAdaptiveInferredPropertyWatchpoint>(speciesCondition, this);
-    m_constructorSpeciesWatchpoint->install(vm);
-
-    // We only watch this from the DFG, and the DFG makes sure to only start watching if the watchpoint is in the IsWatched state.
-    RELEASE_ASSERT(!globalObject->arraySpeciesWatchpoint().isBeingWatched()); 
-    globalObject->arraySpeciesWatchpoint().touch(vm, "Set up array species watchpoint.");
-}
-
-ArrayPrototypeAdaptiveInferredPropertyWatchpoint::ArrayPrototypeAdaptiveInferredPropertyWatchpoint(const ObjectPropertyCondition& key, ArrayPrototype* prototype)
-    : Base(key)
-    , m_arrayPrototype(prototype)
-{
-}
-
-void ArrayPrototypeAdaptiveInferredPropertyWatchpoint::handleFire(VM& vm, const FireDetail& detail)
-{
-    auto lazyDetail = createLazyFireDetail("ArrayPrototype adaption of ", key(), " failed: ", detail);
-
-    if (ArrayPrototypeInternal::verbose)
-        WTF::dataLog(lazyDetail, "\n");
-
-    JSGlobalObject* globalObject = m_arrayPrototype->globalObject(vm);
-    globalObject->arraySpeciesWatchpoint().fireAll(vm, lazyDetail);
-}
-
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.h (242901 => 242902)


--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.h	2019-03-13 20:18:08 UTC (rev 242901)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.h	2019-03-13 20:27:54 UTC (rev 242902)
@@ -24,8 +24,6 @@
 
 namespace JSC {
 
-class ArrayPrototypeAdaptiveInferredPropertyWatchpoint;
-
 class ArrayPrototype final : public JSArray {
 private:
     ArrayPrototype(VM&, Structure*);
@@ -48,20 +46,8 @@
         return Structure::create(vm, globalObject, prototype, TypeInfo(DerivedArrayType, StructureFlags), info(), ArrayClass);
     }
 
-    void tryInitializeSpeciesWatchpoint(ExecState*);
-
-    static const bool needsDestruction = false;
-    // We don't need destruction since we use a finalizer.
-    static void destroy(JSC::JSCell*);
-
 protected:
     void finishCreation(VM&, JSGlobalObject*);
-
-private:
-    // This bit is set if any user modifies the constructor property Array.prototype. This is used to optimize species creation for JSArrays.
-    friend ArrayPrototypeAdaptiveInferredPropertyWatchpoint;
-    std::unique_ptr<ArrayPrototypeAdaptiveInferredPropertyWatchpoint> m_constructorWatchpoint;
-    std::unique_ptr<ArrayPrototypeAdaptiveInferredPropertyWatchpoint> m_constructorSpeciesWatchpoint;
 };
 
 EncodedJSValue JSC_HOST_CALL arrayProtoFuncToString(ExecState*);

Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp (242901 => 242902)


--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp	2019-03-13 20:18:08 UTC (rev 242901)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp	2019-03-13 20:27:54 UTC (rev 242902)
@@ -1821,6 +1821,77 @@
     jsCast<JSGlobalObject*>(cell)->m_rareData = nullptr;
 }
 
+void JSGlobalObject::tryInstallArraySpeciesWatchpoint(ExecState* exec)
+{
+    RELEASE_ASSERT(!m_arrayPrototypeConstructorWatchpoint);
+    RELEASE_ASSERT(!m_arrayConstructorSpeciesWatchpoint);
+
+    VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    // First we need to make sure that the Array.prototype.constructor property points to Array
+    // and that Array[Symbol.species] is the primordial GetterSetter.
+    ArrayPrototype* arrayPrototype = this->arrayPrototype();
+
+    // We only initialize once so flattening the structures does not have any real cost.
+    Structure* prototypeStructure = arrayPrototype->structure(vm);
+    if (prototypeStructure->isDictionary())
+        prototypeStructure = prototypeStructure->flattenDictionaryStructure(vm, arrayPrototype);
+    RELEASE_ASSERT(!prototypeStructure->isDictionary());
+
+    ArrayConstructor* arrayConstructor = this->arrayConstructor();
+
+    auto invalidateWatchpoint = [&] {
+        m_arraySpeciesWatchpoint.invalidate(vm, StringFireDetail("Was not able to set up array species watchpoint."));
+    };
+
+    PropertySlot constructorSlot(arrayPrototype, PropertySlot::InternalMethodType::VMInquiry);
+    arrayPrototype->getOwnPropertySlot(arrayPrototype, exec, vm.propertyNames->constructor, constructorSlot);
+    scope.assertNoException();
+    if (constructorSlot.slotBase() != arrayPrototype
+        || !constructorSlot.isCacheableValue()
+        || constructorSlot.getValue(exec, vm.propertyNames->constructor) != arrayConstructor) {
+        invalidateWatchpoint();
+        return;
+    }
+
+    Structure* constructorStructure = arrayConstructor->structure(vm);
+    if (constructorStructure->isDictionary())
+        constructorStructure = constructorStructure->flattenDictionaryStructure(vm, arrayConstructor);
+
+    PropertySlot speciesSlot(arrayConstructor, PropertySlot::InternalMethodType::VMInquiry);
+    arrayConstructor->getOwnPropertySlot(arrayConstructor, exec, vm.propertyNames->speciesSymbol, speciesSlot);
+    scope.assertNoException();
+    if (speciesSlot.slotBase() != arrayConstructor
+        || !speciesSlot.isCacheableGetter()
+        || speciesSlot.getterSetter() != speciesGetterSetter()) {
+        invalidateWatchpoint();
+        return;
+    }
+
+    // Now we need to setup the watchpoints to make sure these conditions remain valid.
+    prototypeStructure->startWatchingPropertyForReplacements(vm, constructorSlot.cachedOffset());
+    constructorStructure->startWatchingPropertyForReplacements(vm, speciesSlot.cachedOffset());
+
+    ObjectPropertyCondition constructorCondition = ObjectPropertyCondition::equivalence(vm, arrayPrototype, arrayPrototype, vm.propertyNames->constructor.impl(), arrayConstructor);
+    ObjectPropertyCondition speciesCondition = ObjectPropertyCondition::equivalence(vm, arrayPrototype, arrayConstructor, vm.propertyNames->speciesSymbol.impl(), speciesGetterSetter());
+
+    if (!constructorCondition.isWatchable() || !speciesCondition.isWatchable()) {
+        invalidateWatchpoint();
+        return;
+    }
+
+    // We only watch this from the DFG, and the DFG makes sure to only start watching if the watchpoint is in the IsWatched state.
+    RELEASE_ASSERT(!m_arraySpeciesWatchpoint.isBeingWatched());
+    m_arraySpeciesWatchpoint.touch(vm, "Set up array species watchpoint.");
+
+    m_arrayPrototypeConstructorWatchpoint = std::make_unique<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>>(constructorCondition, m_arraySpeciesWatchpoint);
+    m_arrayPrototypeConstructorWatchpoint->install(vm);
+
+    m_arrayConstructorSpeciesWatchpoint = std::make_unique<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>>(speciesCondition, m_arraySpeciesWatchpoint);
+    m_arrayConstructorSpeciesWatchpoint->install(vm);
+}
+
 void slowValidateCell(JSGlobalObject* globalObject)
 {
     RELEASE_ASSERT(globalObject->isGlobalObject());

Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.h (242901 => 242902)


--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.h	2019-03-13 20:18:08 UTC (rev 242901)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.h	2019-03-13 20:27:54 UTC (rev 242902)
@@ -473,6 +473,8 @@
     InlineWatchpointSet m_setAddWatchpoint;
     InlineWatchpointSet m_arraySpeciesWatchpoint;
     InlineWatchpointSet m_numberToStringWatchpoint;
+    std::unique_ptr<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>> m_arrayConstructorSpeciesWatchpoint;
+    std::unique_ptr<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>> m_arrayPrototypeConstructorWatchpoint;
     std::unique_ptr<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>> m_arrayPrototypeSymbolIteratorWatchpoint;
     std::unique_ptr<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>> m_arrayIteratorPrototypeNext;
     std::unique_ptr<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>> m_mapPrototypeSymbolIteratorWatchpoint;
@@ -1008,6 +1010,8 @@
     void setWrapperMap(std::unique_ptr<WrapperMap>&&);
 #endif
 
+    void tryInstallArraySpeciesWatchpoint(ExecState*);
+
 protected:
     struct GlobalPropertyInfo {
         GlobalPropertyInfo(const Identifier& i, JSValue v, unsigned a)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to