Bug 226083 - Don't hang onto expired resources without validation headers in memory cache
Summary: Don't hang onto expired resources without validation headers in memory cache
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-05-21 07:23 PDT by Antti Koivisto
Modified: 2021-05-26 12:19 PDT (History)
6 users (show)

See Also:

Attachments
patch (2.21 KB, patch)
2021-05-21 08:14 PDT, Antti Koivisto 		no flags Details | Formatted Diff | Diff
patch (2.21 KB, patch)
2021-05-21 08:18 PDT, Antti Koivisto 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (8.33 KB, patch)
2021-05-24 12:13 PDT, Antti Koivisto 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (8.41 KB, patch)
2021-05-25 05:19 PDT, Antti Koivisto 		no flags Details | Formatted Diff | Diff
patch (9.46 KB, patch)
2021-05-25 06:20 PDT, Antti Koivisto 		no flags Details | Formatted Diff | Diff
patch (8.77 KB, patch)
2021-05-25 07:36 PDT, Antti Koivisto 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (10.87 KB, patch)
2021-05-26 01:51 PDT, Antti Koivisto 		no flags Details | Formatted Diff | Diff
patch (10.29 KB, patch)
2021-05-26 01:53 PDT, Antti Koivisto 		no flags Details | Formatted Diff | Diff
patch (10.63 KB, patch)
2021-05-26 07:59 PDT, Antti Koivisto 		no flags Details | Formatted Diff | Diff
Show Obsolete (8) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2021-05-21 07:23:54 PDT 
They use memory while only being useful for history navigation. Disk cache can handle that.
Comment 1 Antti Koivisto 2021-05-21 07:44:42 PDT 
rdar://78035612
Comment 2 Antti Koivisto 2021-05-21 08:14:44 PDT 
Created attachment 429292 [details]
patch
Comment 3 Antti Koivisto 2021-05-21 08:18:35 PDT 
Created attachment 429293 [details]
patch
Comment 4 Antti Koivisto 2021-05-24 12:13:10 PDT 
Created attachment 429552 [details]
patch
Comment 5 Antti Koivisto 2021-05-25 05:19:02 PDT 
Created attachment 429647 [details]
patch
Comment 6 Antti Koivisto 2021-05-25 06:20:06 PDT 
Created attachment 429651 [details]
patch
Comment 7 Antti Koivisto 2021-05-25 07:36:26 PDT 
Created attachment 429653 [details]
patch
Comment 8 Antti Koivisto 2021-05-26 01:51:26 PDT 
Created attachment 429737 [details]
patch
Comment 9 Antti Koivisto 2021-05-26 01:53:19 PDT 
Created attachment 429738 [details]
patch
Comment 10 Antti Koivisto 2021-05-26 07:59:18 PDT 
Created attachment 429755 [details]
patch
Comment 11 Chris Dumez 2021-05-26 11:09:49 PDT 
Comment on attachment 429755 [details]
patch

r=me
Comment 12 Geoffrey Garen 2021-05-26 11:11:25 PDT 
Comment on attachment 429755 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=429755&action=review

> Source/WebCore/ChangeLog:29
> +        Remove expired resources without validation headers from the cache if the resource is otherwise deletable.

I guess this is the more conservative approach, so maybe it's best. But I wonder if there's any value to expired resources _with_ validation headers in the memory cache? In that case, we're going to do a network round trip to validate anyway, so maybe an IPC round trip to the network process is also fine. Long ago, Dave Hyatt told me that the real win of the memory cache was when it could respond synchronously in the current RunLoop iteration.
Comment 13 Antti Koivisto 2021-05-26 11:51:58 PDT 
Reusing a resource from memory cache may also save decoding for images, parsing for  stylesheets, text decoding etc. (though decoded versions are thrown out pretty fast anyway for unreferenced resources).

But yeah, it might be time to throw out the entire revalidation logic in memory cache. Would certainly simplify things. Part of its job used to be to cover things networking layer wasn't handling well.
Comment 14 Geoffrey Garen 2021-05-26 12:14:06 PDT 
> But yeah, it might be time to throw out the entire revalidation logic in
> memory cache. Would certainly simplify things. Part of its job used to be to
> cover things networking layer wasn't handling well.

I guess that's a topic for another bug. But might be worth trying out at some point.
Comment 15 EWS 2021-05-26 12:19:36 PDT 
Committed r278119 (238169@main): <https://commits.webkit.org/238169@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 429755 [details].