Changeset 284576 in webkit

Timestamp:

Oct 20, 2021, 3:08:45 PM (5 years ago)

Author:

Justin Michaud

Message:

We should watch isHavingABadTime if we read from the structureCache
​https://bugs.webkit.org/show_bug.cgi?id=232019

Reviewed by Yusuke Suzuki.

We should lock the structure cache when we clear it, and the compiler thread should
watch isHavingABadTime in the case that the cache might get cleared.

• dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

• dfg/DFGConstantFoldingPhase.cpp:

(JSC::DFG::ConstantFoldingPhase::foldConstants):

• runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::haveABadTime):

• runtime/StructureCache.cpp:

(JSC::StructureCache::clear):

• runtime/StructureCache.h:

(JSC::StructureCache::clear): Deleted.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGAbstractInterpreterInlines.h (modified) (1 diff)
• dfg/DFGConstantFoldingPhase.cpp (modified) (1 diff)
• runtime/JSGlobalObject.cpp (modified) (1 diff)
• runtime/StructureCache.cpp (modified) (1 diff)
• runtime/StructureCache.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-10-20  Justin Michaud  <justin_michaud@apple.com>
+
+        We should watch isHavingABadTime if we read from the structureCache
+        https://bugs.webkit.org/show_bug.cgi?id=232019
+
+        Reviewed by Yusuke Suzuki.
+
+        We should lock the structure cache when we clear it, and the compiler thread should
+        watch isHavingABadTime in the case that the cache might get cleared.
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGConstantFoldingPhase.cpp:
+        (JSC::DFG::ConstantFoldingPhase::foldConstants):
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::haveABadTime):
+        * runtime/StructureCache.cpp:
+        (JSC::StructureCache::clear):
+        * runtime/StructureCache.h:
+        (JSC::StructureCache::clear): Deleted.
+
 2021-10-20  Michael Saboff  <msaboff@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h

@@ -3127 +3127 @@
             if (base.isNull())
                 structure = globalObject->nullPrototypeObjectStructure();
-            else if (base.isObject())
-                structure = m_vm.structureCache.emptyObjectStructureConcurrently(globalObject, base.getObject(), JSFinalObject::defaultInlineCapacity());
+            else if (base.isObject()) {
+                // Having a bad time clears the structureCache, and so it should invalidate this structure.
+                bool isHavingABadTime = globalObject->isHavingABadTime();
+                WTF::loadLoadFence();
+                if (!isHavingABadTime)
+                    m_graph.watchpoints().addLazily(globalObject->havingABadTimeWatchpoint());
+                // Normally, we would always install a watchpoint. In this case, however, if we haveABadTime, we
+                // still want to optimize. There is no watchpoint for that case though, so we need to make sure this load
+                // does not get hoisted above the check.
+                WTF::loadLoadFence();
+                structure = m_vm.structureCache
+                    .emptyObjectStructureConcurrently(globalObject, base.getObject(), JSFinalObject::defaultInlineCapacity());
+            }
             
             if (structure) {

trunk/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp

@@ -842 +842 @@
                     if (base.isNull())
                         structure = globalObject->nullPrototypeObjectStructure();
-                    else if (base.isObject())
-                        structure = globalObject->vm().structureCache.emptyObjectStructureConcurrently(globalObject, base.getObject(), JSFinalObject::defaultInlineCapacity());
+                    else if (base.isObject()) {
+                        // Having a bad time clears the structureCache, and so it should invalidate this structure.
+                        bool isHavingABadTime = globalObject->isHavingABadTime();
+                        WTF::loadLoadFence();
+                        if (!isHavingABadTime)
+                            m_graph.watchpoints().addLazily(globalObject->havingABadTimeWatchpoint());
+                        // Normally, we would always install a watchpoint. In this case, however, if we haveABadTime, we
+                        // still want to optimize. There is no watchpoint for that case though, so we need to make sure this load
+                        // does not get hoisted above the check.
+                        WTF::loadLoadFence();
+                        structure = globalObject->vm().structureCache
+                            .emptyObjectStructureConcurrently(globalObject, base.getObject(), JSFinalObject::defaultInlineCapacity());
+                    }
                     
                     if (structure) {

trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

@@ -1985 +1985 @@
         return;
 
+    // This must happen first, because the compiler thread may race with haveABadTime.
+    // Let R_BT, W_BT <- Read/Fire the watchpoint, R_SC, W_SC <- Read/clear the structure cache.
+    // The possible interleavings are:
+    // R_BT, R_SC, W_SC, W_BT: Compiler thread installs a watchpoint, and the code is discarded.
+    // R_BT, W_SC, R_SC, W_BT: ^ Same
+    // R_BT, W_SC, W_BT, W_SC: ^ Same
+    // W_SC, R_BT, R_SC, W_BT: ^ Same
+    // W_SC, R_BT, W_BT, R_SC: ^ Same
+    // W_SC, W_BT, R_BT, R_SC: No watchpoint is installed, but we could not see old structures from the cache.
     vm.structureCache.clear(); // We may be caching array structures in here.
 

trunk/Source/JavaScriptCore/runtime/StructureCache.cpp

@@ -31 +31 @@
 
 namespace JSC {
+
+void StructureCache::clear()
+{
+    Locker locker { m_lock };
+    m_structures.clear();
+}
 
 inline Structure* StructureCache::createEmptyStructure(JSGlobalObject* globalObject, JSObject* prototype, const TypeInfo& typeInfo, const ClassInfo* classInfo, IndexingType indexingType, unsigned inlineCapacity, bool makePolyProtoStructure, FunctionExecutable* executable)

trunk/Source/JavaScriptCore/runtime/StructureCache.h

@@ -49 +49 @@
     }
 
-    void clear() { m_structures.clear(); }
+    JS_EXPORT_PRIVATE void clear();
 
     JS_EXPORT_PRIVATE Structure* emptyObjectStructureForPrototype(JSGlobalObject*, JSObject*, unsigned inlineCapacity, bool makePolyProtoStructure = false, FunctionExecutable* = nullptr);