Changeset 253581 in webkit

Timestamp:

Dec 16, 2019, 2:34:50 PM (5 years ago)

Author:

mark.lam@apple.com

Message:

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.

• 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:

(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:

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• interpreter/Interpreter.cpp (modified) (5 diffs)
• interpreter/InterpreterInlines.h (modified) (2 diffs)
• jsc.cpp (modified) (6 diffs)
• runtime/JSCConfig.h (modified) (2 diffs)
• runtime/VM.h (modified) (1 diff)
• runtime/VMTraps.cpp (modified) (2 diffs)
• runtime/VMTraps.h (modified) (5 diffs)
• tools/VMInspector.cpp (modified) (1 diff)
• tools/VMInspector.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog ¶

@@ +1 @@
+2019-12-16  Mark Lam  <mark.lam@apple.com>
+
+        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.
+
+        * 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:
+        (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  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp ¶

@@ -821 +821 @@
     }
 
-    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());
     }
@@ -882 +882 @@
         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());
     }
@@ -953 +953 @@
         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);
     }
@@ -1137 +1137 @@
     }
 
-    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());
     }
@@ -1187 +1187 @@
     }
 
-    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());
     }

trunk/Source/JavaScriptCore/interpreter/InterpreterInlines.h ¶

@@ -1 +1 @@
 /*
  * 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
@@ -75 +75 @@
     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());
     }

trunk/Source/JavaScriptCore/jsc.cpp ¶

@@ -71 +71 @@
 #include "TestRunnerUtils.h"
 #include "TypedArrayInlines.h"
+#include "VMInspector.h"
 #include "WasmCapabilities.h"
 #include "WasmContext.h"
@@ -85 +86 @@
 #include <type_traits>
 #include <wtf/Box.h>
+#include <wtf/CPUTime.h>
 #include <wtf/CommaPrinter.h>
 #include <wtf/FileSystem.h>
@@ -2428 +2430 @@
 static double s_desiredTimeout;
 static double s_timeoutMultiplier = 1.0;
-
-static void startTimeoutThreadIfNeeded()
+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 initializeTimeoutIfNeeded()
 {
     if (char* timeoutString = getenv("JSCTEST_timeout")) {
@@ -2435 +2465 @@
             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);
 }
 
@@ -3000 +3036 @@
         JSLockHolder locker(vm);
 
+        startTimeoutThreadIfNeeded(vm);
         if (options.m_profile && !vm.m_perBytecodeProfiler)
             vm.m_perBytecodeProfiler = makeUnique<Profiler::Database>(vm);
@@ -3100 +3137 @@
     // Initialize JSC before getting VM.
     JSC::initializeThreading();
-    startTimeoutThreadIfNeeded();
+    initializeTimeoutIfNeeded();
 #if ENABLE(WEBASSEMBLY)
     JSC::Wasm::enableFastMemory();

trunk/Source/JavaScriptCore/runtime/JSCConfig.h ¶

@@ -33 +33 @@
 class ExecutableAllocator;
 class FixedVMPoolExecutableAllocator;
+class VM;
 
 #if CPU(ARM64) || PLATFORM(WATCHOS)
@@ -83 +84 @@
 
             OptionsStorage options;
+
+            void (*shellTimeoutCheckCallback)(VM&);
         };
         char ensureSize[ConfigSizeToProtect];

trunk/Source/JavaScriptCore/runtime/VM.h ¶

@@ -1066 +1066 @@
 
     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); }

trunk/Source/JavaScriptCore/runtime/VMTraps.cpp ¶

@@ -358 +358 @@
             invalidateCodeBlocksOnStack(callFrame);
             break;
-                
+
         case NeedWatchdogCheck:
             ASSERT(vm.watchdog());
@@ -364 +364 @@
                 continue;
             FALLTHROUGH;
+
+        case NeedShellTimeoutCheck:
+            RELEASE_ASSERT(g_jscConfig.shellTimeoutCheckCallback);
+            g_jscConfig.shellTimeoutCheckCallback(vm);
+            break;
 
         case NeedTermination:

trunk/Source/JavaScriptCore/runtime/VMTraps.h ¶

@@ -1 +1 @@
 /*
- * 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
@@ -53 +53 @@
         // Sorted in servicing priority order from highest to lowest.
         NeedDebuggerBreak,
+        NeedShellTimeoutCheck,
         NeedTermination,
         NeedWatchdogCheck,
@@ -62 +63 @@
     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)
         {
@@ -78 +82 @@
     private:
         template<typename... Arguments>
-        void init(EventType eventType, Arguments... args)
+        constexpr void init(EventType eventType, Arguments... args)
         {
             ASSERT(eventType < NumberOfEventTypes);
@@ -85 +89 @@
         }
 
-        void init() { }
+        constexpr void init() { }
 
         BitField m_mask;
     };
+
+    static constexpr Mask interruptingTraps() { return Mask(NeedShellTimeoutCheck, NeedTermination, NeedWatchdogCheck); }
 
     ~VMTraps();

trunk/Source/JavaScriptCore/tools/VMInspector.cpp ¶

@@ -107 +107 @@
 #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>
 {

trunk/Source/JavaScriptCore/tools/VMInspector.h ¶

@@ -61 +61 @@
     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());