Changeset 253613 in webkit


Ignore:
Timestamp:
Dec 17, 2019, 12:50:20 AM (5 years ago)
Author:
mark.lam@apple.com
Message:

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:
Location:
trunk/Source/JavaScriptCore
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r253609 r253613  
     12019-12-16  Mark Lam  <mark.lam@apple.com>
     2
     3        Relanding r253581: Changed jsc shell timeout mechanism to leverage the VMTraps and use CPUTime.
     4        https://bugs.webkit.org/show_bug.cgi?id=205279
     5        <rdar://problem/57971874>
     6
     7        Reviewed by Saam Barati.
     8
     9        This fixes all the timeouts that occur due to CPU time starvation when
     10        running JSC tests on a debug build.
     11
     12        What this means is that the timeout mechanism may trigger asynchronous
     13        OSR exits.  If a test requires no OSR exits, that test should
     14        requireOption("--usePollingTraps=true") so that the VMTraps will use its
     15        polling implementation instead.
     16
     17        I've tested this with a full run of the JSC stress tests with a debug
     18        build and saw 0 timeouts.  I've also tested it with a contrived tests that
     19        loops forever, and saw the expected timeout crash.
     20
     21        Will look into re-tuning needed timeout value (and other JSC tests timeout
     22        cleanup) in https://bugs.webkit.org/show_bug.cgi?id=205298.
     23
     24        Update: in the previously landed patch, I did a last minute sort of the cases
     25        Int the switch statement in VMTraps::handleTraps() before posting my patch.
     26        This is incorrect to do since one of the cases need to fall through to another
     27        case.  This patch undoes the sorting to the order I originally had the cases
     28        in during development and testing.
     29
     30        * interpreter/Interpreter.cpp:
     31        (JSC::Interpreter::executeProgram):
     32        (JSC::Interpreter::executeCall):
     33        (JSC::Interpreter::executeConstruct):
     34        (JSC::Interpreter::execute):
     35        (JSC::Interpreter::executeModuleProgram):
     36        * interpreter/InterpreterInlines.h:
     37        (JSC::Interpreter::execute):
     38        * jsc.cpp:
     39        (startTimeoutTimer):
     40        (timeoutCheckCallback):
     41        (initializeTimeoutIfNeeded):
     42        (startTimeoutThreadIfNeeded):
     43        (runJSC):
     44        (jscmain):
     45        * runtime/JSCConfig.h:
     46        * runtime/VM.h:
     47        (JSC::VM::notifyNeedShellTimeoutCheck):
     48        * runtime/VMTraps.cpp:
     49        (JSC::VMTraps::handleTraps):
     50        * runtime/VMTraps.h:
     51        (JSC::VMTraps::Mask::Mask):
     52        (JSC::VMTraps::Mask::allEventTypes):
     53        (JSC::VMTraps::Mask::init):
     54        (JSC::VMTraps::interruptingTraps):
     55        * tools/VMInspector.cpp:
     56        (JSC::VMInspector::forEachVM):
     57        * tools/VMInspector.h:
     58
    1592019-12-16  Mark Lam  <mark.lam@apple.com>
    260

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

    r253609 r253613  
    821821    }
    822822
    823     VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
    824     if (UNLIKELY(vm.needTrapHandling(mask))) {
    825         vm.handleTraps(globalObject, vm.topCallFrame, mask);
     823    constexpr auto trapsMask = VMTraps::interruptingTraps();
     824    if (UNLIKELY(vm.needTrapHandling(trapsMask))) {
     825        vm.handleTraps(globalObject, vm.topCallFrame, trapsMask);
    826826        RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
    827827    }
    … …  
    882882        newCodeBlock = 0;
    883883
    884     VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
    885     if (UNLIKELY(vm.needTrapHandling(mask))) {
    886         vm.handleTraps(globalObject, vm.topCallFrame, mask);
     884    constexpr auto trapsMask = VMTraps::interruptingTraps();
     885    if (UNLIKELY(vm.needTrapHandling(trapsMask))) {
     886        vm.handleTraps(globalObject, vm.topCallFrame, trapsMask);
    887887        RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
    888888    }
    … …  
    953953        newCodeBlock = 0;
    954954
    955     VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
    956     if (UNLIKELY(vm.needTrapHandling(mask))) {
    957         vm.handleTraps(globalObject, vm.topCallFrame, mask);
     955    constexpr auto trapsMask = VMTraps::interruptingTraps();
     956    if (UNLIKELY(vm.needTrapHandling(trapsMask))) {
     957        vm.handleTraps(globalObject, vm.topCallFrame, trapsMask);
    958958        RETURN_IF_EXCEPTION(throwScope, nullptr);
    959959    }
    … …  
    11371137    }
    11381138
    1139     VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
    1140     if (UNLIKELY(vm.needTrapHandling(mask))) {
    1141         vm.handleTraps(globalObject, vm.topCallFrame, mask);
     1139    constexpr auto trapsMask = VMTraps::interruptingTraps();
     1140    if (UNLIKELY(vm.needTrapHandling(trapsMask))) {
     1141        vm.handleTraps(globalObject, vm.topCallFrame, trapsMask);
    11421142        RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
    11431143    }
    … …  
    11871187    }
    11881188
    1189     VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
    1190     if (UNLIKELY(vm.needTrapHandling(mask))) {
    1191         vm.handleTraps(globalObject, vm.topCallFrame, mask);
     1189    constexpr auto trapsMask = VMTraps::interruptingTraps();
     1190    if (UNLIKELY(vm.needTrapHandling(trapsMask))) {
     1191        vm.handleTraps(globalObject, vm.topCallFrame, trapsMask);
    11921192        RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
    11931193    }

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

    r253609 r253613  
    11/*
    22 * Copyright (C) 2016 Yusuke Suzuki <utatane.tea@gmail.com>
    3  * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
     3 * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
    44 *
    55 * Redistribution and use in source and binary forms, with or without
    … …  
    7575    StackStats::CheckPoint stackCheckPoint;
    7676
    77     VMTraps::Mask mask(VMTraps::NeedTermination, VMTraps::NeedWatchdogCheck);
    78     if (UNLIKELY(vm.needTrapHandling(mask))) {
    79         vm.handleTraps(closure.protoCallFrame->globalObject, closure.oldCallFrame, mask);
     77    constexpr auto trapsMask = VMTraps::interruptingTraps();
     78    if (UNLIKELY(vm.needTrapHandling(trapsMask))) {
     79        vm.handleTraps(closure.protoCallFrame->globalObject, closure.oldCallFrame, trapsMask);
    8080        RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
    8181    }

  • trunk/Source/JavaScriptCore/jsc.cpp

    r253609 r253613  
    7171#include "TestRunnerUtils.h"
    7272#include "TypedArrayInlines.h"
     73#include "VMInspector.h"
    7374#include "WasmCapabilities.h"
    7475#include "WasmContext.h"
    … …  
    8586#include <type_traits>
    8687#include <wtf/Box.h>
     88#include <wtf/CPUTime.h>
    8789#include <wtf/CommaPrinter.h>
    8890#include <wtf/FileSystem.h>
    … …  
    24282430static double s_desiredTimeout;
    24292431static double s_timeoutMultiplier = 1.0;
    2430 
    2431 static void startTimeoutThreadIfNeeded()
     2432static Seconds s_timeoutDuration;
     2433static Seconds s_maxAllowedCPUTime;
     2434static VM* s_vm;
     2435
     2436static void startTimeoutTimer(Seconds duration)
     2437{
     2438    Thread::create("jsc Timeout Thread", [=] () {
     2439        sleep(duration);
     2440        VMInspector::forEachVM([&] (VM& vm) -> VMInspector::FunctorStatus {
     2441            if (&vm != s_vm)
     2442                return VMInspector::FunctorStatus::Continue;
     2443            vm.notifyNeedShellTimeoutCheck();
     2444            return VMInspector::FunctorStatus::Done;
     2445        });
     2446    });
     2447}
     2448
     2449static void timeoutCheckCallback(VM& vm)
     2450{
     2451    RELEASE_ASSERT(&vm == s_vm);
     2452    auto cpuTime = CPUTime::forCurrentThread();
     2453    if (cpuTime >= s_maxAllowedCPUTime) {
     2454        dataLog("Timed out after ", s_timeoutDuration, " seconds!\n");
     2455        CRASH();
     2456    }
     2457    auto remainingTime = s_maxAllowedCPUTime - cpuTime;
     2458    startTimeoutTimer(remainingTime);
     2459}
     2460
     2461static void initializeTimeoutIfNeeded()
    24322462{
    24332463    if (char* timeoutString = getenv("JSCTEST_timeout")) {
    … …  
    24352465            dataLog("WARNING: timeout string is malformed, got ", timeoutString,
    24362466                " but expected a number. Not using a timeout.\n");
    2437         } else {
    2438             Thread::create("jsc Timeout Thread", [] () {
    2439                 Seconds timeoutDuration(s_desiredTimeout * s_timeoutMultiplier);
    2440                 sleep(timeoutDuration);
    2441                 dataLog("Timed out after ", timeoutDuration, " seconds!\n");
    2442                 CRASH();
    2443             });
    2444         }
    2445     }
     2467        } else
     2468            g_jscConfig.shellTimeoutCheckCallback = timeoutCheckCallback;
     2469    }
     2470}
     2471
     2472static void startTimeoutThreadIfNeeded(VM& vm)
     2473{
     2474    if (!g_jscConfig.shellTimeoutCheckCallback)
     2475        return;
     2476
     2477    s_vm = &vm;
     2478    s_timeoutDuration = Seconds(s_desiredTimeout * s_timeoutMultiplier);
     2479    s_maxAllowedCPUTime = CPUTime::forCurrentThread() + s_timeoutDuration;
     2480    Seconds timeoutDuration(s_desiredTimeout * s_timeoutMultiplier);
     2481    startTimeoutTimer(timeoutDuration);
    24462482}
    24472483
    … …  
    30003036        JSLockHolder locker(vm);
    30013037
     3038        startTimeoutThreadIfNeeded(vm);
    30023039        if (options.m_profile && !vm.m_perBytecodeProfiler)
    30033040            vm.m_perBytecodeProfiler = makeUnique<Profiler::Database>(vm);
    … …  
    31003137    // Initialize JSC before getting VM.
    31013138    JSC::initializeThreading();
    3102     startTimeoutThreadIfNeeded();
     3139    initializeTimeoutIfNeeded();
    31033140#if ENABLE(WEBASSEMBLY)
    31043141    JSC::Wasm::enableFastMemory();

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

    r253609 r253613  
    3333class ExecutableAllocator;
    3434class FixedVMPoolExecutableAllocator;
     35class VM;
    3536
    3637#if CPU(ARM64) || PLATFORM(WATCHOS)
    … …  
    8384
    8485            OptionsStorage options;
     86
     87            void (*shellTimeoutCheckCallback)(VM&);
    8588        };
    8689        char ensureSize[ConfigSizeToProtect];

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

    r253609 r253613  
    10681068
    10691069    void notifyNeedDebuggerBreak() { m_traps.fireTrap(VMTraps::NeedDebuggerBreak); }
     1070    void notifyNeedShellTimeoutCheck() { m_traps.fireTrap(VMTraps::NeedShellTimeoutCheck); }
    10701071    void notifyNeedTermination() { m_traps.fireTrap(VMTraps::NeedTermination); }
    10711072    void notifyNeedWatchdogCheck() { m_traps.fireTrap(VMTraps::NeedWatchdogCheck); }

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

    r253609 r253613  
    358358            invalidateCodeBlocksOnStack(callFrame);
    359359            break;
    360                
     360
     361        case NeedShellTimeoutCheck:
     362            RELEASE_ASSERT(g_jscConfig.shellTimeoutCheckCallback);
     363            g_jscConfig.shellTimeoutCheckCallback(vm);
     364            break;
     365
    361366        case NeedWatchdogCheck:
    362367            ASSERT(vm.watchdog());

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

    r253609 r253613  
    11/*
    2  * Copyright (C) 2017 Apple Inc. All rights reserved.
     2 * Copyright (C) 2017-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    5353        // Sorted in servicing priority order from highest to lowest.
    5454        NeedDebuggerBreak,
     55        NeedShellTimeoutCheck,
    5556        NeedTermination,
    5657        NeedWatchdogCheck,
    … …  
    6263    public:
    6364        enum AllEventTypes { AllEventTypesTag };
    64         Mask(AllEventTypes)
     65        constexpr Mask(AllEventTypes)
    6566            : m_mask(std::numeric_limits<BitField>::max())
    6667        { }
    67         static Mask allEventTypes() { return Mask(AllEventTypesTag); }
     68        static constexpr Mask allEventTypes() { return Mask(AllEventTypesTag); }
     69
     70        constexpr Mask(const Mask&) = default;
     71        constexpr Mask(Mask&&) = default;
    6872
    6973        template<typename... Arguments>
    70         Mask(Arguments... args)
     74        constexpr Mask(Arguments... args)
    7175            : m_mask(0)
    7276        {
    … …  
    7882    private:
    7983        template<typename... Arguments>
    80         void init(EventType eventType, Arguments... args)
     84        constexpr void init(EventType eventType, Arguments... args)
    8185        {
    8286            ASSERT(eventType < NumberOfEventTypes);
    … …  
    8589        }
    8690
    87         void init() { }
     91        constexpr void init() { }
    8892
    8993        BitField m_mask;
    9094    };
     95
     96    static constexpr Mask interruptingTraps() { return Mask(NeedShellTimeoutCheck, NeedTermination, NeedWatchdogCheck); }
    9197
    9298    ~VMTraps();

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

    r253609 r253613  
    107107#endif // ENABLE(JIT)
    108108
     109void VMInspector::forEachVM(Function<FunctorStatus(VM&)>&& func)
     110{
     111    VMInspector& inspector = instance();
     112    Locker lock(inspector.getLock());
     113    inspector.iterate(func);
     114}
     115
    109116auto VMInspector::isValidExecutableMemory(const VMInspector::Locker&, void* machinePC) -> Expected<bool, Error>
    110117{

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

    r253609 r253613  
    6161    void iterate(const Locker&, const Functor& functor) { iterate(functor); }
    6262
     63    JS_EXPORT_PRIVATE static void forEachVM(Function<FunctorStatus(VM&)>&&);
     64
    6365    Expected<Locker, Error> lock(Seconds timeout = Seconds::infinity());
    6466
Note: See TracChangeset for help on using the changeset viewer.