Changeset 284808 in webkit

Timestamp:

Oct 25, 2021, 12:10:00 PM (5 years ago)

Author:

Alan Coon

Message:

Cherry-pick r284576. rdar://problem/84338462

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.

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@284576 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:

branches/safari-612-branch/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)

branches/safari-612-branch/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-10-25  Null  <null@apple.com>
+
+        Cherry-pick r284576. rdar://problem/84338462
+
+    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.
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284576 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-25  Null  <null@apple.com>
 

branches/safari-612-branch/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h

@@ -3142 +3142 @@
             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) {

branches/safari-612-branch/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) {

branches/safari-612-branch/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

@@ -1929 +1929 @@
         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.
 

branches/safari-612-branch/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)

branches/safari-612-branch/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);