1563023 - QM: Speedup temporary storage initialization

Status: RESOLVED FIXED

(Core :: Storage: Quota Manager, enhancement, P1)

Description

[Jan Varga [:janv]]

We had to disable LSNG in FF 68 because of slow temporary storage initialization in some cases. I have some ideas how to make it faster. I'll provide more details soon.

Comment 1

[Andrew Overholt [:overholt]]

Related telemetry: https://mzl.la/2XgtLhL

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Tracking as a critical LSNG perf bug.

Comment 3

[Jan Varga [:janv]]

I'm working on a prototype to get some numbers how much speedup we can expect.

Comment 4

[Jan Varga [:janv]]

So, I have some data...
I'm testing on a MacBook Pro (Late 2013 with 512 GB SSD)
My profile has 1246 origin directories in <profile>/storage/default

1. Without experimental patch
   Temporary storage initialization takes 1.791 secs

2. With experimental (quick and dirty) patch
   Temporary storage initialization takes 0.026 secs

So the caching helps a lot which is not surprising. However the final patch will have to cover all the corner cases that can happen and it can also happen that we would need to still cleanup/init origins that were used in previous Firefox session.
Also, after a crash we will have to check all origin directories like we do now.

Comment 5

[Jan Varga [:janv]]

Attachment: Bug 1563023 - Part 1: Implement per client usage tracking; r=asuth

This patch adds a fixed-size array of client usages to OriginInfo and modifies
quota tracking APIs to require the client type to be passed in.
A new method ResetUsageForClient is implemented. The method is used during
client-specific origin clearing. ResetUsageForClient is much faster than calling
GetUsageForOrigin and calling DecreaseUsageForOrigin after that.
LockedUsage now has an assertion that verifies that the total sum of client
usages matches total origin usage. This method should be called instead of touching mUsage directly.
A new assertion is added to GetQuotaObject which verifies that passed file
belongs to the given persistence type, origin, and client.

Comment 6

[Jan Varga [:janv]]

Attachment: Bug 1563023 - Part 2: Add support for getting origin usage from memory; r=asuth

This patch modifies getUsageForPrincipal to support getting origin usage from
memory. Support for getting group usage is factored out to a standalone method
called Estimate.
Operations based on NormalOriginOperationBase can now avoid directory locking
if they don't touch disk.

Comment 7

[Jan Varga [:janv]]

Attachment: Bug 1563023 - Part 3: Change client usage type to support the null value; r=asuth

This patch wraps the uint64_t type in a Maybe container, so the client usage can
represent a state when there are no files on disk for the given client. Zero
usage then represents a state when there are some files but they are empty or
the client tracks logical size (not physical size of files on disk) and the
logical size is zero. This can be useful especially for LocalStorage.

Comment 8

[Jan Varga [:janv]]

Attachment: Bug 1563023 - Part 4: Get rid of custom usage tracking in LS by using client usage tracked by QM; r=asuth

This patch gets rid of gUsages in LSNG. This provides better consistency and
makes it easier to cache quota info on disk.
The patch also fixes some edge cases when usage was not adjusted correctly after
a failed file or database operation.

Comment 9

[Jan Varga [:janv]]

Some notes on caching design:

• a major version bump shouldn't be needed (only minor)
• RemoveComponentRegistries nsAppRunner.cpp could be used for clearing the cache when different builds touch the same profile
• if FF was not shutdown cleanly, all origin dirs are scanned after startup
• origin dirs that were touched in the previous FF session need to be scanned despite clean shutdown
• origin dirs are scanned on first access if they weren't scanned already, any differences between cached and real info must be addressed

Proposed storage.sqlite changes:

CREATE TABLE repository (
id INTEGER PRIMARY KEY,
name TEXT NOT NULL
);

CREATE TABLE origin (
repository_id INTEGER NOT NULL,
name TEXT NOT NULL,
group_ TEXT NOT NULL,
client_usages TEXT NOT NULL,
usage INTEGER NOT NULL,
last_access_time INTEGER NOT NULL,
persisted INTEGER NOT NULL,
PRIMARY KEY (repository_id, name),
FOREIGN KEY (repository_id) REFERENCES repository(id)
);

INSERT INTO repository VALUES(0, 'persistent');
INSERT INTO repository VALUES(1, 'temporary');
INSERT INTO repository VALUES(2, 'default');

Comment 10

[Jan Varga [:janv]]

Attachment: Bug 1563023 - Part 5: Rationalize Client::TypeToText and Client::TypeFromText methods; r=asuth

This patch makes it easier to create new Client::TypeTo and Client::TypeFrom
variations by creating generic reusable helpers.

Comment 11

[Jan Varga [:janv]]

Attachment: Bug 1563023 - Part 6: Simplify iterations over all client types; r=asuth

