Bug 224798 – Make sure we don't exit the GPUProcess too frequently while under memory pressure

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

Make sure we don't exit the GPUProcess too frequently while under memory pressure.

[Chris Dumez]

Created attachment 426514 [details]
Patch

[Darin Adler]

Comment on attachment 426514 [details]
Patch

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

> Source/WebKit/GPUProcess/GPUProcess.cpp:136
> +    // To avoid exiting the GPUProcess too aggressively while under memory pressure, we don't exit if we've been running
> +    // for less than 5 seconds. In case of simulated memory pressure, we ignore this rule to avoid generating flakiness
> +    // in our benchmarks and tests.
> +    if ((MonotonicTime::now() - m_creationTime) < 5_s && !MemoryPressureHandler::singleton().isSimulatingMemoryPressure())
> +        return false;

In cases like this I really like a comment somewhere explaining the choice of 5 seconds. One way to do that is to use a named constant at the top of the file and have the comment there focus on how we selected 5 seconds over other possible values. It’s sort of a "human being taking a pause" constant, I think?

[EWS]

Committed r276305 (236787@main): <https://commits.webkit.org/236787@main>

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

[Geoffrey Garen]

If we know that we're under memory pressure, and we also know that it's too soon to exit the GPU Process right now, shouldn't we schedule an exit for a time that would be more appropriate?

[Chris Dumez]

(In reply to Geoffrey Garen from comment #4)
> If we know that we're under memory pressure, and we also know that it's too
> soon to exit the GPU Process right now, shouldn't we schedule an exit for a
> time that would be more appropriate?

I think it would be appropriate although probably not critical.

I made this change because when the system is under memory pressure, the GPUProcess would exit very shortly after launch, even though it was about the be useful. If we launch the GPUProcess, then it indicates the WebProcess is about to use it very shortly. When the WebProcess, stops using it, we will already exit the GPUProcess if we are still under memory pressure and this time.

So the proposal to schedule an exit when receiving a memory pressure signal if we've just launched seems to be for this scenario:

The GPUProcess launched less than 5 seconds ago, it was used for a very short amount of time by the WebProcess (less than 5 seconds) and we did not exit when the WebProcess stopped using it because we were not under memory pressure and it had been less than 5 seconds. We then receive a memory pressure signal before the 5 seconds deadline and we don't exit. After the 5 seconds mark, we don't exit the GPUProcess if we are still under memory pressure, which is a bit unfortunate indeed. To cause the GPUProcess to exit at that point, we'd need either a new memory pressure signal OR a WebProcess would have to use the GPUProcess then stop using it.

I think based on this, I will follow-up and and such a timer.

[Chris Dumez]

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

[Chris Dumez]

(In reply to Geoffrey Garen from comment #4)
> If we know that we're under memory pressure, and we also know that it's too
> soon to exit the GPU Process right now, shouldn't we schedule an exit for a
> time that would be more appropriate?

Following up to do this in Bug 224829.

[Ling Ho]

rdar://76918236