Bug 221104 – [macOS] Policy for warning about or killing processes using too much memory triggers too easily

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

Policy for warning about or killing processes using too much memory triggers too easily.

[Chris Dumez]

<rdar://73625621>

[Chris Dumez]

Created attachment 418672 [details]
Patch

[Mark Lam]

Comment on attachment 418672 [details]
Patch

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

> Source/WTF/wtf/MemoryPressureHandler.cpp:-110
> -    size_t baseThreshold = 2 * GB;
> -#if CPU(X86_64) || CPU(ARM64)
> -    if (processState == WebsamProcessState::Active)
> -        baseThreshold = 4 * GB;
> +    size_t baseThreshold = 4 * GB;
>      if (tabCount > 1)
>          baseThreshold += std::min(tabCount - 1, 4u) * 1 * GB;
> -#else
> -    if ((tabCount > 1) || (processState == WebsamProcessState::Active))
> -        baseThreshold = 3 * GB;
> -#endif

This also increases the threshold for armv7k and other (presumably embedded) linux ports using non-x86 CPUs.  Is this intensional?

[Chris Dumez]

Created attachment 418674 [details]
Patch

[Chris Dumez]

(In reply to Mark Lam from comment #3)
> Comment on attachment 418672 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=418672&action=review
> 
> > Source/WTF/wtf/MemoryPressureHandler.cpp:-110
> > -    size_t baseThreshold = 2 * GB;
> > -#if CPU(X86_64) || CPU(ARM64)
> > -    if (processState == WebsamProcessState::Active)
> > -        baseThreshold = 4 * GB;
> > +    size_t baseThreshold = 4 * GB;
> >      if (tabCount > 1)
> >          baseThreshold += std::min(tabCount - 1, 4u) * 1 * GB;
> > -#else
> > -    if ((tabCount > 1) || (processState == WebsamProcessState::Active))
> > -        baseThreshold = 3 * GB;
> > -#endif
> 
> This also increases the threshold for armv7k and other (presumably embedded)
> linux ports using non-x86 CPUs.  Is this intensional?

Yes, I was trying to simplify things and it was not obvious to me what platform this was targeting. Anyway, I have now updated my patch to maintain previous behavior for armv7k.

[Geoffrey Garen]

Comment on attachment 418674 [details]
Patch

r=me

[Saam Barati]

Comment on attachment 418674 [details]
Patch

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

> Source/WTF/wtf/MemoryPressureHandler.cpp:104
> +    size_t baseThreshold = 4 * GB;
>      if (tabCount > 1)
>          baseThreshold += std::min(tabCount - 1, 4u) * 1 * GB;

what's the point of this math?

[Chris Dumez]

Comment on attachment 418674 [details]
Patch

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

>> Source/WTF/wtf/MemoryPressureHandler.cpp:104
>>          baseThreshold += std::min(tabCount - 1, 4u) * 1 * GB;
> 
> what's the point of this math?

We had some pathological cases where a many tabs would end up in the same WebProcess (Clicking links in a Gmail tab iirc). This was a way to give more memory to processes that are shared by more than 1 tab (we give an extra GB per extra tab, no more than 4 extra GBs total).

[EWS]

commit-queue failed to commit attachment 418674 [details] to WebKit repository.

[Chris Dumez]

Created attachment 418689 [details]
Patch

[EWS]

Committed r272046: <https://trac.webkit.org/changeset/272046>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 418689 [details].