This patch converts index based client type loops to iterator based client type
loops. This way the static cast is avoided and the loops are simpler and more
readable.

Comment 12

[Jan Varga [:janv]]

Patches part 1 - 6 are preparation patches. The next patch will implement basic caching functionality.

Comment 13

[Jan Varga [:janv]]

Attachment: Bug 1563023 - Part 7: Implement caching functionality; r=asuth

Comment 14

[Jan Varga [:janv]]

• a major version bump shouldn't be needed (only minor)
  Done

• RemoveComponentRegistries nsAppRunner.cpp could be used for clearing the cache when different builds touch the same profile
  Working on a patch for this

• if FF was not shutdown cleanly, all origin dirs are scanned after startup
  Done

• origin dirs that were touched in the previous FF session need to be scanned despite clean shutdown
  Done

• origin dirs are scanned on first access if they weren't scanned already, any differences between cached and real info must be addressed
  Not done yet.

Latest try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e55018c028d52f73664c630285a65665f311ccd3

Comment 15

[Jan Varga [:janv]]

Attachment: Bug 1563023 - Part 8: Ignore quota cache if the user loads the same profile in different builds; r=asuth

This patch adds support for quota caching purging if the profile is loaded in different builds.

Comment 16

[Jan Varga [:janv]]

Attachment: Bug 1563023 - Part 9: Ignore quota cache if LSNG is disabled; r=asuth

Comment 17

[Ryan VanderMeulen [:RyanVM]]

LSNG was disabled on Fx69.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8192923fd123
Part 1: Implement per client usage tracking; r=asuth

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e7c68f2d86c5
Part 2: Add support for getting origin usage from memory; r=asuth

Comment 20

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/8192923fd123

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/94258b96e70d
Part 3: Change client usage type to support the null value; r=asuth

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5df00af5913e
Part 4: Get rid of custom usage tracking in LS by using client usage tracked by QM; r=asuth

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b62dd021e170
Part 5: Rationalize Client::TypeToText and Client::TypeFromText methods; r=asuth
https://hg.mozilla.org/integration/autoland/rev/b70ad16ea2ae
Part 6: Simplify iterations over all client types; r=asuth

Comment 24

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/e7c68f2d86c5
https://hg.mozilla.org/mozilla-central/rev/94258b96e70d
https://hg.mozilla.org/mozilla-central/rev/5df00af5913e

Comment 25

[Jan Varga [:janv]]

All the preparation patches landed. The main patch should land soon, I just need some clarification from :asuth

Comment 26

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/b62dd021e170
https://hg.mozilla.org/mozilla-central/rev/b70ad16ea2ae

Comment 27

[Jan Varga [:janv]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9eea0cd028f5a03a0ec0c1a5538c15dd28a36aa1

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3d2a85653d22
Part 7: Implement caching functionality; r=asuth
https://hg.mozilla.org/integration/autoland/rev/d20d9464030e
Part 8: Ignore quota cache if the user loads the same profile in different builds; r=asuth
https://hg.mozilla.org/integration/autoland/rev/7c0c4b630628
Part 9: Ignore quota cache if LSNG is disabled; r=asuth

Comment 29

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/3d2a85653d22
https://hg.mozilla.org/mozilla-central/rev/d20d9464030e
https://hg.mozilla.org/mozilla-central/rev/7c0c4b630628

Comment 30

[Jan Varga [:janv]]

Initial telemetry:

The day just before caching landed:
https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2019-08-24&include_spill=0&keys=__none__!__none__!__none__&max_channel_version=nightly%252F70&measure=QM_REPOSITORIES_INITIALIZATION_TIME&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_by_value=0&sort_keys=submissions&start_date=2019-08-24&table=0&trim=1&use_submission_date=0

The day after caching landed:
https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2019-08-25&include_spill=0&keys=__none__!__none__!__none__&max_channel_version=nightly%252F70&measure=QM_REPOSITORIES_INITIALIZATION_TIME&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_by_value=0&sort_keys=submissions&start_date=2019-08-25&table=0&trim=1&use_submission_date=0

Median went down from 0.51602 s to 0.06333 s
75th Percentile went down from 1.8 s to 0.46739 s
95th Percentile went down from 14.5 s to 5.04 s

I think the numbers may improve even more after couple of days. Note that the cache can't be used if the build id changes, so Nightly users are currently slightly penalized.

Comment 31

[Chris H-C :chutten]

This bug may have contributed to a sudden change in the Telemetry probe QM_REPOSITORIES_INITIALIZATION_TIME which seems to have occurred in Nightly 20190825[3].

There was a sudden and prolonged drop in the reported values. This might me a confirmation that this work had the desired speedup effect in the wild.

Is this intentional? Is this expected?

Comment 32

[Jan Varga [:janv]]

Yes it's intentional and expected.