Bug 188091 - Use SPI to compute the jetsam limit on iOS instead of hardcoding 840MB
Summary: Use SPI to compute the jetsam limit on iOS instead of hardcoding 840MB
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: bmalloc (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-07-26 21:23 PDT by Saam Barati
Modified: 2018-07-27 14:08 PDT (History)
11 users (show)

See Also:


Attachments
patch (5.87 KB, patch)
2018-07-26 23:51 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
patch (5.19 KB, patch)
2018-07-26 23:53 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
patch (5.20 KB, patch)
2018-07-26 23:55 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
patch (5.21 KB, patch)
2018-07-26 23:56 PDT, Saam Barati
simon.fraser: review+
Details | Formatted Diff | Diff
patch for landing (10.63 KB, patch)
2018-07-27 12:08 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
patch for landing (10.66 KB, patch)
2018-07-27 12:49 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2018-07-26 21:23:30 PDT
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.
Comment 1 Saam Barati 2018-07-26 21:31:14 PDT
<rdar://problem/42647697>
Comment 2 Saam Barati 2018-07-26 23:51:27 PDT
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.
Comment 3 EWS Watchlist 2018-07-26 23:53:11 PDT
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.
Comment 4 Saam Barati 2018-07-26 23:53:28 PDT
Created attachment 345904 [details]
patch

without the syslog this time
Comment 5 Saam Barati 2018-07-26 23:55:13 PDT
Created attachment 345905 [details]
patch
Comment 6 Saam Barati 2018-07-26 23:56:05 PDT
Created attachment 345906 [details]
patch
Comment 7 EWS Watchlist 2018-07-26 23:58:26 PDT
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.
Comment 8 Simon Fraser (smfr) 2018-07-27 10:41:54 PDT
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?
Comment 9 Saam Barati 2018-07-27 11:18:46 PDT
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.
Comment 10 Saam Barati 2018-07-27 12:08:52 PDT
Created attachment 345932 [details]
patch for landing
Comment 11 EWS Watchlist 2018-07-27 12:10:49 PDT
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.
Comment 12 Saam Barati 2018-07-27 12:49:40 PDT
Created attachment 345940 [details]
patch for landing
Comment 13 EWS Watchlist 2018-07-27 12:52:16 PDT
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.
Comment 14 WebKit Commit Bot 2018-07-27 13:35:12 PDT
Comment on attachment 345940 [details]
patch for landing

Clearing flags on attachment: 345940

Committed r234326: <https://trac.webkit.org/changeset/234326>
Comment 15 WebKit Commit Bot 2018-07-27 13:35:14 PDT
All reviewed patches have been landed.  Closing bug.
Comment 16 Joseph Pecoraro 2018-07-27 14:01:32 PDT
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?
Comment 17 Saam Barati 2018-07-27 14:08:32 PDT
(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)