Bug 227001 – Too much CPU time is spent under MemoryPressureHandler::currentMemoryUsagePolicy()

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Chris Dumez]

Too much CPU time is spent under MemoryPressureHandler::currentMemoryUsagePolicy().

Looking at profiles of our WebContent processes, we frequently see traces like these:
Sample Count, Samples %, Normalized CPU %, Symbol
31, 0.0%, 0.0%, WebCore::RenderLayerCompositor::cacheAcceleratedCompositingFlags() (in WebCore)
30, 0.0%, 0.0%,     WTF::MemoryPressureHandler::currentMemoryUsagePolicy() (in JavaScriptCore)
30, 0.0%, 0.0%,         task_info (in libsystem_kernel.dylib)

It is a decent amount of samples and shows several times per process.

We should try and minimize calls to WTF::MemoryPressureHandler::currentMemoryUsagePolicy() to reduce CPU usage.

[Ben Nham]

Maybe we should just ditch computing the memory footprint on every call and map MemoryUsagePolicy to OS memory pressure levels (e.g. unrestricted => normal pressure, conservative => warning pressure, strict => critical pressure). Then we could just rely on the OS memory pressure notification handler to update MemoryUsagePolicy.

I guess the other obvious thing to do here is to cache the memoryFootprint for X seconds.

[Chris Dumez]

Created attachment 431904 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 431904 [details]
Patch

10s seems too long. Maybe 1s or 2s?

[Simon Fraser (smfr)]

Or we could do it once per rendering update.

[Chris Dumez]

Created attachment 431906 [details]
Patch

[Geoffrey Garen]

Can we pick the delay based on what it would take to make the % time in a representative profile negligible?

In the snippet posted here, the % time is already 0.0% (rounded, I assume), so I'm not sure any delay would be justified.

To say out loud what is perhaps implicit so far: memory pressure can change rapidly, so there is some benefit to responding as quickly as possible (but not so quickly that we waste CPU).

[Simon Fraser (smfr)]

(In reply to Geoffrey Garen from comment #6)
> Can we pick the delay based on what it would take to make the % time in a
> representative profile negligible?
> 
> In the snippet posted here, the % time is already 0.0% (rounded, I assume),
> so I'm not sure any delay would be justified.
> 
> To say out loud what is perhaps implicit so far: memory pressure can change
> rapidly, so there is some benefit to responding as quickly as possible (but
> not so quickly that we waste CPU).

I suspect that the only cases where it's non-negligible are those where there are many forced layouts per rendering update. So once per rendering update feels right to me.

[Chris Dumez]

(In reply to Geoffrey Garen from comment #6)
> Can we pick the delay based on what it would take to make the % time in a
> representative profile negligible?
> 
> In the snippet posted here, the % time is already 0.0% (rounded, I assume),
> so I'm not sure any delay would be justified.

This shows many times per profile. Not sure how to aggregate all the calls in the profiler.

> 
> To say out loud what is perhaps implicit so far: memory pressure can change
> rapidly, so there is some benefit to responding as quickly as possible (but
> not so quickly that we waste CPU).

[Chris Dumez]

(In reply to Chris Dumez from comment #8)
> (In reply to Geoffrey Garen from comment #6)
> > Can we pick the delay based on what it would take to make the % time in a
> > representative profile negligible?
> > 
> > In the snippet posted here, the % time is already 0.0% (rounded, I assume),
> > so I'm not sure any delay would be justified.
> 
> This shows many times per profile. Not sure how to aggregate all the calls
> in the profiler.
> 
> > 
> > To say out loud what is perhaps implicit so far: memory pressure can change
> > rapidly, so there is some benefit to responding as quickly as possible (but
> > not so quickly that we waste CPU).

If the point is to respond to memory pressure, then we have the memory pressure warning/signal for that. This code relies on a certain % of the memory being used, which is not the same as memory pressure.

[Chris Dumez]

(In reply to Chris Dumez from comment #9)
> (In reply to Chris Dumez from comment #8)
> > (In reply to Geoffrey Garen from comment #6)
> > > Can we pick the delay based on what it would take to make the % time in a
> > > representative profile negligible?
> > > 
> > > In the snippet posted here, the % time is already 0.0% (rounded, I assume),
> > > so I'm not sure any delay would be justified.
> > 
> > This shows many times per profile. Not sure how to aggregate all the calls
> > in the profiler.
> > 
> > > 
> > > To say out loud what is perhaps implicit so far: memory pressure can change
> > > rapidly, so there is some benefit to responding as quickly as possible (but
> > > not so quickly that we waste CPU).
> 
> If the point is to respond to memory pressure, then we have the memory
> pressure warning/signal for that. This code relies on a certain % of the
> memory being used, which is not the same as memory pressure.

And this threshold appears to be 33% of RAM size of macOS and 50% of RAM size on iOS.

[EWS]

Committed r279084 (239003@main): <https://commits.webkit.org/239003@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/79581141>