RESOLVED FIXED 180861
[GTK][WPE] Enable WebProcess memory monitor
https://bugs.webkit.org/show_bug.cgi?id=180861
Summary [GTK][WPE] Enable WebProcess memory monitor
Carlos Garcia Campos
Reported 2017-12-15 02:47:18 PST
For some reason this is only enabled in mac. We want to enable it also in GTK and WPE ports. This runs every 30 seconds to release memory or even kill the process if necessary. Carlos López has realized that in some applications using video tags, the memory grows a lot and it's never released. It seems it's not memory leaked, but simply large memory allocations (I guess it's gst allocating video frames) that make the heap grow. The memory pressure calls malloc_trim that releases all that memory keeping the web process footprint stable. Since enabling this can make the web process to be killed, we need to expose the web process termination reason in the API. I'll file another bug for this.
Attachments
Patch (2.03 KB, patch)
2017-12-15 02:50 PST, Carlos Garcia Campos
mcatanzaro: review+
Carlos Garcia Campos
Comment 1 2017-12-15 02:50:08 PST
Michael Catanzaro
Comment 2 2017-12-15 08:52:24 PST
What is the memory limit?
Carlos Garcia Campos
Comment 3 2017-12-16 03:36:38 PST
(In reply to Michael Catanzaro from comment #2) > What is the memory limit? It depends. There are several limits: - thresholdForMemoryKill: this is the one checked to decide whether to kill the process. This is calculated depending on the number of page/tabs in the web process and depending on whether the web process is currently active or not. If the limit is reached, a release memory is done and if after that the limit is still reached then the web process asks the UI to kill it. See: size_t baseThreshold; if (processState == WebsamProcessState::Active) baseThreshold = 4 * GB; else baseThreshold = 2 * GB; if (tabCount <= 1) return baseThreshold; return baseThreshold + (std::min(tabCount - 1, 4u) * 1 * GB); - UsagePolicyBasedOnFootprint: this is to decide whether to try to free some memory or not depending on the web process current memory footprint. If the policy is set to Unrestricted nothing happens, if Conservative then non-critical memory is released and Strict tries to release all possible memory. See: switch (policy) { case MemoryUsagePolicy::Conservative: return 1 * GB; case MemoryUsagePolicy::Strict: return 1.5 * GB; case MemoryUsagePolicy::Unrestricted: default: ASSERT_NOT_REACHED(); return 0; } We might want to make this configurable, though. In a device with 1GB of RAM these values don't make sense.
Carlos Alberto Lopez Perez
Comment 4 2017-12-16 06:57:44 PST
(In reply to Carlos Garcia Campos from comment #3) > - UsagePolicyBasedOnFootprint: this is to decide whether to try to free > some memory or not depending on the web process current memory footprint. If > the policy is set to Unrestricted nothing happens, if Conservative then > non-critical memory is released and Strict tries to release all possible > memory. See: > > switch (policy) { > case MemoryUsagePolicy::Conservative: > return 1 * GB; > case MemoryUsagePolicy::Strict: > return 1.5 * GB; > case MemoryUsagePolicy::Unrestricted: > default: > ASSERT_NOT_REACHED(); > return 0; > } > > We might want to make this configurable, though. In a device with 1GB of RAM > these values don't make sense. Instead of making this configurable, we can have a dynamic thresold that depends on the amount of RAM of the device. A simple rule may be: baseThresholdForPolicy == std::min(3 * GB, totalSystemRAM) switch (policy) { case MemoryUsagePolicy::Conservative: return baseThresholdForPolicy / 2; case MemoryUsagePolicy::Strict: return baseThresholdForPolicy / 3; case MemoryUsagePolicy::Unrestricted: default: ASSERT_NOT_REACHED(); return 0; } This should give the same values it is already giving if the device has 3GB or more, otherwise it will give values that depend on the amount of RAM and should always make sense. WDYT?
Carlos Alberto Lopez Perez
Comment 5 2017-12-16 07:01:21 PST
(In reply to Carlos Alberto Lopez Perez from comment #4) > Instead of making this configurable, we can have a dynamic thresold that > depends on the amount of RAM of the device. A simple rule may be: > > > baseThresholdForPolicy == std::min(3 * GB, totalSystemRAM) > > switch (policy) { > case MemoryUsagePolicy::Conservative: > return baseThresholdForPolicy / 2; ^ 3 > case MemoryUsagePolicy::Strict: > return baseThresholdForPolicy / 3; ^ 2 > case MemoryUsagePolicy::Unrestricted: > default: > ASSERT_NOT_REACHED(); > return 0; > } > Ups, I got confused with the values above. The 2 and the 3 should be exchanged. @@ -2,9 +2,9 @@ switch (policy) { case MemoryUsagePolicy::Conservative: - return baseThresholdForPolicy / 2; + return baseThresholdForPolicy / 3; case MemoryUsagePolicy::Strict: - return baseThresholdForPolicy / 3; + return baseThresholdForPolicy / 2; case MemoryUsagePolicy::Unrestricted: default: ASSERT_NOT_REACHED();
Carlos Garcia Campos
Comment 6 2017-12-17 01:30:45 PST
(In reply to Carlos Alberto Lopez Perez from comment #4) > (In reply to Carlos Garcia Campos from comment #3) > > - UsagePolicyBasedOnFootprint: this is to decide whether to try to free > > some memory or not depending on the web process current memory footprint. If > > the policy is set to Unrestricted nothing happens, if Conservative then > > non-critical memory is released and Strict tries to release all possible > > memory. See: > > > > switch (policy) { > > case MemoryUsagePolicy::Conservative: > > return 1 * GB; > > case MemoryUsagePolicy::Strict: > > return 1.5 * GB; > > case MemoryUsagePolicy::Unrestricted: > > default: > > ASSERT_NOT_REACHED(); > > return 0; > > } > > > > We might want to make this configurable, though. In a device with 1GB of RAM > > these values don't make sense. > > Instead of making this configurable, we can have a dynamic thresold that > depends on the amount of RAM of the device. Sounds like a good idea, please file a new bug report for that and include akling in the CC. > A simple rule may be: > > > baseThresholdForPolicy == std::min(3 * GB, totalSystemRAM) > > switch (policy) { > case MemoryUsagePolicy::Conservative: > return baseThresholdForPolicy / 2; > case MemoryUsagePolicy::Strict: > return baseThresholdForPolicy / 3; > case MemoryUsagePolicy::Unrestricted: > default: > ASSERT_NOT_REACHED(); > return 0; > } > > This should give the same values it is already giving if the device has 3GB > or more, otherwise it will give values that depend on the amount of RAM and > should always make sense. > > > WDYT?
Carlos Garcia Campos
Comment 7 2017-12-18 00:25:16 PST
Radar WebKit Bug Importer
Comment 8 2017-12-18 00:26:19 PST
Carlos Alberto Lopez Perez
Comment 9 2018-04-18 16:01:43 PDT
*** Bug 175133 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.