WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
201329
Allow process cache to cache processes when using a non-default persistent WebsiteDataStore
https://bugs.webkit.org/show_bug.cgi?id=201329
Summary
Allow process cache to cache processes when using a non-default persistent We...
Alex Christensen
Reported
2019-08-29 21:21:14 PDT
Allow process cache to cache processes when using a non-default persistent WebsiteDataStore
Attachments
Patch
(1.82 KB, patch)
2019-08-29 21:32 PDT
,
Alex Christensen
no flags
Details
Formatted Diff
Diff
Patch
(3.60 KB, patch)
2019-08-30 10:39 PDT
,
Alex Christensen
cdumez
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Alex Christensen
Comment 1
2019-08-29 21:32:53 PDT
Created
attachment 377675
[details]
Patch
Chris Dumez
Comment 2
2019-08-30 09:29:51 PDT
Comment on
attachment 377675
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=377675&action=review
> Source/WebKit/UIProcess/WebProcessCache.cpp:69 > + if (sessionID.isEphemeral() && !process.processPool().hasPagesUsingWebsiteDataStore(process.websiteDataStore())) {
I am not sure we want to change this behavior. It currently matches the logic WebProcessPool::pageBeginUsingWebsiteDataStore() / WebProcessPool::pageEndUsingWebsiteDataStore(). Once there is no longer any page using a page using a non-default session, we remove the session from the network process. Once this is done, why do we want to keep caching WebContent processes for that session? Your bug title claims to allow caching for WebContent processes for non-default persistent WebsiteDataStores. However, they are already cached as long as there are pages using the WebsiteDataStore in question. What your patch does is allow to cache such processes *after* that session has been removed in memory because there are no longer any pages using it. It is not clear to be we want to do this.
Alex Christensen
Comment 3
2019-08-30 09:39:47 PDT
I think I agree, but there's definitely something strange going on. Could you try with the PR attached to
rdar://problem/47030981
? We're also removing sessions from the NetworkProcess when I don't think we should be, exposing
https://bugs.webkit.org/show_bug.cgi?id=201324
. There's still some implicit assumption that the only persistent session is the default session. I think it's related to a combination of WebProcessPool::pageBeginUsingWebsiteDataStore and the use of nonDefaultDataStores()
Chris Dumez
Comment 4
2019-08-30 09:44:08 PDT
> We're also removing sessions from the NetworkProcess when I don't think we should be.
Do you mean : 1. that you seem non-default persistent sessions getting destroyed even though there are still pages using it? or 2. that we should keep non-default persistent sessions in memory even when they no longer have no pages, similarly to what we do for the default session ?
Chris Dumez
Comment 5
2019-08-30 09:46:42 PDT
(In reply to Chris Dumez from
comment #4
)
> > We're also removing sessions from the NetworkProcess when I don't think we should be. > > Do you mean : > 1. that you seem non-default persistent sessions getting destroyed even > though there are still pages using it? > or > 2. that we should keep non-default persistent sessions in memory even when > they no longer have no pages, similarly to what we do for the default session > > ?
If we made it so that non-default persistent session stay in memory (i.e. update WebProcessPool::pageBeginUsingWebsiteDataStore() / WebProcessPool::pageEndUsingWebsiteDataStore(), so option 2) then I think the change you're suggesting would then make sense. And maybe it would perf in the scenario you're considering.
Alex Christensen
Comment 6
2019-08-30 09:50:29 PDT
Our use of nonDefaultDataStores() isn't related to the perf scenario I'm looking at. I think option 2 might be what we need. I'll look into it.
Alex Christensen
Comment 7
2019-08-30 10:39:01 PDT
Created
attachment 377724
[details]
Patch
Alex Christensen
Comment 8
2019-08-30 13:24:13 PDT
http://trac.webkit.org/r249341
Radar WebKit Bug Importer
Comment 9
2019-08-30 13:25:22 PDT
<
rdar://problem/54894208
>
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