Bug 188867 - [WTF] Add generic implementation for Memory querying
Summary: [WTF] Add generic implementation for Memory querying
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Don Olmstead
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-22 15:27 PDT by Don Olmstead
Modified: 2018-08-23 12:48 PDT (History)
11 users (show)

See Also:


Attachments
Patch (11.75 KB, patch)
2018-08-22 15:40 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (11.75 KB, patch)
2018-08-22 15:45 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (10.47 KB, patch)
2018-08-22 16:43 PDT, Don Olmstead
Hironori.Fujii: review+
Details | Formatted Diff | Diff
Patch (10.47 KB, patch)
2018-08-22 19:13 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff
Build fix (1.19 KB, patch)
2018-08-23 11:04 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Don Olmstead 2018-08-22 15:27:57 PDT
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.
Comment 1 Don Olmstead 2018-08-22 15:40:21 PDT
Created attachment 347858 [details]
Patch
Comment 2 Don Olmstead 2018-08-22 15:45:14 PDT
Created attachment 347861 [details]
Patch
Comment 3 Don Olmstead 2018-08-22 16:43:45 PDT
Created attachment 347875 [details]
Patch
Comment 4 Fujii Hironori 2018-08-22 18:47:47 PDT
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"?
Comment 5 Don Olmstead 2018-08-22 18:57:55 PDT
(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.
Comment 6 Don Olmstead 2018-08-22 19:13:34 PDT
Created attachment 347893 [details]
Patch

Updated based on review comments
Comment 7 WebKit Commit Bot 2018-08-22 20:01:47 PDT
Comment on attachment 347893 [details]
Patch

Clearing flags on attachment: 347893

Committed r235215: <https://trac.webkit.org/changeset/235215>
Comment 8 WebKit Commit Bot 2018-08-22 20:01:49 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2018-08-22 20:02:26 PDT
<rdar://problem/43630726>
Comment 10 Don Olmstead 2018-08-23 11:01:45 PDT
A future change where there would be a generic bmalloc implementation snuck in the last patch. It needs to be removed.
Comment 11 Don Olmstead 2018-08-23 11:04:52 PDT
Created attachment 347938 [details]
Build fix
Comment 12 WebKit Commit Bot 2018-08-23 12:48:23 PDT
Comment on attachment 347938 [details]
Build fix

Clearing flags on attachment: 347938

Committed r235235: <https://trac.webkit.org/changeset/235235>
Comment 13 WebKit Commit Bot 2018-08-23 12:48:25 PDT
All reviewed patches have been landed.  Closing bug.