Bug 188091 – Use SPI to compute the jetsam limit on iOS instead of hardcoding 840MB

• Component: bmalloc
• Status: RESOLVED FIXED

[Saam Barati]

This requires adopting an entitlement on iOS for the WebContent (and other) processes. We'll fall back to 840 (the current behavior) when the SPI fails due to the lack of an entitlement.

[Saam Barati]

<rdar://problem/42647697>

[Saam Barati]

Created attachment 345903 [details]
patch

This patch doesn't add the entitlement for the minimal simulator build. We may want to add it if it works in such a build, so we don't end up with 840MB hardcoded on Mac.

[EWS Watchlist]

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


ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:55:  memlimit_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:56:  memlimit_active_attr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:57:  memlimit_inactive is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:58:  memlimit_inactive_attr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:67:  memorystatus_control is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:106:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:107:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:108:  Tab found; better to use spaces  [whitespace/tab] [1]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:108:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:109:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:111:  Missing spaces around /  [whitespace/operators] [3]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:128:  Missing spaces around /  [whitespace/operators] [3]
Total errors found: 12 in 6 files


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

[Saam Barati]

Created attachment 345904 [details]
patch

without the syslog this time

[Saam Barati]

Created attachment 345905 [details]
patch

[Saam Barati]

Created attachment 345906 [details]
patch

[EWS Watchlist]

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


ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:55:  memlimit_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:56:  memlimit_active_attr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:57:  memlimit_inactive is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:58:  memlimit_inactive_attr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/AvailableMemory.cpp:67:  memorystatus_control is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 6 files


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

[Simon Fraser (smfr)]

Comment on attachment 345906 [details]
patch

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

> Source/bmalloc/bmalloc/AvailableMemory.cpp:70
> +#if BPLATFORM(IOS)
> +#if __has_include(<System/sys/kern_memorystatus.h>)
> +extern "C" {
> +#include <System/sys/kern_memorystatus.h>
> +}
> +#else
> +extern "C" {
> +
> +typedef struct memorystatus_memlimit_properties {
> +    int32_t memlimit_active;                /* jetsam memory limit (in MB) when process is active */
> +    uint32_t memlimit_active_attr;
> +    int32_t memlimit_inactive;              /* jetsam memory limit (in MB) when process is inactive */
> +    uint32_t memlimit_inactive_attr;
> +} memorystatus_memlimit_properties_t;
> +
> +#define MEMORYSTATUS_CMD_GET_MEMLIMIT_PROPERTIES 8
> +
> +}
> +#endif // __has_include(<System/sys/kern_memorystatus.h>)
> +
> +extern "C" {
> +int memorystatus_control(uint32_t command, int32_t pid, uint32_t flags, void *buffer, size_t buffersize);
> +}
> +
> +#endif // BPLATFORM(IOS)

Should we introduce the concept of SPI headers in bmalloc?

[Saam Barati]

Comment on attachment 345906 [details]
patch

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

>> Source/bmalloc/bmalloc/AvailableMemory.cpp:70
>> +#endif // BPLATFORM(IOS)
> 
> Should we introduce the concept of SPI headers in bmalloc?

Yeah that'd probably be cleaner.

[Saam Barati]

Created attachment 345932 [details]
patch for landing

[EWS Watchlist]

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


ERROR: Source/bmalloc/bmalloc/darwin/MemoryStatusSPI.h:39:  memlimit_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/darwin/MemoryStatusSPI.h:40:  memlimit_active_attr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/darwin/MemoryStatusSPI.h:41:  memlimit_inactive is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/darwin/MemoryStatusSPI.h:42:  memlimit_inactive_attr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/darwin/MemoryStatusSPI.h:51:  memorystatus_control is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 8 files


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

[Saam Barati]

Created attachment 345940 [details]
patch for landing

[EWS Watchlist]

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


ERROR: Source/bmalloc/bmalloc/darwin/MemoryStatusSPI.h:39:  memlimit_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/darwin/MemoryStatusSPI.h:40:  memlimit_active_attr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/darwin/MemoryStatusSPI.h:41:  memlimit_inactive is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/darwin/MemoryStatusSPI.h:42:  memlimit_inactive_attr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/darwin/MemoryStatusSPI.h:51:  memorystatus_control is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 8 files


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

[WebKit Commit Bot]

Comment on attachment 345940 [details]
patch for landing

Clearing flags on attachment: 345940

Committed r234326: <https://trac.webkit.org/changeset/234326>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

Comment on attachment 345940 [details]
patch for landing

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

> Source/bmalloc/bmalloc/AvailableMemory.cpp:85
> +    return static_cast<size_t>(properties.memlimit_active) * bmalloc::MB;

What if memlimit_active is -1, which may happen during development. Might it be better to just convert -1 to max<size_t> or some max value of our choice?

[Saam Barati]

(In reply to Joseph Pecoraro from comment #16)
> Comment on attachment 345940 [details]
> patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=345940&action=review
> 
> > Source/bmalloc/bmalloc/AvailableMemory.cpp:85
> > +    return static_cast<size_t>(properties.memlimit_active) * bmalloc::MB;
> 
> What if memlimit_active is -1, which may happen during development. Might it
> be better to just convert -1 to max<size_t> or some max value of our choice?

I'll make a patch to explicitly handle it (though in practice this means we currently size_t max)