Bug 171944 – [iOS] Use memory footprint to dynamically adjust behavior of allocators

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

[Michael Saboff]

On iOS, there is a soft memory limit of 840MB for WebContent processes.  When we get close to or above that limit and don’t reduce memory usage it is likely the OS will jetsam the process.  The OS provides memory pressure notifications, but that mechanism is course and may not provide a timely way to modify behavior.

It makes more sense to modify our allocation policies and return free memory back to the OS more agressively as we approach our memory limit.

<rdar://problem/32116376>

[Michael Saboff]

Created attachment 309640 [details]
Almost done patch for review

My only question is how to layer bmalloc and WTF.   Currently bmalloc uses ramSize() from WTF, but I could be convinced to move ramSize to bmalloc.  I'm open to other ideas as well.

[Filip Pizlo]

Comment on attachment 309640 [details]
Almost done patch for review

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

This looks good to me!

> Source/JavaScriptCore/heap/Heap.cpp:117
> +    size_t memoryFootprint = bmalloc::api::memoryFootprint();

Would be cool if bmalloc::api::memoryFootprint was abstracted in WTF behind fastMallocMemoryFootprint, or something.

Not super important.

> Source/bmalloc/bmalloc/Heap.cpp:45
> +#if BPLATFORM(IOS)
> +namespace WTF {
> +
> +size_t ramSize();
> +
> +}
> +#endif

Would be a lot better if you moved WTF::ramSize into bmalloc, and then WTF::ramSize would be a forwarding function that calls bmalloc::api::ramSize or whatever.

[Michael Saboff]

Created attachment 309728 [details]
Updated patch - should fix build issues.

(In reply to Filip Pizlo from comment #2)
> Comment on attachment 309640 [details]
> Almost done patch for review
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=309640&action=review
> 
> This looks good to me!
> 
> > Source/JavaScriptCore/heap/Heap.cpp:117
> > +    size_t memoryFootprint = bmalloc::api::memoryFootprint();
> 
> Would be cool if bmalloc::api::memoryFootprint was abstracted in WTF behind
> fastMallocMemoryFootprint, or something.
> 
> Not super important.

I'll do this is in a followup patch.

> > Source/bmalloc/bmalloc/Heap.cpp:45
> > +#if BPLATFORM(IOS)
> > +namespace WTF {
> > +
> > +size_t ramSize();
> > +
> > +}
> > +#endif
> 
> Would be a lot better if you moved WTF::ramSize into bmalloc, and then
> WTF::ramSize would be a forwarding function that calls bmalloc::api::ramSize
> or whatever.

Done.

[Build Bot]

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


ERROR: Source/bmalloc/bmalloc/Heap.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/darwin/AvailableMemory.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 2 in 15 files


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

[Michael Saboff]

Created attachment 309731 [details]
Patch with another build fix

[Build Bot]

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


ERROR: Source/bmalloc/bmalloc/Heap.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/darwin/AvailableMemory.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 2 in 15 files


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

[Michael Saboff]

Created attachment 309747 [details]
Another build fix for EWS to try

[Build Bot]

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


ERROR: Source/bmalloc/bmalloc/Heap.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/darwin/AvailableMemory.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 2 in 16 files


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

[Michael Saboff]

Created attachment 309752 [details]
Another build fix.  Moved AvailableMemory files to stdlib and added the .cpp to CMakeLists.txt

[Build Bot]

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


ERROR: Source/bmalloc/bmalloc/Heap.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/darwin/AvailableMemory.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 2 in 17 files


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

[Michael Saboff]

Created attachment 309804 [details]
Hopefully last EWS patch - Moved AvailableMemory.{cpp, h} into main bmalloc directory.

[Build Bot]

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


ERROR: Source/bmalloc/bmalloc/Heap.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 2 in 17 files


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

[Michael Saboff]

Created attachment 309807 [details]
Another speculative GTK build fix.  Eliminated Darwin conditional inclusion of AvailableMemory.h in bmalloc.h

[Build Bot]

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


ERROR: Source/bmalloc/bmalloc/Heap.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/bmalloc.h:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 3 in 17 files


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

[Michael Saboff]

Created attachment 309811 [details]
Speculative GTK build fix

[Build Bot]

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


ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 1 in 17 files


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

[Michael Saboff]

Created attachment 309823 [details]
Another speculative GTK build fix.  Eliminated Darwin conditional inclusion of AvailableMemory.h in bmalloc.h

[Build Bot]

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


ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 1 in 17 files


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

[Build Bot]

Comment on attachment 309823 [details]
Another speculative GTK build fix.  Eliminated Darwin conditional inclusion of AvailableMemory.h in bmalloc.h

Attachment 309823 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3722299

New failing tests:
fast/workers/dedicated-worker-lifecycle.html

[Build Bot]

Created attachment 309844 [details]
Archive of layout-test-results from ews113 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Mark Lam]

(In reply to Build Bot from comment #19)
> New failing tests:
> fast/workers/dedicated-worker-lifecycle.html

FYI, this is not your failure.

[Michael Saboff]

Committed r216763: <http://trac.webkit.org/changeset/216763>

[Geoffrey Garen]

This looks like a 1% regression on the MBP and MBA JetStream bots.

I suspect the change to speed up the scavenger is the cause, since the rest of this code shouldn't run on Mac.

[Michael Saboff]

(In reply to Geoffrey Garen from comment #23)
> This looks like a 1% regression on the MBP and MBA JetStream bots.
> 
> I suspect the change to speed up the scavenger is the cause, since the rest
> of this code shouldn't run on Mac.

In addition to the scavenger delay being reduced to 250ms, the scavenger thread QOS was changed from QOS_CLASS_UNSPECIFIED to QOS_CLASS_USER_INITIATED.  I agree though that the likely reason is cutting the delay in half.  I'll check locally and post a followup patch.  Tracked in <https://bugs.webkit.org/show_bug.cgi?id=172124>