Closed Bug 1563023 Opened 5 months ago Closed 3 months ago

QM: Speedup temporary storage initialization

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox69 + disabled
firefox70 + fixed

People

(Reporter: janv, Assigned: janv)

References

Details

Attachments

(9 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

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.

Tracking as a critical LSNG perf bug.

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

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.

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.

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.

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.

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.

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');

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

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.

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

Attachment #9079607 - Attachment description: Bug 1563023 - Part 7: Implement basic caching functionality; r=asuth → Bug 1563023 - Part 7: Implement caching functionality; r=asuth
  • 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

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

Depends on: 1570644
Keywords: leave-open
Pushed by jvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8192923fd123
Part 1: Implement per client usage tracking; r=asuth
Regressions: 1575893
Pushed by jvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e7c68f2d86c5
Part 2: Add support for getting origin usage from memory; r=asuth
Pushed by jvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/94258b96e70d
Part 3: Change client usage type to support the null value; r=asuth
Pushed by jvarga@mozilla.com:
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
Pushed by jvarga@mozilla.com:
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
See Also: → 1576132

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

Pushed by jvarga@mozilla.com:
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
Keywords: leave-open
No longer regressions: 1575893
Depends on: 1576453
Regressions: 1576453
Regressions: 1576593
Blocks: 1560697
See Also: → 1578177
See Also: → 1578313

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?

Flags: needinfo?(jvarga)

Yes it's intentional and expected.

Flags: needinfo?(jvarga)
You need to log in before you can comment on or make changes to this bug.