Bug 188867 – [WTF] Add generic implementation for Memory querying

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Don Olmstead]

Currently there is a compilation path for a generic implementation of MemoryPressureHandler and MemoryFootprint but they're in multiple places. They should be split into MemoryPressureHandlerGeneric and MemoryFootprintGeneric.

[Don Olmstead]

Created attachment 347858 [details]
Patch

[Don Olmstead]

Created attachment 347861 [details]
Patch

[Don Olmstead]

Created attachment 347875 [details]
Patch

[Fujii Hironori]

Comment on attachment 347875 [details]
Patch

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

> Source/WTF/wtf/PlatformJSCOnly.cmake:59
> +elseif (CMAKE_SYSTEM_NAME MATCHES ".*Linux")

You don't need '.*'.

https://github.com/WebKit/webkit/blob/master/Source/cmake/OptionsGTK.cmake#L113

> if (CMAKE_SYSTEM_NAME MATCHES "Linux")

BTW, the following code just use STREQUAL.

https://github.com/WebKit/webkit/blob/master/Source/cmake/OptionsCommon.cmake#L22

> ${CMAKE_SYSTEM_NAME} STREQUAL "Linux"

How does the variable CMAKE_SYSTEM_NAME look like? Can it be "FooBar Linux"?

[Don Olmstead]

(In reply to Fujii Hironori from comment #4)
> Comment on attachment 347875 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=347875&action=review
> 
> > Source/WTF/wtf/PlatformJSCOnly.cmake:59
> > +elseif (CMAKE_SYSTEM_NAME MATCHES ".*Linux")
> 
> You don't need '.*'.
> 
> https://github.com/WebKit/webkit/blob/master/Source/cmake/OptionsGTK.
> cmake#L113
> 
> > if (CMAKE_SYSTEM_NAME MATCHES "Linux")
> 
> BTW, the following code just use STREQUAL.
> 
> https://github.com/WebKit/webkit/blob/master/Source/cmake/OptionsCommon.
> cmake#L22
> 
> > ${CMAKE_SYSTEM_NAME} STREQUAL "Linux"
> 
> How does the variable CMAKE_SYSTEM_NAME look like? Can it be "FooBar Linux"?

From https://github.com/WebKit/webkit/blob/master/Source/ThirdParty/openvr/CMakeLists.txt#L23

> if(CMAKE_SYSTEM_NAME MATCHES ".*Linux")

https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_NAME.html doesn't explicitly state that it is just Linux. and it sounds like you could technically set it when cross compiling to whatever you like.

Since its JSCOnly its likely that there are out of trunk ports that might set something weird, but I don't have a strong opinion here.

[Don Olmstead]

Created attachment 347893 [details]
Patch

Updated based on review comments

[WebKit Commit Bot]

Comment on attachment 347893 [details]
Patch

Clearing flags on attachment: 347893

Committed r235215: <https://trac.webkit.org/changeset/235215>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/43630726>

[Don Olmstead]

A future change where there would be a generic bmalloc implementation snuck in the last patch. It needs to be removed.

[Don Olmstead]

Created attachment 347938 [details]
Build fix

[WebKit Commit Bot]

Comment on attachment 347938 [details]
Build fix

Clearing flags on attachment: 347938

Committed r235235: <https://trac.webkit.org/changeset/235235>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.