Attachment 346599 Details for Bug 188299 – patch

[patch] patch

process-limit-notifications-3.patch (text/plain), 6.80 KB, created by Antti Koivisto on 2018-08-05 03:26:33 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2018-08-05 03:26:33 PDT

Size: 6.80 KB

patch

obsolete

>Index: Source/WTF/ChangeLog
>===================================================================
>--- Source/WTF/ChangeLog	(revision 234539)
>+++ Source/WTF/ChangeLog	(working copy)
>@@ -1,3 +1,25 @@
>+2018-08-03  Antti Koivisto  <antti@apple.com>
>+
>+        Web process never leaves memory pressured state if caused by process size limit
>+        https://bugs.webkit.org/show_bug.cgi?id=188299
>+        <rdar://problem/42157442>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        For vm memory pressure warnings we get notified when exiting the state and we can clear
>+        the isUnderMemoryPressure bit. However as a compatibility behavior we were also notified using
>+        the same event when approaching the process size limit. In this case there is no "all clear"
>+        event so we'd stay in pressured state forever, leading to unnecessarily degraded user experience.
>+
>+        * wtf/cocoa/MemoryPressureHandlerCocoa.mm:
>+        (WTF::MemoryPressureHandler::install):
>+
>+        Install a handler for process size limit events. This disables the compatibility behavior,
>+        vm pressure events will be received for vm pressure only.
>+
>+        Process size limit events are treated as one-shot. We do cleanups based on criticality but
>+        don't enter the pressured state.
>+
> 2018-08-02  Saam Barati  <sbarati@apple.com>
> 
>         Reading instructionPointer from PlatformRegisters may fail when using pointer tagging
>Index: Source/WTF/wtf/cocoa/MemoryPressureHandlerCocoa.mm
>===================================================================
>--- Source/WTF/wtf/cocoa/MemoryPressureHandlerCocoa.mm	(revision 234496)
>+++ Source/WTF/wtf/cocoa/MemoryPressureHandlerCocoa.mm	(working copy)
>@@ -31,6 +31,10 @@
> #import <malloc/malloc.h>
> #import <notify.h>
> 
>+#if PLATFORM(IOS) && USE(APPLE_INTERNAL_SDK)
>+#include <dispatch/private.h>
>+#endif
>+
> #define ENABLE_FMW_FOOTPRINT_COMPARISON 0
> 
> extern "C" void cache_simulate_memory_warning_event(uint64_t);
>@@ -46,8 +50,8 @@ void MemoryPressureHandler::platformRele
>     }
> }
> 
>-static dispatch_source_t _cache_event_source = 0;
>-static dispatch_source_t _timer_event_source = 0;
>+static dispatch_source_t _memory_pressure_event_source = nullptr;
>+static dispatch_source_t _timer_event_source = nullptr;
> static int _notifyTokens[3];
> 
> // Disable memory event reception for a minimum of s_minimumHoldOffTime
>@@ -68,32 +72,51 @@ void MemoryPressureHandler::install()
> 
>     dispatch_async(dispatch_get_main_queue(), ^{
> #if PLATFORM(IOS)
>-        _cache_event_source = dispatch_source_create(DISPATCH_SOURCE_TYPE_MEMORYPRESSURE, 0, DISPATCH_MEMORYPRESSURE_NORMAL | DISPATCH_MEMORYPRESSURE_WARN | DISPATCH_MEMORYPRESSURE_CRITICAL, dispatch_get_main_queue());
>-#elif PLATFORM(MAC)
>-        _cache_event_source = dispatch_source_create(DISPATCH_SOURCE_TYPE_MEMORYPRESSURE, 0, DISPATCH_MEMORYPRESSURE_CRITICAL, dispatch_get_main_queue());
>+        auto memoryStatusFlags = DISPATCH_MEMORYPRESSURE_NORMAL | DISPATCH_MEMORYPRESSURE_WARN | DISPATCH_MEMORYPRESSURE_CRITICAL;
>+#if USE(APPLE_INTERNAL_SDK)
>+        memoryStatusFlags |= DISPATCH_MEMORYPRESSURE_PROC_LIMIT_WARN | DISPATCH_MEMORYPRESSURE_PROC_LIMIT_CRITICAL;
>+#endif
>+#else // PLATFORM(MAC)
>+        auto memoryStatusFlags = DISPATCH_MEMORYPRESSURE_CRITICAL;
>+        auto response = std::make_optional(Critical::Yes);
> #endif
>+        _memory_pressure_event_source = dispatch_source_create(DISPATCH_SOURCE_TYPE_MEMORYPRESSURE, 0, memoryStatusFlags, dispatch_get_main_queue());
> 
>-        dispatch_set_context(_cache_event_source, this);
>-        dispatch_source_set_event_handler(_cache_event_source, ^{
>-            bool critical = true;
>+        dispatch_source_set_event_handler(_memory_pressure_event_source, ^{
>+            auto status = dispatch_source_get_data(_memory_pressure_event_source);
> #if PLATFORM(IOS)
>-            unsigned long status = dispatch_source_get_data(_cache_event_source);
>-            critical = status == DISPATCH_MEMORYPRESSURE_CRITICAL;
>-            auto& memoryPressureHandler = MemoryPressureHandler::singleton();
>-            bool wasCritical = memoryPressureHandler.isUnderMemoryPressure();
>-            memoryPressureHandler.setUnderMemoryPressure(critical);
>-            if (status == DISPATCH_MEMORYPRESSURE_NORMAL) {
>-                if (ReliefLogger::loggingEnabled())
>-                    NSLog(@"System is no longer under (%s) memory pressure.", wasCritical ? "critical" : "non-critical");
>-                return;
>-            }
>+            std::optional<Critical> response;
> 
>-            if (ReliefLogger::loggingEnabled())
>-                NSLog(@"Got memory pressure notification (%s)", critical ? "critical" : "non-critical");
>+            switch (status) {
>+            // VM pressure events.
>+            case DISPATCH_MEMORYPRESSURE_NORMAL:
>+                MemoryPressureHandler::singleton().setUnderMemoryPressure(false);
>+                break;
>+            case DISPATCH_MEMORYPRESSURE_WARN:
>+                response = Critical::No;
>+                MemoryPressureHandler::singleton().setUnderMemoryPressure(false);
>+                break;
>+            case DISPATCH_MEMORYPRESSURE_CRITICAL:
>+                response = Critical::Yes;
>+                MemoryPressureHandler::singleton().setUnderMemoryPressure(true);
>+                break;
>+#if USE(APPLE_INTERNAL_SDK)
>+            // Process memory limit events.
>+            case DISPATCH_MEMORYPRESSURE_PROC_LIMIT_WARN:
>+                response = Critical::No;
>+                break;
>+            case DISPATCH_MEMORYPRESSURE_PROC_LIMIT_CRITICAL:
>+                response = Critical::Yes;
>+                break;
> #endif
>-            MemoryPressureHandler::singleton().respondToMemoryPressure(critical ? Critical::Yes : Critical::No);
>+            }
>+#endif
>+            WTFLogAlways("Received memory pressure event %lu vm pressure %d critical %d", status, MemoryPressureHandler::singleton().isUnderMemoryPressure(), response && *response == Critical::Yes);
>+
>+            if (response)
>+                MemoryPressureHandler::singleton().respondToMemoryPressure(*response);
>         });
>-        dispatch_resume(_cache_event_source);
>+        dispatch_resume(_memory_pressure_event_source);
>     });
> 
>     // Allow simulation of memory pressure with "notifyutil -p org.WebKit.lowMemory"
>@@ -132,14 +155,14 @@ void MemoryPressureHandler::uninstall()
>         return;
> 
>     dispatch_async(dispatch_get_main_queue(), ^{
>-        if (_cache_event_source) {
>-            dispatch_source_cancel(_cache_event_source);
>-            _cache_event_source = 0;
>+        if (_memory_pressure_event_source) {
>+            dispatch_source_cancel(_memory_pressure_event_source);
>+            _memory_pressure_event_source = nullptr;
>         }
> 
>         if (_timer_event_source) {
>             dispatch_source_cancel(_timer_event_source);
>-            _timer_event_source = 0;
>+            _timer_event_source = nullptr;
>         }
>     });
>

Flags:

darin: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 188299: 346465 | 346470 | 346599 | 346625