From 678691f70aab81905a96734b51dab99c0439635a Mon Sep 17 00:00:00 2001 From: Hans Boehm Date: Wed, 8 May 2024 17:01:36 -0700 Subject: Conditionally increase suspend timeout This does two things: 1. Respect ro.hw_timeout_multiplier when computing the default suspend timeout. 2. Increase the timeout by a factor of 2 when the thread initiating the suspension has low priority (positive nice value or Java priority < 5). Case 2 should not normally apply for threads with a 5 second ANR timeout. For suspension initiated from low priority threads, this should get us back to close to the original 10 second timeout. It should also give slow device the option of avoiding suspension timeouts by setting ro.hw_timeout_multiplier. Also includes some minor macro cleanups. Test: Treehugger Test: Some manual checking of suspend timeout values Bug: 339660702 Bug: 337585993 Bug: 327315839 Bug: 199683153 Bug: 297125507 (and others) Change-Id: I18638667a8d46d117b06ecedca13df9a6a7f8530 Change-Id: I12d2d7c047194775e1789b02603d3c1d96cc3191 --- compiler/driver/compiler_options_map.h | 2 +- runtime/runtime.cc | 23 ++++++++++++++++------- runtime/runtime_options.def | 3 +-- runtime/runtime_options.h | 4 +--- runtime/thread_list.cc | 18 ++++++++++++++---- 5 files changed, 33 insertions(+), 17 deletions(-) diff --git a/compiler/driver/compiler_options_map.h b/compiler/driver/compiler_options_map.h index b2dd57d00e..136af36096 100644 --- a/compiler/driver/compiler_options_map.h +++ b/compiler/driver/compiler_options_map.h @@ -42,7 +42,7 @@ struct CompilerOptionsMap : VariantMap { #include "compiler_options_map.def" }; -#undef DECLARE_KEY +#undef COMPILER_OPTIONS_KEY } // namespace art diff --git a/runtime/runtime.cc b/runtime/runtime.cc index 4f8872f322..bc833b8cfa 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -32,16 +32,17 @@ #include // for _NSGetEnviron #endif +#include +#include +#include + #include #include #include -#include #include #include #include -#include "android-base/strings.h" - #include "arch/arm/registers_arm.h" #include "arch/arm64/registers_arm64.h" #include "arch/context.h" @@ -75,8 +76,8 @@ #include "debugger.h" #include "dex/art_dex_file_loader.h" #include "dex/dex_file_loader.h" -#include "entrypoints/runtime_asm_entrypoints.h" #include "entrypoints/entrypoint_utils-inl.h" +#include "entrypoints/runtime_asm_entrypoints.h" #include "experimental_flags.h" #include "fault_handler.h" #include "gc/accounting/card_table-inl.h" @@ -116,8 +117,8 @@ #include "mirror/throwable.h" #include "mirror/var_handle.h" #include "monitor.h" -#include "native/dalvik_system_DexFile.h" #include "native/dalvik_system_BaseDexClassLoader.h" +#include "native/dalvik_system_DexFile.h" #include "native/dalvik_system_VMDebug.h" #include "native/dalvik_system_VMRuntime.h" #include "native/dalvik_system_VMStack.h" @@ -143,12 +144,12 @@ #include "native/java_lang_reflect_Parameter.h" #include "native/java_lang_reflect_Proxy.h" #include "native/java_util_concurrent_atomic_AtomicLong.h" +#include "native/jdk_internal_misc_Unsafe.h" #include "native/libcore_io_Memory.h" #include "native/libcore_util_CharsetUtils.h" #include "native/org_apache_harmony_dalvik_ddmc_DdmServer.h" #include "native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.h" #include "native/sun_misc_Unsafe.h" -#include "native/jdk_internal_misc_Unsafe.h" #include "native_bridge_art_interface.h" #include "native_stack_dump.h" #include "nativehelper/scoped_local_ref.h" @@ -1512,6 +1513,14 @@ static std::vector FileFdsToFileObjects(std::vector&& fds) { return files; } +inline static uint64_t GetThreadSuspendTimeout(const RuntimeArgumentMap* runtime_options) { + auto suspend_timeout_opt = runtime_options->GetOptional(RuntimeArgumentMap::ThreadSuspendTimeout); + return suspend_timeout_opt.has_value() ? + suspend_timeout_opt.value().GetNanoseconds() : + ThreadList::kDefaultThreadSuspendTimeout * + android::base::GetIntProperty("ro.hw_timeout_multiplier", 1); +} + bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) { // (b/30160149): protect subprocesses from modifications to LD_LIBRARY_PATH, etc. // Take a snapshot of the environment at the time the runtime was created, for use by Exec, etc. @@ -1657,7 +1666,7 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) { monitor_list_ = new MonitorList; monitor_pool_ = MonitorPool::Create(); - thread_list_ = new ThreadList(runtime_options.GetOrDefault(Opt::ThreadSuspendTimeout)); + thread_list_ = new ThreadList(GetThreadSuspendTimeout(&runtime_options)); intern_table_ = new InternTable; monitor_timeout_enable_ = runtime_options.GetOrDefault(Opt::MonitorTimeoutEnable); diff --git a/runtime/runtime_options.def b/runtime/runtime_options.def index 8dcba19113..e82f6566b2 100644 --- a/runtime/runtime_options.def +++ b/runtime/runtime_options.def @@ -70,8 +70,7 @@ RUNTIME_OPTIONS_KEY (MillisecondsToNanoseconds, \ LongPauseLogThreshold, gc::Heap::kDefaultLongPauseLogThreshold) RUNTIME_OPTIONS_KEY (MillisecondsToNanoseconds, \ LongGCLogThreshold, gc::Heap::kDefaultLongGCLogThreshold) -RUNTIME_OPTIONS_KEY (MillisecondsToNanoseconds, \ - ThreadSuspendTimeout, ThreadList::kDefaultThreadSuspendTimeout) +RUNTIME_OPTIONS_KEY (MillisecondsToNanoseconds, ThreadSuspendTimeout) RUNTIME_OPTIONS_KEY (bool, MonitorTimeoutEnable, false) RUNTIME_OPTIONS_KEY (int, MonitorTimeout, Monitor::kDefaultMonitorTimeoutMs) RUNTIME_OPTIONS_KEY (Unit, DumpGCPerformanceOnShutdown) diff --git a/runtime/runtime_options.h b/runtime/runtime_options.h index 3cadd09bf8..9e127925ee 100644 --- a/runtime/runtime_options.h +++ b/runtime/runtime_options.h @@ -41,8 +41,6 @@ class DexFile; struct XGcOption; struct BackgroundGcOption; -#define DECLARE_KEY(Type, Name) static const Key Name - // Define a key that is usable with a RuntimeArgumentMap. // This key will *not* work with other subtypes of VariantMap. template @@ -75,7 +73,7 @@ struct EXPORT RuntimeArgumentMap : VariantMap +#include +#include +#include // For getpriority() #include #include @@ -26,10 +29,6 @@ #include #include "android-base/stringprintf.h" -#include "nativehelper/scoped_local_ref.h" -#include "nativehelper/scoped_utf_chars.h" -#include "unwindstack/AndroidUnwinder.h" - #include "art_field-inl.h" #include "base/aborting.h" #include "base/histogram-inl.h" @@ -52,6 +51,7 @@ #include "scoped_thread_state_change-inl.h" #include "thread.h" #include "trace.h" +#include "unwindstack/AndroidUnwinder.h" #include "well_known_classes.h" #if ART_USE_FUTEXES @@ -714,6 +714,16 @@ std::optional ThreadList::WaitForSuspendBarrier(AtomicInteger* barr #endif uint64_t timeout_ns = attempt_of_4 == 0 ? thread_suspend_timeout_ns_ : thread_suspend_timeout_ns_ / 4; + if (attempt_of_4 != 1 && getpriority(PRIO_PROCESS, 0 /* this thread */) > 0) { + // We're a low priority thread, and thus have a longer ANR timeout. Double the suspend + // timeout. To avoid the getpriority system call in the common case, we fail to double the + // first of 4 waits, but then triple the third one to compensate. + if (attempt_of_4 == 3) { + timeout_ns *= 3; + } else { + timeout_ns *= 2; + } + } bool collect_state = (t != 0 && (attempt_of_4 == 0 || attempt_of_4 == 4)); int32_t cur_val = barrier->load(std::memory_order_acquire); if (cur_val <= 0) { -- cgit v1.2.3