Relanding r253581: Changed jsc shell timeout mechanism to leverage the VMTraps and...
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Dec 2019 08:50:20 +0000 (08:50 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Dec 2019 08:50:20 +0000 (08:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=205279
<rdar://problem/57971874>

Reviewed by Saam Barati.

This fixes all the timeouts that occur due to CPU time starvation when
running JSC tests on a debug build.

What this means is that the timeout mechanism may trigger asynchronous
OSR exits.  If a test requires no OSR exits, that test should
requireOption("--usePollingTraps=true") so that the VMTraps will use its
polling implementation instead.

I've tested this with a full run of the JSC stress tests with a debug
build and saw 0 timeouts.  I've also tested it with a contrived tests that
loops forever, and saw the expected timeout crash.

Will look into re-tuning needed timeout value (and other JSC tests timeout
cleanup) in https://bugs.webkit.org/show_bug.cgi?id=205298.

Update: in the previously landed patch, I did a last minute sort of the cases
Int the switch statement in VMTraps::handleTraps() before posting my patch.
This is incorrect to do since one of the cases need to fall through to another
case.  This patch undoes the sorting to the order I originally had the cases
in during development and testing.

* interpreter/Interpreter.cpp:
(JSC::Interpreter::executeProgram):
(JSC::Interpreter::executeCall):
(JSC::Interpreter::executeConstruct):
(JSC::Interpreter::execute):
(JSC::Interpreter::executeModuleProgram):
* interpreter/InterpreterInlines.h:
(JSC::Interpreter::execute):
* jsc.cpp:
(startTimeoutTimer):
(timeoutCheckCallback):
(initializeTimeoutIfNeeded):
(startTimeoutThreadIfNeeded):
(runJSC):
(jscmain):
* runtime/JSCConfig.h:
* runtime/VM.h:
(JSC::VM::notifyNeedShellTimeoutCheck):
* runtime/VMTraps.cpp:
(JSC::VMTraps::handleTraps):
* runtime/VMTraps.h:
(JSC::VMTraps::Mask::Mask):
(JSC::VMTraps::Mask::allEventTypes):
(JSC::VMTraps::Mask::init):
(JSC::VMTraps::interruptingTraps):
* tools/VMInspector.cpp:
(JSC::VMInspector::forEachVM):
* tools/VMInspector.h:

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/interpreter/Interpreter.cpp
Source/JavaScriptCore/interpreter/InterpreterInlines.h
Source/JavaScriptCore/jsc.cpp
Source/JavaScriptCore/runtime/JSCConfig.h
Source/JavaScriptCore/runtime/VM.h
Source/JavaScriptCore/runtime/VMTraps.cpp
Source/JavaScriptCore/runtime/VMTraps.h
Source/JavaScriptCore/tools/VMInspector.cpp
Source/JavaScriptCore/tools/VMInspector.h

index 9a204ae..c77c5c1 100644 (file)
@@ -1,5 +1,63 @@
 2019-12-16  Mark Lam  <mark.lam@apple.com>
 
+        Relanding r253581: Changed jsc shell timeout mechanism to leverage the VMTraps and use CPUTime.
+        https://bugs.webkit.org/show_bug.cgi?id=205279
+        <rdar://problem/57971874>
+
+        Reviewed by Saam Barati.
+
+        This fixes all the timeouts that occur due to CPU time starvation when
+        running JSC tests on a debug build.
+
+        What this means is that the timeout mechanism may trigger asynchronous
+        OSR exits.  If a test requires no OSR exits, that test should
+        requireOption("--usePollingTraps=true") so that the VMTraps will use its
+        polling implementation instead.
+
+        I've tested this with a full run of the JSC stress tests with a debug
+        build and saw 0 timeouts.  I've also tested it with a contrived tests that
+        loops forever, and saw the expected timeout crash.
+
+        Will look into re-tuning needed timeout value (and other JSC tests timeout
+        cleanup) in https://bugs.webkit.org/show_bug.cgi?id=205298.
+
+        Update: in the previously landed patch, I did a last minute sort of the cases
+        Int the switch statement in VMTraps::handleTraps() before posting my patch.
+        This is incorrect to do since one of the cases need to fall through to another
+        case.  This patch undoes the sorting to the order I originally had the cases
+        in during development and testing.
+
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::executeProgram):
+        (JSC::Interpreter::executeCall):
+        (JSC::Interpreter::executeConstruct):
+        (JSC::Interpreter::execute):
+        (JSC::Interpreter::executeModuleProgram):
+        * interpreter/InterpreterInlines.h:
+        (JSC::Interpreter::execute):
+        * jsc.cpp:
+        (startTimeoutTimer):
+        (timeoutCheckCallback):
+        (initializeTimeoutIfNeeded):
+        (startTimeoutThreadIfNeeded):
+        (runJSC):
+        (jscmain):
+        * runtime/JSCConfig.h:
+        * runtime/VM.h:
+        (JSC::VM::notifyNeedShellTimeoutCheck):
+        * runtime/VMTraps.cpp:
+        (JSC::VMTraps::handleTraps):
+        * runtime/VMTraps.h:
+        (JSC::VMTraps::Mask::Mask):
+        (JSC::VMTraps::Mask::allEventTypes):
+        (JSC::VMTraps::Mask::init):
+        (JSC::VMTraps::interruptingTraps):
+        * tools/VMInspector.cpp:
+        (JSC::VMInspector::forEachVM):
+        * tools/VMInspector.h:
+
+2019-12-16  Mark Lam  <mark.lam@apple.com>
+
         Rolling out: r253581 is failing tests on a release build.
         https://bugs.webkit.org/show_bug.cgi?id=205279
         <rdar://problem/57971874>
index e7a3d22..daa94c8 100644 (file)
@@ -820,9 +820,9 @@ failedJSONP:
         codeBlock = jsCast<ProgramCodeBlock*>(tempCodeBlock);
     }
 
