Added FreeBSD implementation for memoryFootprint() implementation and share Linux's MemoryPressureHandler among Unix ports.
Created attachment 393305 [details] WIP To see this works for other ports.
Created attachment 393310 [details] PATCH
Comment on attachment 393310 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=393310&action=review > Source/WTF/wtf/unix/MemoryPressureHandlerUnix.cppSource/WTF/wtf/linux/MemoryPressureHandlerLinux.cpp:133 > + return static_cast<size_t>(info.ki_rssize - info.ki_tsize) * pageSize; I wonder if we really need to duplicate this implementations between WTF and bmalloc. Couldn't we just call bmalloc/AvailableMemory.h stuff from WTF if bmalloc is enabled, and build just that part of bmalloc and call it if USE(SYSTEM_MALLOC) is enabled?
Comment on attachment 393310 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=393310&action=review > Source/WTF/wtf/PlatformGTK.cmake:49 > list(APPEND WTF_SOURCES > linux/CurrentProcessMemoryStatus.cpp > linux/MemoryFootprintLinux.cpp > - linux/MemoryPressureHandlerLinux.cpp > + > + unix/MemoryPressureHandlerUnix.cpp This file should now be added inside a different if condition. Something like "if (CMAKE_SYSTEM_NAME MATCHES "Linux") OR (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")" > Source/WTF/wtf/PlatformJSCOnly.cmake:86 > list(APPEND WTF_SOURCES > linux/CurrentProcessMemoryStatus.cpp > linux/MemoryFootprintLinux.cpp > - linux/MemoryPressureHandlerLinux.cpp > + > + unix/MemoryPressureHandlerUnix.cpp Ditto > Source/WTF/wtf/unix/MemoryPressureHandlerUnix.cppSource/WTF/wtf/linux/MemoryPressureHandlerLinux.cpp:139 > void MemoryPressureHandler::respondToMemoryPressure(Critical critical, Synchronous synchronous) On Linux we have a thread on the UIProcess polling memory free and sending memoryPressureEvents (via WebKit IPC) to all the webprocess and networprocess. See MemoryPressureMonitor::start() at Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp What its calling MemoryPressureHandler::triggerMemoryPressureEvent() and MemoryPressureHandler::respondToMemoryPressure() on FreeBSD?
Comment on attachment 393310 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=393310&action=review >> Source/WTF/wtf/PlatformGTK.cmake:49 >> + unix/MemoryPressureHandlerUnix.cpp > > This file should now be added inside a different if condition. > Something like "if (CMAKE_SYSTEM_NAME MATCHES "Linux") OR (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")" Alright. >> Source/WTF/wtf/unix/MemoryPressureHandlerUnix.cppSource/WTF/wtf/linux/MemoryPressureHandlerLinux.cpp:133 >> + return static_cast<size_t>(info.ki_rssize - info.ki_tsize) * pageSize; > > I wonder if we really need to duplicate this implementations between WTF and bmalloc. Couldn't we just call bmalloc/AvailableMemory.h stuff from WTF if bmalloc is enabled, and build just that part of bmalloc and call it if USE(SYSTEM_MALLOC) is enabled? That's a good point. But isn't that a little bit confusing? I'm now porting bmalloc for Windows. Once that's done, no port is required USE_SYSTEN_MALLOC and we can remove that from WTF. DebugHeap of bmalloc uses system's malloc so we still can have backup for switch back to native malloc implementation. Until then, can we keep this duplication? >> Source/WTF/wtf/unix/MemoryPressureHandlerUnix.cppSource/WTF/wtf/linux/MemoryPressureHandlerLinux.cpp:139 >> void MemoryPressureHandler::respondToMemoryPressure(Critical critical, Synchronous synchronous) > > On Linux we have a thread on the UIProcess polling memory free and sending memoryPressureEvents (via WebKit IPC) to all the webprocess and networprocess. See MemoryPressureMonitor::start() at Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp > What its calling MemoryPressureHandler::triggerMemoryPressureEvent() and MemoryPressureHandler::respondToMemoryPressure() on FreeBSD? That's still under a plan. We are trying some cases and currently we only enable setShouldUsePeriodicMemoryMonitor() from WebProcess and this respondToMemoryPressure will be called from TestRunners.
Created attachment 398892 [details] PATCH
Comment on attachment 393310 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=393310&action=review >>> Source/WTF/wtf/unix/MemoryPressureHandlerUnix.cppSource/WTF/wtf/linux/MemoryPressureHandlerLinux.cpp:133 >>> + return static_cast<size_t>(info.ki_rssize - info.ki_tsize) * pageSize; >> >> I wonder if we really need to duplicate this implementations between WTF and bmalloc. Couldn't we just call bmalloc/AvailableMemory.h stuff from WTF if bmalloc is enabled, and build just that part of bmalloc and call it if USE(SYSTEM_MALLOC) is enabled? > > That's a good point. But isn't that a little bit confusing? > > I'm now porting bmalloc for Windows. Once that's done, no port is required USE_SYSTEN_MALLOC and we can remove that from WTF. DebugHeap of bmalloc uses system's malloc so we still can have backup for switch back to native malloc implementation. Until then, can we keep this duplication? I personally think that we should have some layer under the bmalloc and always enable it, like, low-level library (LLL) or something like that, and we should move threading etc. to that, but for now, I think just duplicating is better. Conditionally building part of bmalloc makes bmalloc development complicated compared to introducing new low-level library layer.
Comment on attachment 398892 [details] PATCH r=me
(In reply to Yusuke Suzuki from comment #7) > Comment on attachment 393310 [details] > PATCH > > View in context: > https://bugs.webkit.org/attachment.cgi?id=393310&action=review > > >>> Source/WTF/wtf/unix/MemoryPressureHandlerUnix.cppSource/WTF/wtf/linux/MemoryPressureHandlerLinux.cpp:133 > >>> + return static_cast<size_t>(info.ki_rssize - info.ki_tsize) * pageSize; > >> > >> I wonder if we really need to duplicate this implementations between WTF and bmalloc. Couldn't we just call bmalloc/AvailableMemory.h stuff from WTF if bmalloc is enabled, and build just that part of bmalloc and call it if USE(SYSTEM_MALLOC) is enabled? > > > > That's a good point. But isn't that a little bit confusing? > > > > I'm now porting bmalloc for Windows. Once that's done, no port is required USE_SYSTEN_MALLOC and we can remove that from WTF. DebugHeap of bmalloc uses system's malloc so we still can have backup for switch back to native malloc implementation. Until then, can we keep this duplication? > > I personally think that we should have some layer under the bmalloc and > always enable it, like, low-level library (LLL) or something like that, and > we should move threading etc. to that, > but for now, I think just duplicating is better. Conditionally building part > of bmalloc makes bmalloc development complicated compared to introducing new > low-level library layer. Yeah, I really love that idea. Memory, process, thread, socket, etc. Thanks for r+!
Created attachment 398906 [details] PATCH
Committed r261428: <https://trac.webkit.org/changeset/261428> All reviewed patches have been landed. Closing bug and clearing flags on attachment 398906 [details].
<rdar://problem/63044143>