Bug 180861 – [GTK][WPE] Enable WebProcess memory monitor

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

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

Created attachment 329481 [details]
Patch

[Michael Catanzaro]

What is the memory limit?

[Carlos Garcia Campos]

(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]

(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]

(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]

(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]

Committed r226018: <https://trac.webkit.org/changeset/226018>

[Radar WebKit Bug Importer]

<rdar://problem/36101218>

[Carlos Alberto Lopez Perez]

*** Bug 175133 has been marked as a duplicate of this bug. ***