-    VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
-    if (UNLIKELY(vm.needTrapHandling(mask))) {
-        vm.handleTraps(globalObject, vm.topCallFrame, mask);
+    constexpr auto trapsMask = VMTraps::interruptingTraps();
+    if (UNLIKELY(vm.needTrapHandling(trapsMask))) {
+        vm.handleTraps(globalObject, vm.topCallFrame, trapsMask);
         RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
     }
 
@@ -881,9 +881,9 @@ JSValue Interpreter::executeCall(JSGlobalObject* lexicalGlobalObject, JSObject*
     } else
         newCodeBlock = 0;
 
-    VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
-    if (UNLIKELY(vm.needTrapHandling(mask))) {
-        vm.handleTraps(globalObject, vm.topCallFrame, mask);
+    constexpr auto trapsMask = VMTraps::interruptingTraps();
+    if (UNLIKELY(vm.needTrapHandling(trapsMask))) {
+        vm.handleTraps(globalObject, vm.topCallFrame, trapsMask);
         RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
     }
 
@@ -952,9 +952,9 @@ JSObject* Interpreter::executeConstruct(JSGlobalObject* lexicalGlobalObject, JSO
     } else
         newCodeBlock = 0;
 
-    VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
-    if (UNLIKELY(vm.needTrapHandling(mask))) {
-        vm.handleTraps(globalObject, vm.topCallFrame, mask);
+    constexpr auto trapsMask = VMTraps::interruptingTraps();
+    if (UNLIKELY(vm.needTrapHandling(trapsMask))) {
+        vm.handleTraps(globalObject, vm.topCallFrame, trapsMask);
         RETURN_IF_EXCEPTION(throwScope, nullptr);
     }
 
@@ -1136,9 +1136,9 @@ JSValue Interpreter::execute(EvalExecutable* eval, JSGlobalObject* lexicalGlobal
         }
     }
 
-    VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
-    if (UNLIKELY(vm.needTrapHandling(mask))) {
-        vm.handleTraps(globalObject, vm.topCallFrame, mask);
+    constexpr auto trapsMask = VMTraps::interruptingTraps();
+    if (UNLIKELY(vm.needTrapHandling(trapsMask))) {
+        vm.handleTraps(globalObject, vm.topCallFrame, trapsMask);
         RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
     }
 
@@ -1186,9 +1186,9 @@ JSValue Interpreter::executeModuleProgram(ModuleProgramExecutable* executable, J
         codeBlock = jsCast<ModuleProgramCodeBlock*>(tempCodeBlock);
     }
 
-    VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
-    if (UNLIKELY(vm.needTrapHandling(mask))) {
-        vm.handleTraps(globalObject, vm.topCallFrame, mask);
+    constexpr auto trapsMask = VMTraps::interruptingTraps();
+    if (UNLIKELY(vm.needTrapHandling(trapsMask))) {
+        vm.handleTraps(globalObject, vm.topCallFrame, trapsMask);
         RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
     }
 
index e024652..e1a9294 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2016 Yusuke Suzuki <utatane.tea@gmail.com>
- * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -74,9 +74,9 @@ ALWAYS_INLINE JSValue Interpreter::execute(CallFrameClosure& closure)
 
     StackStats::CheckPoint stackCheckPoint;
 
-    VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
-    if (UNLIKELY(vm.needTrapHandling(mask))) {
-        vm.handleTraps(closure.protoCallFrame->globalObject, closure.oldCallFrame, mask);
+    constexpr auto trapsMask = VMTraps::interruptingTraps();
+    if (UNLIKELY(vm.needTrapHandling(trapsMask))) {
+        vm.handleTraps(closure.protoCallFrame->globalObject, closure.oldCallFrame, trapsMask);
         RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
     }
 
index 663c34c..1ce27c5 100644 (file)
@@ -70,6 +70,7 @@
 #include "SuperSampler.h"
 #include "TestRunnerUtils.h"
 #include "TypedArrayInlines.h"
+#include "VMInspector.h"
 #include "WasmCapabilities.h"
 #include "WasmContext.h"
 #include "WasmFaultSignalHandler.h"
@@ -84,6 +85,7 @@
 #include <thread>
 #include <type_traits>
 #include <wtf/Box.h>
+#include <wtf/CPUTime.h>
 #include <wtf/CommaPrinter.h>
 #include <wtf/FileSystem.h>
 #include <wtf/MainThread.h>
@@ -2427,24 +2429,58 @@ int jscmain(int argc, char** argv);
 
 static double s_desiredTimeout;
 static double s_timeoutMultiplier = 1.0;
+static Seconds s_timeoutDuration;
+static Seconds s_maxAllowedCPUTime;
+static VM* s_vm;
+
+static void startTimeoutTimer(Seconds duration)
+{
+    Thread::create("jsc Timeout Thread", [=] () {
+        sleep(duration);
+        VMInspector::forEachVM([&] (VM& vm) -> VMInspector::FunctorStatus {
+            if (&vm != s_vm)
+                return VMInspector::FunctorStatus::Continue;
+            vm.notifyNeedShellTimeoutCheck();
+            return VMInspector::FunctorStatus::Done;
+        });
+    });
+}
+
+static void timeoutCheckCallback(VM& vm)
+{
+    RELEASE_ASSERT(&vm == s_vm);
+    auto cpuTime = CPUTime::forCurrentThread();
+    if (cpuTime >= s_maxAllowedCPUTime) {
+        dataLog("Timed out after ", s_timeoutDuration, " seconds!\n");
+        CRASH();
+    }
+    auto remainingTime = s_maxAllowedCPUTime - cpuTime;
+    startTimeoutTimer(remainingTime);
+}
 
-static void startTimeoutThreadIfNeeded()
+static void initializeTimeoutIfNeeded()
 {
     if (char* timeoutString = getenv("JSCTEST_timeout")) {
         if (sscanf(timeoutString, "%lf", &s_desiredTimeout) != 1) {
             dataLog("WARNING: timeout string is malformed, got ", timeoutString,
                 " but expected a number. Not using a timeout.\n");
-        } else {
-            Thread::create("jsc Timeout Thread", [] () {
-                Seconds timeoutDuration(s_desiredTimeout * s_timeoutMultiplier);
-                sleep(timeoutDuration);
-                dataLog("Timed out after ", timeoutDuration, " seconds!\n");
-                CRASH();
-            });
-        }
+        } else
+            g_jscConfig.shellTimeoutCheckCallback = timeoutCheckCallback;
     }
 }
 
+static void startTimeoutThreadIfNeeded(VM& vm)
+{
+    if (!g_jscConfig.shellTimeoutCheckCallback)
+        return;
+
+    s_vm = &vm;
+    s_timeoutDuration = Seconds(s_desiredTimeout * s_timeoutMultiplier);
+    s_maxAllowedCPUTime = CPUTime::forCurrentThread() + s_timeoutDuration;
+    Seconds timeoutDuration(s_desiredTimeout * s_timeoutMultiplier);
+    startTimeoutTimer(timeoutDuration);
+}
+
 int main(int argc, char** argv)
 {
 #if PLATFORM(IOS_FAMILY) && CPU(ARM_THUMB2)
@@ -2999,6 +3035,7 @@ int runJSC(const CommandLine& options, bool isWorker, const Func& func)
     {
         JSLockHolder locker(vm);
 
+        startTimeoutThreadIfNeeded(vm);
         if (options.m_profile && !vm.m_perBytecodeProfiler)
             vm.m_perBytecodeProfiler = makeUnique<Profiler::Database>(vm);
 
@@ -3099,7 +3136,7 @@ int jscmain(int argc, char** argv)
 
     // Initialize JSC before getting VM.
     JSC::initializeThreading();
-    startTimeoutThreadIfNeeded();
+    initializeTimeoutIfNeeded();
 #if ENABLE(WEBASSEMBLY)
     JSC::Wasm::enableFastMemory();
 #endif
index 2f48f75..ea2446f 100644 (file)
@@ -32,6 +32,7 @@ namespace JSC {
 
 class ExecutableAllocator;
 class FixedVMPoolExecutableAllocator;
+class VM;
 
 #if CPU(ARM64) || PLATFORM(WATCHOS)
 constexpr size_t PageSize = 16 * KB;
@@ -82,6 +83,8 @@ struct Config {
 #endif
 
             OptionsStorage options;
+
+            void (*shellTimeoutCheckCallback)(VM&);
         };
         char ensureSize[ConfigSizeToProtect];
     };
index fe1e9bf..28086c4 100644 (file)
@@ -1067,6 +1067,7 @@ public:
     void* needTrapHandlingAddress() { return m_traps.needTrapHandlingAddress(); }
 
     void notifyNeedDebuggerBreak() { m_traps.fireTrap(VMTraps::NeedDebuggerBreak); }
+    void notifyNeedShellTimeoutCheck() { m_traps.fireTrap(VMTraps::NeedShellTimeoutCheck); }
     void notifyNeedTermination() { m_traps.fireTrap(VMTraps::NeedTermination); }
     void notifyNeedWatchdogCheck() { m_traps.fireTrap(VMTraps::NeedWatchdogCheck); }
 
index bf66092..8449d45 100644 (file)
@@ -357,7 +357,12 @@ void VMTraps::handleTraps(JSGlobalObject* globalObject, CallFrame* callFrame, VM
             dataLog("VM ", RawPointer(&vm), " on pid ", getCurrentProcessID(), " received NeedDebuggerBreak trap\n");
             invalidateCodeBlocksOnStack(callFrame);
             break;
-                
+
+        case NeedShellTimeoutCheck:
+            RELEASE_ASSERT(g_jscConfig.shellTimeoutCheckCallback);
+            g_jscConfig.shellTimeoutCheckCallback(vm);
+            break;
+
         case NeedWatchdogCheck:
             ASSERT(vm.watchdog());
             if (LIKELY(!vm.watchdog()->shouldTerminate(globalObject)))
index 9ae8ec8..72c0716 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -52,6 +52,7 @@ public:
     enum EventType {
         // Sorted in servicing priority order from highest to lowest.
         NeedDebuggerBreak,
+        NeedShellTimeoutCheck,
         NeedTermination,
         NeedWatchdogCheck,
         NumberOfEventTypes, // This entry must be last in this list.
@@ -61,13 +62,16 @@ public:
     class Mask {
     public:
         enum AllEventTypes { AllEventTypesTag };
-        Mask(AllEventTypes)
+        constexpr Mask(AllEventTypes)
             : m_mask(std::numeric_limits<BitField>::max())
         { }
-        static Mask allEventTypes() { return Mask(AllEventTypesTag); }
+        static constexpr Mask allEventTypes() { return Mask(AllEventTypesTag); }
+
+        constexpr Mask(const Mask&) = default;
+        constexpr Mask(Mask&&) = default;
 
         template<typename... Arguments>
-        Mask(Arguments... args)
+        constexpr Mask(Arguments... args)
             : m_mask(0)
         {
             init(args...);
@@ -77,18 +81,20 @@ public:
 
     private:
         template<typename... Arguments>
-        void init(EventType eventType, Arguments... args)
+        constexpr void init(EventType eventType, Arguments... args)
         {
             ASSERT(eventType < NumberOfEventTypes);
             m_mask |= (1 << eventType);
             init(args...);
         }
 
-        void init() { }
+        constexpr void init() { }
 
         BitField m_mask;
     };
 
+    static constexpr Mask interruptingTraps() { return Mask(NeedShellTimeoutCheck, NeedTermination, NeedWatchdogCheck); }
+
     ~VMTraps();
     VMTraps();
 
index f03f16f..81c9f1f 100644 (file)
@@ -106,6 +106,13 @@ static bool ensureIsSafeToLock(Lock& lock)
 };
 #endif // ENABLE(JIT)
 
+void VMInspector::forEachVM(Function<FunctorStatus(VM&)>&& func)
+{
+    VMInspector& inspector = instance();
+    Locker lock(inspector.getLock());
+    inspector.iterate(func);
+}
+
 auto VMInspector::isValidExecutableMemory(const VMInspector::Locker&, void* machinePC) -> Expected<bool, Error>
 {
 #if ENABLE(JIT)
index 5f5c1d8..30108bd 100644 (file)
@@ -60,6 +60,8 @@ public:
     template <typename Functor>
     void iterate(const Locker&, const Functor& functor) { iterate(functor); }
 
+    JS_EXPORT_PRIVATE static void forEachVM(Function<FunctorStatus(VM&)>&&);
+
     Expected<Locker, Error> lock(Seconds timeout = Seconds::infinity());
 
     Expected<bool, Error> isValidExecutableMemory(const Locker&, void*);