Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (253612 => 253613)
--- trunk/Source/_javascript_Core/ChangeLog 2019-12-17 08:14:07 UTC (rev 253612)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-12-17 08:50:20 UTC (rev 253613)
@@ -1,5 +1,63 @@
2019-12-16 Mark Lam <mark....@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....@apple.com>
+
Rolling out: r253581 is failing tests on a release build.
https://bugs.webkit.org/show_bug.cgi?id=205279
<rdar://problem/57971874>
Modified: trunk/Source/_javascript_Core/interpreter/Interpreter.cpp (253612 => 253613)
--- trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2019-12-17 08:14:07 UTC (rev 253612)
+++ trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2019-12-17 08:50:20 UTC (rev 253613)
@@ -820,9 +820,9 @@
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 @@
} 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 @@
} 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 @@
}
}
- 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 @@
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());
}
Modified: trunk/Source/_javascript_Core/interpreter/InterpreterInlines.h (253612 => 253613)
--- trunk/Source/_javascript_Core/interpreter/InterpreterInlines.h 2019-12-17 08:14:07 UTC (rev 253612)
+++ trunk/Source/_javascript_Core/interpreter/InterpreterInlines.h 2019-12-17 08:50:20 UTC (rev 253613)
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2016 Yusuke Suzuki <utatane....@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 @@
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());
}
Modified: trunk/Source/_javascript_Core/jsc.cpp (253612 => 253613)
--- trunk/Source/_javascript_Core/jsc.cpp 2019-12-17 08:14:07 UTC (rev 253612)
+++ trunk/Source/_javascript_Core/jsc.cpp 2019-12-17 08:50:20 UTC (rev 253613)
@@ -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 @@
static double s_desiredTimeout;
static double s_timeoutMultiplier = 1.0;
+static Seconds s_timeoutDuration;
+static Seconds s_maxAllowedCPUTime;
+static VM* s_vm;
-static void startTimeoutThreadIfNeeded()
+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")) {
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 @@
{
JSLockHolder locker(vm);
+ startTimeoutThreadIfNeeded(vm);
if (options.m_profile && !vm.m_perBytecodeProfiler)
vm.m_perBytecodeProfiler = makeUnique<Profiler::Database>(vm);
@@ -3099,7 +3136,7 @@
// Initialize JSC before getting VM.
JSC::initializeThreading();
- startTimeoutThreadIfNeeded();
+ initializeTimeoutIfNeeded();
#if ENABLE(WEBASSEMBLY)
JSC::Wasm::enableFastMemory();
#endif
Modified: trunk/Source/_javascript_Core/runtime/JSCConfig.h (253612 => 253613)
--- trunk/Source/_javascript_Core/runtime/JSCConfig.h 2019-12-17 08:14:07 UTC (rev 253612)
+++ trunk/Source/_javascript_Core/runtime/JSCConfig.h 2019-12-17 08:50:20 UTC (rev 253613)
@@ -32,6 +32,7 @@
class ExecutableAllocator;
class FixedVMPoolExecutableAllocator;
+class VM;
#if CPU(ARM64) || PLATFORM(WATCHOS)
constexpr size_t PageSize = 16 * KB;
@@ -82,6 +83,8 @@
#endif
OptionsStorage options;
+
+ void (*shellTimeoutCheckCallback)(VM&);
};
char ensureSize[ConfigSizeToProtect];
};
Modified: trunk/Source/_javascript_Core/runtime/VM.h (253612 => 253613)
--- trunk/Source/_javascript_Core/runtime/VM.h 2019-12-17 08:14:07 UTC (rev 253612)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2019-12-17 08:50:20 UTC (rev 253613)
@@ -1067,6 +1067,7 @@
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); }
Modified: trunk/Source/_javascript_Core/runtime/VMTraps.cpp (253612 => 253613)
--- trunk/Source/_javascript_Core/runtime/VMTraps.cpp 2019-12-17 08:14:07 UTC (rev 253612)
+++ trunk/Source/_javascript_Core/runtime/VMTraps.cpp 2019-12-17 08:50:20 UTC (rev 253613)
@@ -357,7 +357,12 @@
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)))
Modified: trunk/Source/_javascript_Core/runtime/VMTraps.h (253612 => 253613)
--- trunk/Source/_javascript_Core/runtime/VMTraps.h 2019-12-17 08:14:07 UTC (rev 253612)
+++ trunk/Source/_javascript_Core/runtime/VMTraps.h 2019-12-17 08:50:20 UTC (rev 253613)
@@ -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 @@
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 @@
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,7 +81,7 @@
private:
template<typename... Arguments>
- void init(EventType eventType, Arguments... args)
+ constexpr void init(EventType eventType, Arguments... args)
{
ASSERT(eventType < NumberOfEventTypes);
m_mask |= (1 << eventType);
@@ -84,11 +88,13 @@
init(args...);
}
- void init() { }
+ constexpr void init() { }
BitField m_mask;
};
+ static constexpr Mask interruptingTraps() { return Mask(NeedShellTimeoutCheck, NeedTermination, NeedWatchdogCheck); }
+
~VMTraps();
VMTraps();
Modified: trunk/Source/_javascript_Core/tools/VMInspector.cpp (253612 => 253613)
--- trunk/Source/_javascript_Core/tools/VMInspector.cpp 2019-12-17 08:14:07 UTC (rev 253612)
+++ trunk/Source/_javascript_Core/tools/VMInspector.cpp 2019-12-17 08:50:20 UTC (rev 253613)
@@ -106,6 +106,13 @@
};
#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)
Modified: trunk/Source/_javascript_Core/tools/VMInspector.h (253612 => 253613)
--- trunk/Source/_javascript_Core/tools/VMInspector.h 2019-12-17 08:14:07 UTC (rev 253612)
+++ trunk/Source/_javascript_Core/tools/VMInspector.h 2019-12-17 08:50:20 UTC (rev 253613)
@@ -60,6 +60,8 @@
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*);
