[Webkit-unassigned] [Bug 188299] Web process never leaves memory pressured state if caused by process size limit

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=188299

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |darin at apple.com
 Attachment #346599|review?                     |review+
              Flags|                            |

--- Comment #8 from Darin Adler <darin at apple.com> ---
Comment on attachment 346599
  --> https://bugs.webkit.org/attachment.cgi?id=346599
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=346599&action=review

> Source/WTF/wtf/cocoa/MemoryPressureHandlerCocoa.mm:36
> +#if PLATFORM(IOS) && USE(APPLE_INTERNAL_SDK)
> +#include <dispatch/private.h>
> +#endif

Can we use an XXXSPI.h header for this instead?

> Source/WTF/wtf/cocoa/MemoryPressureHandlerCocoa.mm:55
> -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];

Why not use standard WebKit naming style? The underscores here are not our usual style.

> Source/WTF/wtf/cocoa/MemoryPressureHandlerCocoa.mm:78
> +#if USE(APPLE_INTERNAL_SDK)
> +        memoryStatusFlags |= DISPATCH_MEMORYPRESSURE_PROC_LIMIT_WARN | DISPATCH_MEMORYPRESSURE_PROC_LIMIT_CRITICAL;
> +#endif

Don’t we want to avoid things like this where we compile incorrectly when USE(APPLE_INTERNAL_SDK) is not set?

> Source/WTF/wtf/cocoa/MemoryPressureHandlerCocoa.mm:104
> +#if USE(APPLE_INTERNAL_SDK)
> +            // Process memory limit events.

Ditto.

> Source/WTF/wtf/cocoa/MemoryPressureHandlerCocoa.mm:117
> +            if (response)
> +                MemoryPressureHandler::singleton().respondToMemoryPressure(*response);

Seems like we could make these calls inside the switch statement. Or could move the setUnderMemoryPressure call out here.

I don’t see a compelling reason to use std::optional for "response" but to for "under memory pressure".

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180805/4024c795/attachment.html>