Bug 171289 – Call bmalloc scavenger first when handling a memory pressure event

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

When we are responding to a memory pressure notification, we free up invoke the release memory handlers for various subsystems and then call the bmalloc scavenger synchronously to free pages back to the OS.  There is a very good chance that when we receieve the low memory event that we already have physical free pages that we can immediately return to the OS for use.  Therefore we should invoke the bmalloc scavenger first in the release memory handler as well as at the end as we currently do.

<rdar://problem/31820242>

[Michael Saboff]

Created attachment 308142 [details]
Patch

[Geoffrey Garen]

I think we need a handler in bmalloc. Otherwise, stand-alone JavaScriptCore clients won't get the benefit. Here's roughly the code we need (in a Darwin-only file):

    dispatch_async(dispatch_get_main_queue(), ^{
        m_cache_event_source = dispatch_source_create(DISPATCH_SOURCE_TYPE_MEMORYPRESSURE, 0, DISPATCH_MEMORYPRESSURE_CRITICAL, dispatch_get_main_queue());
        dispatch_source_set_event_handler(m_cache_event_source, ^{
            scavenge();
        });
        dispatch_resume(m_cache_event_source);
    });

[Geoffrey Garen]

(In reply to Geoffrey Garen from comment #2)
> I think we need a handler in bmalloc. Otherwise, stand-alone JavaScriptCore
> clients won't get the benefit. Here's roughly the code we need (in a
> Darwin-only file):
> 
>     dispatch_async(dispatch_get_main_queue(), ^{
>         m_cache_event_source =
> dispatch_source_create(DISPATCH_SOURCE_TYPE_MEMORYPRESSURE, 0,
> DISPATCH_MEMORYPRESSURE_CRITICAL, dispatch_get_main_queue());
>         dispatch_source_set_event_handler(m_cache_event_source, ^{
>             scavenge();
>         });
>         dispatch_resume(m_cache_event_source);
>     });

...actually, we want to schedule our source on a custom queue, so it can run on any thread.

[Geoffrey Garen]

... we can also add this line in WebCore, to double-super guarantee WebCore behavior. There's no cost to scavenging twice: the second scavenge will be a cheap no-op.

[Michael Saboff]

Created attachment 308158 [details]
Updated Patch

[Build Bot]

Attachment 308158 [details] did not pass style-queue:


ERROR: Source/bmalloc/bmalloc/Heap.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 308158 [details]
Updated Patch

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

r=me

> Source/bmalloc/bmalloc/Heap.cpp:54
> +    auto queue = dispatch_queue_create("WebKit Malloc Memory Pressure Handler", DISPATCH_QUEUE_CONCURRENT);

It's a very minor point, but this should be DISPATCH_QUEUE_SERIAL, because we don't intend to handle multiple concurrent pressure events.

[Michael Saboff]

(In reply to Geoffrey Garen from comment #7)
> Comment on attachment 308158 [details]
> > Source/bmalloc/bmalloc/Heap.cpp:54
> > +    auto queue = dispatch_queue_create("WebKit Malloc Memory Pressure Handler", DISPATCH_QUEUE_CONCURRENT);
> 
> It's a very minor point, but this should be DISPATCH_QUEUE_SERIAL, because
> we don't intend to handle multiple concurrent pressure events.

I made that change locally.

[Michael Saboff]

Created attachment 308164 [details]
Speculative build fix for EWS - added #include

[Michael Saboff]

Created attachment 308165 [details]
Another EWS build fix

[Michael Saboff]

Committed r215775: <http://trac.webkit.org/changeset/215775>