From a63e2430bfcf6532de12226f5e37ea59fc4975c8 Mon Sep 17 00:00:00 2001 From: Ray Kraesig Date: Wed, 27 Jul 2022 19:54:36 +0000 Subject: [PATCH] Bug 1716727 - [3/3] make stalling behavior conditional on process type r=glandium For now, make Set_XREProcessType set a flag in mozjemalloc to avoid stalling repeatedly in auxiliary processes. Differential Revision: https://phabricator.services.mozilla.com/D151332 --- memory/build/mozjemalloc.cpp | 25 ++++++++++++++++++++++--- memory/build/mozmemory.h | 8 ++++++++ toolkit/xre/nsEmbedFunctions.cpp | 26 +++++++++++++++++++------- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/memory/build/mozjemalloc.cpp b/memory/build/mozjemalloc.cpp index a6ce92f57f60..8797865210b8 100644 --- a/memory/build/mozjemalloc.cpp +++ b/memory/build/mozjemalloc.cpp @@ -1366,6 +1366,13 @@ static inline void ApplyZeroOrJunk(void* aPtr, size_t aSize) { // Experiment under bug 1716727. (See ./moz.build for details.) #ifdef XP_WIN + +// Whether the current process should always stall, or only stall once. +static bool sShouldAlwaysStall = true; +MOZ_JEMALLOC_API void mozjemalloc_experiment_set_always_stall(bool aVal) { + sShouldAlwaysStall = aVal; +} + # ifdef MOZ_STALL_ON_OOM // Implementation of VirtualAlloc wrapper (bug 1716727). @@ -1377,7 +1384,16 @@ constexpr size_t kMaxAttempts = 10; // Microsoft's documentation for ::Sleep() for details.) constexpr size_t kDelayMs = 50; -// Drop-in wrapper around VirtualAlloc. When out of memory, attempts to stall +Atomic sHasStalled{false}; +static bool ShouldStallAndRetry() { + if (sShouldAlwaysStall) { + return true; + } + // Otherwise, stall at most once. + return sHasStalled.compareExchange(false, true); +} + +// Drop-in wrapper around VirtualAlloc. When out of memory, may attempt to stall // and retry rather than returning immediately, in hopes that the page file is // about to be expanded by Windows. // @@ -1409,8 +1425,11 @@ constexpr size_t kDelayMs = 50; if (!(flAllocationType & MEM_COMMIT)) return nullptr; } - // Unconditionally retry. (At this level, we don't know whether the allocation - // is fallible, and arguably we should retry even if we knew that it was.) + // Also return if we just aren't supposed to be retrying at the moment, for + // whatever reason. + if (!ShouldStallAndRetry()) return nullptr; + + // Otherwise, retry. for (size_t i = 0; i < kMaxAttempts; ++i) { ::Sleep(kDelayMs); void* ptr = ::VirtualAlloc(lpAddress, dwSize, flAllocationType, flProtect); diff --git a/memory/build/mozmemory.h b/memory/build/mozmemory.h index 20b26dc307a9..6db23b9660f9 100644 --- a/memory/build/mozmemory.h +++ b/memory/build/mozmemory.h @@ -24,6 +24,7 @@ #include "mozilla/Attributes.h" #include "mozilla/Types.h" #include "mozjemalloc_types.h" +#include "stdbool.h" #ifdef MOZ_MEMORY // On OSX, malloc/malloc.h contains the declaration for malloc_good_size, @@ -60,6 +61,13 @@ static inline void jemalloc_stats(jemalloc_stats_t* aStats) { } # endif +// Temporary configurator for experiment associated with bug 1716727. +# if defined(XP_WIN) +MOZ_JEMALLOC_API void mozjemalloc_experiment_set_always_stall(bool); +# else +static inline void mozjemalloc_experiment_set_always_stall(bool){}; +# endif + #endif // MOZ_MEMORY #define NOTHROW_MALLOC_DECL(name, return_type, ...) \ diff --git a/toolkit/xre/nsEmbedFunctions.cpp b/toolkit/xre/nsEmbedFunctions.cpp index 6c17e561ce4d..5d1e8d0088fa 100644 --- a/toolkit/xre/nsEmbedFunctions.cpp +++ b/toolkit/xre/nsEmbedFunctions.cpp @@ -5,6 +5,7 @@ #include "mozilla/DebugOnly.h" #include "nsXULAppAPI.h" +#include "mozmemory.h" #include #if defined(MOZ_WIDGET_GTK) @@ -274,14 +275,25 @@ void XRE_SetProcessType(const char* aProcessTypeString) { } called = true; - sChildProcessType = GeckoProcessType_Invalid; - for (GeckoProcessType t : - MakeEnumeratedRange(GeckoProcessType::GeckoProcessType_End)) { - if (!strcmp(XRE_GeckoProcessTypeToString(t), aProcessTypeString)) { - sChildProcessType = t; - return; + sChildProcessType = [&] { + for (GeckoProcessType t : + MakeEnumeratedRange(GeckoProcessType::GeckoProcessType_End)) { + if (!strcmp(XRE_GeckoProcessTypeToString(t), aProcessTypeString)) { + return t; + } } - } + return GeckoProcessType_Invalid; + }(); + +#ifdef MOZ_MEMORY + // For the parent process, we're probably willing to accept an apparent + // lockup in preference to a crash. Always stall and retry. + // + // For child processes, an obvious OOM-crash may be preferable to slow + // performance. Retry at most once per process, then give up. + mozjemalloc_experiment_set_always_stall(sChildProcessType == + GeckoProcessType_Default); +#endif } #if defined(XP_WIN)