Bug 221104

Summary: [macOS] Policy for warning about or killing processes using too much memory triggers too easily
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, benjamin, cmarcelo, ews-watchlist, fpizlo, ggaren, mark.lam, mjs, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch none

Chris Dumez
Reported 2021-01-28 14:27:57 PST
Policy for warning about or killing processes using too much memory triggers too easily.
Attachments
Patch (8.70 KB, patch)
2021-01-28 14:35 PST, Chris Dumez 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (8.76 KB, patch)
2021-01-28 15:08 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (8.77 KB, patch)
2021-01-28 18:32 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2021-01-28 14:28:08 PST
<rdar://73625621>
Chris Dumez
Comment 2 2021-01-28 14:35:46 PST
Created attachment 418672 [details] Patch
Mark Lam
Comment 3 2021-01-28 15:02:55 PST
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
Comment 4 2021-01-28 15:08:38 PST
Created attachment 418674 [details] Patch
Chris Dumez
Comment 5 2021-01-28 15:10:02 PST
(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 6 2021-01-28 15:13:31 PST
Comment on attachment 418674 [details] Patch r=me
Saam Barati
Comment 7 2021-01-28 15:47:06 PST
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 8 2021-01-28 15:49:06 PST
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
Comment 9 2021-01-28 17:20:20 PST
commit-queue failed to commit attachment 418674 [details] to WebKit repository.
Chris Dumez
Comment 10 2021-01-28 18:32:24 PST
Created attachment 418689 [details] Patch
EWS
Comment 11 2021-01-28 19:57:59 PST
Committed r272046: <https://trac.webkit.org/changeset/272046> All reviewed patches have been landed. Closing bug and clearing flags on attachment 418689 [details].
Note You need to log in before you can comment on or make changes to this bug.