RESOLVED FIXED 172244
[WK2] Notify WebPageProxy client when an active process goes over the inactive memory limit
https://bugs.webkit.org/show_bug.cgi?id=172244
Summary [WK2] Notify WebPageProxy client when an active process goes over the inactiv...
Andreas Kling
Reported 2017-05-17 14:35:46 PDT
We should let the UI-side WK2 client know when the active process comes under memory pressure. <rdar://problem/31800943>
Attachments
Proposed patch (11.39 KB, patch)
2017-05-17 14:38 PDT, Andreas Kling
no flags
Proposed patch (13.80 KB, patch)
2017-05-18 12:07 PDT, Andreas Kling
no flags
Proposed patch (13.78 KB, patch)
2017-05-18 15:23 PDT, Andreas Kling
ggaren: review+
Patch for landing (13.78 KB, patch)
2017-05-18 15:33 PDT, Andreas Kling
no flags
Andreas Kling
Comment 1 2017-05-17 14:38:42 PDT
Created attachment 310446 [details] Proposed patch Ok here's a cut, pls roast.
Chris Dumez
Comment 2 2017-05-17 14:56:20 PDT
Comment on attachment 310446 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=310446&action=review > Source/WebKit2/UIProcess/API/C/WKPageUIClient.h:930 > + WKPageDispatchMemoryPressureNotificationCallback dispatchMemoryPressureNotification; When I discussed this with Geoff yesterday, my understanding was that the callback would be something like: didExceededBackgroundResourceLimitWhileInForeground(ResourceLimit); enum class ResourceLimit { Memory, CPU }; Otherwise, we'll end up with 2 separate callback for memory and CPU.
Andreas Kling
Comment 3 2017-05-17 14:57:43 PDT
(In reply to Chris Dumez from comment #2) > Comment on attachment 310446 [details] > Proposed patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=310446&action=review > > > Source/WebKit2/UIProcess/API/C/WKPageUIClient.h:930 > > + WKPageDispatchMemoryPressureNotificationCallback dispatchMemoryPressureNotification; > > When I discussed this with Geoff yesterday, my understanding was that the > callback would be something like: > didExceededBackgroundResourceLimitWhileInForeground(ResourceLimit); > enum class ResourceLimit { Memory, CPU }; > > Otherwise, we'll end up with 2 separate callback for memory and CPU. Oh, okay, that sounds p good too. I can certainly make it look like that instead!
Chris Dumez
Comment 4 2017-05-17 14:58:27 PDT
Comment on attachment 310446 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=310446&action=review >> Source/WebKit2/UIProcess/API/C/WKPageUIClient.h:930 >> + WKPageDispatchMemoryPressureNotificationCallback dispatchMemoryPressureNotification; > > When I discussed this with Geoff yesterday, my understanding was that the callback would be something like: > didExceededBackgroundResourceLimitWhileInForeground(ResourceLimit); > enum class ResourceLimit { Memory, CPU }; > > Otherwise, we'll end up with 2 separate callback for memory and CPU. Also, does this patch rely on memory pressure notifications from the system rather than using our inactive memory limit?
Andreas Kling
Comment 5 2017-05-18 09:38:25 PDT
(In reply to Chris Dumez from comment #4) > Comment on attachment 310446 [details] > Proposed patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=310446&action=review > > >> Source/WebKit2/UIProcess/API/C/WKPageUIClient.h:930 > >> + WKPageDispatchMemoryPressureNotificationCallback dispatchMemoryPressureNotification; > > > > When I discussed this with Geoff yesterday, my understanding was that the callback would be something like: > > didExceededBackgroundResourceLimitWhileInForeground(ResourceLimit); > > enum class ResourceLimit { Memory, CPU }; > > > > Otherwise, we'll end up with 2 separate callback for memory and CPU. > > Also, does this patch rely on memory pressure notifications from the system > rather than using our inactive memory limit? No, but this patch had some architectural mistakes so I'm redoing it to fit your suggested API name (which I love)
Andreas Kling
Comment 6 2017-05-18 12:07:03 PDT
Created attachment 310527 [details] Proposed patch New version with Chris's suggested API using ResourceLimit { Memory, CPU } enum.
Andreas Kling
Comment 7 2017-05-18 15:23:02 PDT
Created attachment 310556 [details] Proposed patch
Geoffrey Garen
Comment 8 2017-05-18 15:25:59 PDT
Comment on attachment 310556 [details] Proposed patch r=me
Andreas Kling
Comment 9 2017-05-18 15:33:48 PDT
Created attachment 310559 [details] Patch for landing
WebKit Commit Bot
Comment 10 2017-05-18 23:54:32 PDT
Comment on attachment 310559 [details] Patch for landing Clearing flags on attachment: 310559 Committed r217101: <http://trac.webkit.org/changeset/217101>
WebKit Commit Bot
Comment 11 2017-05-18 23:54:34 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.