Bug 188867

Summary: [WTF] Add generic implementation for Memory querying
Product: WebKit Reporter: Don Olmstead <don.olmstead>
Component: Web Template FrameworkAssignee: Don Olmstead <don.olmstead>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, benjamin, cdumez, cmarcelo, commit-queue, dbates, ews-watchlist, fujii.hironori, mcatanzaro, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
fujii.hironori: review+
Patch
none
Build fix none

Don Olmstead
Reported 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.
Attachments
Patch (11.75 KB, patch)
2018-08-22 15:40 PDT, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Patch (11.75 KB, patch)
2018-08-22 15:45 PDT, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Patch (10.47 KB, patch)
2018-08-22 16:43 PDT, Don Olmstead 		fujii.hironori: review+
DetailsFormatted DiffDiff
Patch (10.47 KB, patch)
2018-08-22 19:13 PDT, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Build fix (1.19 KB, patch)
2018-08-23 11:04 PDT, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Don Olmstead
Comment 1 2018-08-22 15:40:21 PDT
Created attachment 347858 [details] Patch
Don Olmstead
Comment 2 2018-08-22 15:45:14 PDT
Created attachment 347861 [details] Patch
Don Olmstead
Comment 3 2018-08-22 16:43:45 PDT
Created attachment 347875 [details] Patch
Fujii Hironori
Comment 4 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"?
Don Olmstead
Comment 5 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.
Don Olmstead
Comment 6 2018-08-22 19:13:34 PDT
Created attachment 347893 [details] Patch Updated based on review comments
WebKit Commit Bot
Comment 7 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>
WebKit Commit Bot
Comment 8 2018-08-22 20:01:49 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9 2018-08-22 20:02:26 PDT
<rdar://problem/43630726>
Don Olmstead
Comment 10 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.
Don Olmstead
Comment 11 2018-08-23 11:04:52 PDT
Created attachment 347938 [details] Build fix
WebKit Commit Bot
Comment 12 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>
WebKit Commit Bot
Comment 13 2018-08-23 12:48:25 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.