[JSC] Increment bytecode age only when SlotVisitor is first-visit
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Jul 2019 22:26:58 +0000 (22:26 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Jul 2019 22:26:58 +0000 (22:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200196

Reviewed by Robin Morisset.

JSTests:

* stress/reparsing-unlinked-codeblock.js:

Source/JavaScriptCore:

WriteBarrier can cause multiple visits for the same UnlinkedCodeBlock. But this does not mean that we are having multiple cycles of GC.
We should increment the age of the UnlinkedCodeBlock only when the SlotVisitor is saying that this is the first visit.

In practice,this almost never happens. Multiple visits can happen only when the marked UnlinkedCodeBlock gets a write-barrier. But, mutation
of UnlinkedCodeBlock is rare or none after it is initialized. I ran all the JSTests and I cannot find any tests that get re-visiting of UnlinkedCodeBlock.
This patch extends JSTests/stress/reparsing-unlinked-codeblock.js to ensure that UnlinkedCodeBlockJettisoning feature is working after this change.

* bytecode/UnlinkedCodeBlock.cpp:
(JSC::UnlinkedCodeBlock::visitChildren):
* heap/SlotVisitor.h:
(JSC::SlotVisitor::isFirstVisit const):
* parser/Parser.cpp:
* parser/Parser.h:
(JSC::parse):
(JSC::parseFunctionForFunctionConstructor):
* runtime/Options.h:
* tools/JSDollarVM.cpp:
(JSC::functionParseCount):
(JSC::JSDollarVM::finishCreation):

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

JSTests/ChangeLog
JSTests/stress/reparsing-unlinked-codeblock.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp
Source/JavaScriptCore/heap/SlotVisitor.h
Source/JavaScriptCore/parser/Parser.cpp
Source/JavaScriptCore/parser/Parser.h
Source/JavaScriptCore/runtime/Options.h
Source/JavaScriptCore/tools/JSDollarVM.cpp

index 432dee2..93c8a06 100644 (file)
@@ -1,3 +1,12 @@
+2019-07-29  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Increment bytecode age only when SlotVisitor is first-visit
+        https://bugs.webkit.org/show_bug.cgi?id=200196
+
+        Reviewed by Robin Morisset.
+
+        * stress/reparsing-unlinked-codeblock.js:
+
 2019-07-29  Justin Michaud  <justin_michaud@apple.com>
 
         [X86] Emit BT instruction for shift + mask in B3
index b319637..35c47d2 100644 (file)
@@ -1,4 +1,4 @@
-//@ runDefault("--forceCodeBlockToJettisonDueToOldAge=1", "--useUnlinkedCodeBlockJettisoning=1")
+//@ runDefault("--forceCodeBlockToJettisonDueToOldAge=1", "--useUnlinkedCodeBlockJettisoning=1", "--countParseTimes=1", "--useConcurrentJIT=0")
 
 function shouldBe(actual, expected) {
     if (actual !== expected)
@@ -17,8 +17,11 @@ function hello()
 
 // Compile hello and world function.
 shouldBe(hello(), 42);
+var first = $vm.parseCount();
 // Kick full GC 20 times to make UnlinkedCodeBlock aged and destroyed. Jettison hello CodeBlock, and underlying world UnlinkedCodeBlock.
 for (var i = 0; i < 20; ++i)
     fullGC();
 // Recompile world.
 shouldBe(hello(), 42);
+var second = $vm.parseCount() - first;
+shouldBe(second >= 3, true); // `hello`, `inner`, `world`. Other functions can be destroyed, so using >= here.
index 6438de8..c5fd318 100644 (file)
@@ -1,3 +1,30 @@
+2019-07-29  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Increment bytecode age only when SlotVisitor is first-visit
+        https://bugs.webkit.org/show_bug.cgi?id=200196
+
+        Reviewed by Robin Morisset.
+
+        WriteBarrier can cause multiple visits for the same UnlinkedCodeBlock. But this does not mean that we are having multiple cycles of GC.
+        We should increment the age of the UnlinkedCodeBlock only when the SlotVisitor is saying that this is the first visit.
+
+        In practice,this almost never happens. Multiple visits can happen only when the marked UnlinkedCodeBlock gets a write-barrier. But, mutation
+        of UnlinkedCodeBlock is rare or none after it is initialized. I ran all the JSTests and I cannot find any tests that get re-visiting of UnlinkedCodeBlock.
+        This patch extends JSTests/stress/reparsing-unlinked-codeblock.js to ensure that UnlinkedCodeBlockJettisoning feature is working after this change.
+
+        * bytecode/UnlinkedCodeBlock.cpp:
+        (JSC::UnlinkedCodeBlock::visitChildren):
+        * heap/SlotVisitor.h:
+        (JSC::SlotVisitor::isFirstVisit const):
+        * parser/Parser.cpp:
+        * parser/Parser.h:
+        (JSC::parse):
+        (JSC::parseFunctionForFunctionConstructor):
+        * runtime/Options.h:
+        * tools/JSDollarVM.cpp:
+        (JSC::functionParseCount):
+        (JSC::JSDollarVM::finishCreation):
+
 2019-07-28  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r247886.
index 7745a41..e006d16 100644 (file)
@@ -89,7 +89,8 @@ void UnlinkedCodeBlock::visitChildren(JSCell* cell, SlotVisitor& visitor)
     ASSERT_GC_OBJECT_INHERITS(thisObject, info());
     Base::visitChildren(thisObject, visitor);
     auto locker = holdLock(thisObject->cellLock());
-    thisObject->m_age = std::min<unsigned>(static_cast<unsigned>(thisObject->m_age) + 1, maxAge);
+    if (visitor.isFirstVisit())
+        thisObject->m_age = std::min<unsigned>(static_cast<unsigned>(thisObject->m_age) + 1, maxAge);
     for (FunctionExpressionVector::iterator ptr = thisObject->m_functionDecls.begin(), end = thisObject->m_functionDecls.end(); ptr != end; ++ptr)
         visitor.append(*ptr);
     for (FunctionExpressionVector::iterator ptr = thisObject->m_functionExprs.begin(), end = thisObject->m_functionExprs.end(); ptr != end; ++ptr)
index ad4eb7a..e035bae 100644 (file)
@@ -119,6 +119,8 @@ public:
 
     bool isEmpty() { return m_collectorStack.isEmpty() && m_mutatorStack.isEmpty(); }
 
+    bool isFirstVisit() const { return m_isFirstVisit; }
+
     void didStartMarking();
     void reset();
     void clearMarkStacks();
index e254dc9..f8dbe71 100644 (file)
@@ -88,6 +88,8 @@
 
 namespace JSC {
 
+std::atomic<unsigned> globalParseCount { 0 };
+
 ALWAYS_INLINE static SourceParseMode getAsynFunctionBodyParseMode(SourceParseMode parseMode)
 {
     if (isAsyncGeneratorWrapperParseMode(parseMode))
index 10a3121..69aa0bc 100644 (file)
@@ -150,6 +150,8 @@ ALWAYS_INLINE static bool isSafeContextualKeyword(const JSToken& token)
     return token.m_type >= FirstSafeContextualKeywordToken && token.m_type <= LastSafeContextualKeywordToken;
 }
 
+JS_EXPORT_PRIVATE extern std::atomic<unsigned> globalParseCount;
+
 struct Scope {
     WTF_MAKE_NONCOPYABLE(Scope);
 
@@ -2031,6 +2033,9 @@ std::unique_ptr<ParsedNode> parse(
             *positionBeforeLastNewline = parser.positionBeforeLastNewline();
     }
 
+    if (UNLIKELY(Options::countParseTimes()))
+        globalParseCount++;
+
     if (UNLIKELY(Options::reportParseTimes())) {
         MonotonicTime after = MonotonicTime::now();
         ParseHash hash(source);
@@ -2063,6 +2068,9 @@ inline std::unique_ptr<ProgramNode> parseFunctionForFunctionConstructor(VM& vm,
             *positionBeforeLastNewline = parser.positionBeforeLastNewline();
     }
 
+    if (UNLIKELY(Options::countParseTimes()))
+        globalParseCount++;
+
     if (UNLIKELY(Options::reportParseTimes())) {
         MonotonicTime after = MonotonicTime::now();
         ParseHash hash(source);
index f5fe088..366c419 100644 (file)
@@ -198,6 +198,7 @@ constexpr bool enableWebAssemblyStreamingApi = false;
     v(bool, reportTotalCompileTimes, false, Normal, nullptr) \
     v(bool, reportParseTimes, false, Normal, "dumps JS function signature and the time it took to parse") \
     v(bool, reportBytecodeCompileTimes, false, Normal, "dumps JS function signature and the time it took to bytecode compile") \
+    v(bool, countParseTimes, false, Normal, "counts parse times") \
     v(bool, verboseExitProfile, false, Normal, nullptr) \
     v(bool, verboseCFA, false, Normal, nullptr) \
     v(bool, verboseDFGFailure, false, Normal, nullptr) \
index 9a0c935..745e0a2 100644 (file)
@@ -40,6 +40,7 @@
 #include "JSONObject.h"
 #include "JSProxy.h"
 #include "JSString.h"
+#include "Parser.h"
 #include "ShadowChicken.h"
 #include "Snippet.h"
 #include "SnippetParams.h"
@@ -2186,6 +2187,11 @@ static EncodedJSValue JSC_HOST_CALL functionTotalGCTime(ExecState* exec)
     return JSValue::encode(jsNumber(vm.heap.totalGCTime().seconds()));
 }
 
+static EncodedJSValue JSC_HOST_CALL functionParseCount(ExecState*)
+{
+    return JSValue::encode(jsNumber(globalParseCount.load()));
+}
+
 void JSDollarVM::finishCreation(VM& vm)
 {
     Base::finishCreation(vm);
@@ -2299,6 +2305,8 @@ void JSDollarVM::finishCreation(VM& vm)
     addFunction(vm, "deltaBetweenButterflies", functionDeltaBetweenButterflies, 2);
     
     addFunction(vm, "totalGCTime", functionTotalGCTime, 0);
+
+    addFunction(vm, "parseCount", functionParseCount, 0);
 }
 
 void JSDollarVM::addFunction(VM& vm, JSGlobalObject* globalObject, const char* name, NativeFunction function, unsigned arguments)