1866240 - Maintain usage information in the database for a massive speedup at startup when QuotaManager needs to initialize an origin; especially beneficial for pinned tabs

Status: RESOLVED FIXED

(Core :: Storage: Cache API, task, P1)

Description

[Jan Varga [:janv]]

CacheQuotaClient::InitOrigin currently must get file size information for all stored responses. This can be extremely slow when there are thousands of files. The usage information could be maintained in the database instead.

Comment 1

[Jan Varga [:janv]]

Attachment: Bug 1866240 - Maintain usage information in the database; r=#dom-storage

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0279020821f5
Maintain usage information in the database; r=dom-storage-reviewers,asuth

Comment 3

[Cristian Tuns]

Backed out for causing multiple failures in test_cache_padding.html

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | dom/cache/test/mochitest/test_cache_padding.html | The opaque response should have larger size than the normal response.

And xpcshell failures: https://treeherder.mozilla.org/logviewer?job_id=438508244&repo=autoland

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d035302ec62
Maintain usage information in the database; r=dom-storage-reviewers,asuth

Comment 5

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/1d035302ec62

Comment 6

[Jan Varga [:janv]]

The telemetry for the time needed to load quota information has improved significantly after this fix. Should we consider an uplift to beta ?

Comment 7

[Jens Stutte [:jstutte]]

I leave it to Andrew to give a risk assessment for this. The patch looks non trivial, and I assume it changes even the DB schema?

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

(I changed my mind about setting a tracking flag here, but the autofilled text got lodged in here)

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

Attachment: Bug 1866240 - Maintain usage information in the database; r=#dom-storage

Original Revision: https://phabricator.services.mozilla.com/D195081

Comment 10

[Phabricator Automation]

Uplift Approval Request

• Explanation of risk level: We have high confidence in the code as the design is intentionally simple. As noted in the impact section, though, for this major startup time improvement we are paying a one-time upgrade cost per origin. Progress will be saved if the user kills the browser.
• Code covered by automated testing: yes
• Risk associated with taking this patch: Low-Med.
• Needs manual QE test: no
• User impact if declined: Slow browser startup / delayed usability for users that use sites like spotify or gmail that can store a lot of data in Cache API storage. When we need to init the origin because it was open during the previous run of Firefox, we go from (without patch) O(number of files stored in Cache API for the origin) file size gathering to O(open 1 database) for the origin. The caveat is that an upgrade step is needed to accomplish this and that step can will fundamentally be at least as slow as the pre-patch origin init time and is likely to take longer. That said, note that: 1) this is in nightly and will be in next beta regardless, and 2) users will only notice the slowness if a site or extension does something that blocks on QuotaManager access with LocalStorage being the most likely reason.
• Fix verified in Nightly: yes
• Steps to reproduce for manual QE testing: n/a
• Is Android affected?: yes
• String changes made/needed: none

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9367551 [details]
Bug 1866240 - Maintain usage information in the database; r=#dom-storage

Approved for 121.0b9.

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/f86cb690ce3d

Comment 13

[Cosmin Sabou [:CosminS]]

Backed out for causing xpcshell failures on test_unknownFiles.js

Push with failures: https://treeherder.mozilla.org/jobs?repo=mozilla-beta&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=f86cb690ce3d2d4b7cdda07cc40be83b3ca1cb21&searchStr=xpc&selectedTaskRun=ECarODuIRIeLbyNyGk7h5w.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=439300622&repo=mozilla-beta

Backout link: https://hg.mozilla.org/releases/mozilla-beta/rev/8fb6d84ab2c8e1fc73fb741955546550fed32953

Comment 14

[Jan Varga [:janv]]

The patch for Beta is now updated and can be landed again.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9367551 [details]
Bug 1866240 - Maintain usage information in the database; r=#dom-storage

Sorry, but we're out of betas this cycle and I'm not comfortable taking this directly into an RC, especially with this release going out so close to the holidays. Sorry this uplift got burned by the Lando bug :(

Comment 16

[Jan Varga [:janv]]

Ok, no problem.