Bug 130926 - [iOS WebKit2] Disable tile cohort retention for now
Summary: [iOS WebKit2] Disable tile cohort retention for now
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-03-29 05:12 PDT by Tim Horton
Modified: 2014-03-31 12:38 PDT (History)
9 users (show)

See Also:


Attachments
patch (22.28 KB, patch)
2014-03-29 05:23 PDT, Tim Horton
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2014-03-29 05:12:23 PDT
At the moment, we retain unparented cohorts of tiles for four-ish seconds after they leave the tile coverage rect.

However, since the remote layer tree code doesn't support purgeable unparented tiles, this means that scrolling quickly leads to a backing store explosion with no recourse.

We should disable tile cohort retention for now (I have confirmed that it hugely improves performance/reduces process death when scrolling quickly). We will enable aggressive tile retention (like Mac uses) once we have purgeable unparented tile support.
Comment 1 Radar WebKit Bug Importer 2014-03-29 05:12:46 PDT
<rdar://problem/16465413>
Comment 2 Tim Horton 2014-03-29 05:23:54 PDT
Created attachment 228106 [details]
patch
Comment 3 Tim Horton 2014-03-29 15:33:57 PDT
(In reply to comment #0)
> ...retain unparented cohorts of tiles for four-ish seconds after they leave...

two-ish actually
Comment 4 Maciej Stachowiak 2014-03-31 11:55:03 PDT
Comment on attachment 228106 [details]
patch

I thought this would be just  prefs change but there are a bunch of structural changes that I don't entirely follow. If I'm understanding this correctly, what this patch actually seems to do is introduce a new setting that retains tile cohorts temporarily instead of indefinitely. Is that correct? I'm not sure "disable tile cohort retention for now" is the clearest way to express that.
Comment 5 Tim Horton 2014-03-31 11:59:36 PDT
(In reply to comment #4)
> (From update of attachment 228106 [details])
> I thought this would be just  prefs change but there are a bunch of structural changes that I don't entirely follow. If I'm understanding this correctly, what this patch actually seems to do is introduce a new setting that retains tile cohorts temporarily instead of indefinitely. Is that correct? I'm not sure "disable tile cohort retention for now" is the clearest way to express that.

No, it adds a new setting (which previously was not a setting, just was always true) to enable temporary retention, and turns that setting *off* on iOS. Thus, disabling retention of cohorts.

There has long been a setting for indefinite ("aggressive") retention, and that one is off by default as well (but gets turned on on Mac by clients).
Comment 6 Tim Horton 2014-03-31 12:38:05 PDT
http://trac.webkit.org/changeset/166516