Attachment 346625 Details for Bug 188299 – patch

[patch] patch

process-limit-notifications-4.patch (text/plain), 13.13 KB, created by Antti Koivisto on 2018-08-06 06:34:53 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2018-08-06 06:34:53 PDT

Size: 13.13 KB

patch

obsolete

>Index: Source/WTF/ChangeLog
>===================================================================
>--- Source/WTF/ChangeLog	(revision 234598)
>+++ Source/WTF/ChangeLog	(working copy)
>@@ -1,3 +1,30 @@
>+2018-08-06  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 Darin Adler.
>+
>+        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.xcodeproj/project.pbxproj:
>+        * 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.
>+
>+        (WTF::MemoryPressureHandler::uninstall):
>+        (WTF::MemoryPressureHandler::holdOff):
>+        * wtf/spi/darwin/DispatchSPI.h: Added.
>+
> 2018-08-03  Ben Richards  <benton_richards@apple.com>
> 
>         We should cache the compiled sandbox profile in a data vault
>Index: Source/WTF/WTF.xcodeproj/project.pbxproj
>===================================================================
>--- Source/WTF/WTF.xcodeproj/project.pbxproj	(revision 234496)
>+++ Source/WTF/WTF.xcodeproj/project.pbxproj	(working copy)
>@@ -632,6 +632,7 @@
> 		E3A32BC21FC830E2007D7E76 /* JSValueMalloc.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSValueMalloc.cpp; sourceTree = "<group>"; };
> 		E3A32BC31FC830E2007D7E76 /* JSValueMalloc.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSValueMalloc.h; sourceTree = "<group>"; };
> 		E3E158251EADA53C004A079D /* SystemFree.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SystemFree.h; sourceTree = "<group>"; };
>+		E431CC4A21187ADB000C8A07 /* DispatchSPI.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DispatchSPI.h; sourceTree = "<group>"; };
> 		E4A0AD371A96245500536DF6 /* WorkQueue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WorkQueue.cpp; sourceTree = "<group>"; };
> 		E4A0AD381A96245500536DF6 /* WorkQueue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WorkQueue.h; sourceTree = "<group>"; };
> 		E4A0AD3C1A96253C00536DF6 /* WorkQueueCocoa.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WorkQueueCocoa.cpp; sourceTree = "<group>"; };
>@@ -1287,6 +1288,7 @@
> 			isa = PBXGroup;
> 			children = (
> 				DE5A09FB1BA36992003D4424 /* CommonCryptoSPI.h */,
>+				E431CC4A21187ADB000C8A07 /* DispatchSPI.h */,
> 				93DDE9311CDC052D00FD3491 /* dyldSPI.h */,
> 				A5098AFF1C169E0700087797 /* SandboxSPI.h */,
> 				CE73E02419DCB7AB00580D5C /* XPCSPI.h */,
>Index: Source/WTF/wtf/cocoa/MemoryPressureHandlerCocoa.mm
>===================================================================
>--- Source/WTF/wtf/cocoa/MemoryPressureHandlerCocoa.mm	(revision 234496)
>+++ Source/WTF/wtf/cocoa/MemoryPressureHandlerCocoa.mm	(working copy)
>@@ -30,6 +30,7 @@
> #import <mach/task_info.h>
> #import <malloc/malloc.h>
> #import <notify.h>
>+#import <wtf/spi/darwin/DispatchSPI.h>
> 
> #define ENABLE_FMW_FOOTPRINT_COMPARISON 0
> 
>@@ -46,9 +47,9 @@ void MemoryPressureHandler::platformRele
>     }
> }
> 
>-static dispatch_source_t _cache_event_source = 0;
>-static dispatch_source_t _timer_event_source = 0;
>-static int _notifyTokens[3];
>+static dispatch_source_t memoryPressureEventSource = nullptr;
>+static dispatch_source_t timerEventSource = nullptr;
>+static int notifyTokens[3];
> 
> // Disable memory event reception for a minimum of s_minimumHoldOffTime
> // seconds after receiving an event. Don't let events fire any sooner than
>@@ -63,41 +64,51 @@ static const unsigned s_holdOffMultiplie
> 
> void MemoryPressureHandler::install()
> {
>-    if (m_installed || _timer_event_source)
>+    if (m_installed || timerEventSource)
>         return;
> 
>     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 | DISPATCH_MEMORYPRESSURE_PROC_LIMIT_WARN | DISPATCH_MEMORYPRESSURE_PROC_LIMIT_CRITICAL;
>+#else // PLATFORM(MAC)
>+        auto memoryStatusFlags = DISPATCH_MEMORYPRESSURE_CRITICAL;
> #endif
>+        memoryPressureEventSource = 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(memoryPressureEventSource, ^{
>+            auto status = dispatch_source_get_data(memoryPressureEventSource);
> #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;
>+            switch (status) {
>+            // VM pressure events.
>+            case DISPATCH_MEMORYPRESSURE_NORMAL:
>+                setUnderMemoryPressure(false);
>+                break;
>+            case DISPATCH_MEMORYPRESSURE_WARN:
>+                setUnderMemoryPressure(false);
>+                respondToMemoryPressure(Critical::No);
>+                break;
>+            case DISPATCH_MEMORYPRESSURE_CRITICAL:
>+                setUnderMemoryPressure(true);
>+                respondToMemoryPressure(Critical::Yes);
>+                break;
>+            // Process memory limit events.
>+            case DISPATCH_MEMORYPRESSURE_PROC_LIMIT_WARN:
>+                respondToMemoryPressure(Critical::No);
>+                break;
>+            case DISPATCH_MEMORYPRESSURE_PROC_LIMIT_CRITICAL:
>+                respondToMemoryPressure(Critical::Yes);
>+                break;
>             }
>-
>-            if (ReliefLogger::loggingEnabled())
>-                NSLog(@"Got memory pressure notification (%s)", critical ? "critical" : "non-critical");
>+#else // PLATFORM(MAC)
>+            respondToMemoryPressure(Critical::Yes);
> #endif
>-            MemoryPressureHandler::singleton().respondToMemoryPressure(critical ? Critical::Yes : Critical::No);
>+            WTFLogAlways("Received memory pressure event %lu vm pressure %d", status, isUnderMemoryPressure());
>         });
>-        dispatch_resume(_cache_event_source);
>+        dispatch_resume(memoryPressureEventSource);
>     });
> 
>     // Allow simulation of memory pressure with "notifyutil -p org.WebKit.lowMemory"
>-    notify_register_dispatch("org.WebKit.lowMemory", &_notifyTokens[0], dispatch_get_main_queue(), ^(int) {
>+    notify_register_dispatch("org.WebKit.lowMemory", &notifyTokens[0], dispatch_get_main_queue(), ^(int) {
> #if ENABLE(FMW_FOOTPRINT_COMPARISON)
>         auto footprintBefore = pagesPerVMTag();
> #endif
>@@ -116,10 +127,10 @@ void MemoryPressureHandler::install()
>         });
>     });
> 
>-    notify_register_dispatch("org.WebKit.lowMemory.begin", &_notifyTokens[1], dispatch_get_main_queue(), ^(int) {
>+    notify_register_dispatch("org.WebKit.lowMemory.begin", &notifyTokens[1], dispatch_get_main_queue(), ^(int) {
>         beginSimulatedMemoryPressure();
>     });
>-    notify_register_dispatch("org.WebKit.lowMemory.end", &_notifyTokens[2], dispatch_get_main_queue(), ^(int) {
>+    notify_register_dispatch("org.WebKit.lowMemory.end", &notifyTokens[2], dispatch_get_main_queue(), ^(int) {
>         endSimulatedMemoryPressure();
>     });
> 
>@@ -132,40 +143,40 @@ 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 (memoryPressureEventSource) {
>+            dispatch_source_cancel(memoryPressureEventSource);
>+            memoryPressureEventSource = nullptr;
>         }
> 
>-        if (_timer_event_source) {
>-            dispatch_source_cancel(_timer_event_source);
>-            _timer_event_source = 0;
>+        if (timerEventSource) {
>+            dispatch_source_cancel(timerEventSource);
>+            timerEventSource = nullptr;
>         }
>     });
> 
>     m_installed = false;
> 
>-    for (auto& token : _notifyTokens)
>+    for (auto& token : notifyTokens)
>         notify_cancel(token);
> }
> 
> void MemoryPressureHandler::holdOff(Seconds seconds)
> {
>     dispatch_async(dispatch_get_main_queue(), ^{
>-        _timer_event_source = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, dispatch_get_main_queue());
>-        if (_timer_event_source) {
>-            dispatch_set_context(_timer_event_source, this);
>+        timerEventSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, dispatch_get_main_queue());
>+        if (timerEventSource) {
>+            dispatch_set_context(timerEventSource, this);
>             // FIXME: The final argument `s_minimumHoldOffTime.seconds()` seems wrong.
>             // https://bugs.webkit.org/show_bug.cgi?id=183277
>-            dispatch_source_set_timer(_timer_event_source, dispatch_time(DISPATCH_TIME_NOW, seconds.seconds() * NSEC_PER_SEC), DISPATCH_TIME_FOREVER, s_minimumHoldOffTime.seconds());
>-            dispatch_source_set_event_handler(_timer_event_source, ^{
>-                if (_timer_event_source) {
>-                    dispatch_source_cancel(_timer_event_source);
>-                    _timer_event_source = 0;
>+            dispatch_source_set_timer(timerEventSource, dispatch_time(DISPATCH_TIME_NOW, seconds.seconds() * NSEC_PER_SEC), DISPATCH_TIME_FOREVER, s_minimumHoldOffTime.seconds());
>+            dispatch_source_set_event_handler(timerEventSource, ^{
>+                if (timerEventSource) {
>+                    dispatch_source_cancel(timerEventSource);
>+                    timerEventSource = nullptr;
>                 }
>                 MemoryPressureHandler::singleton().install();
>             });
>-            dispatch_resume(_timer_event_source);
>+            dispatch_resume(timerEventSource);
>         }
>     });
> }
>Index: Source/WTF/wtf/spi/darwin/DispatchSPI.h
>===================================================================
>--- Source/WTF/wtf/spi/darwin/DispatchSPI.h	(nonexistent)
>+++ Source/WTF/wtf/spi/darwin/DispatchSPI.h	(working copy)
>@@ -0,0 +1,39 @@
>+/*
>+ * Copyright (C) 2018 Apple Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
>+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
>+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
>+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>+ * THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#pragma once
>+
>+#if USE(APPLE_INTERNAL_SDK)
>+
>+#include <dispatch/private.h>
>+
>+#else
>+
>+enum {
>+    DISPATCH_MEMORYPRESSURE_PROC_LIMIT_WARN = 0x10,
>+    DISPATCH_MEMORYPRESSURE_PROC_LIMIT_CRITICAL = 0x20,
>+};
>+
>+#endif

Actions: View | Formatted Diff | Diff

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