Bug 188091

<rdar://problem/42647697>

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.

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.

Created attachment 345904 [details]
patch

without the syslog this time

Created attachment 345905 [details]
patch

Created attachment 345906 [details]
patch

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 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 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.

Created attachment 345932 [details]
patch for landing

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.

Created attachment 345940 [details]
patch for landing

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 on attachment 345940 [details]
patch for landing

Clearing flags on attachment: 345940

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

All reviewed patches have been landed.  Closing bug.

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?

(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)