WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Proposed patch
(13.80 KB, patch)
2017-05-18 12:07 PDT
,
Andreas Kling
no flags
Details
Formatted Diff
Diff
Proposed patch
(13.78 KB, patch)
2017-05-18 15:23 PDT
,
Andreas Kling
ggaren
: review+
Details
Formatted Diff
Diff
Patch for landing
(13.78 KB, patch)
2017-05-18 15:33 PDT
,
Andreas Kling
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
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.
Top of Page
Format For Printing
XML
Clone This Bug