Closed
Bug 1425960
Opened 7 years ago
Closed 7 years ago
Sync shouldn't hammer preferences so much.
Categories
(Firefox :: Sync, enhancement, P1)
Firefox
Sync
Tracking
()
RESOLVED
FIXED
Firefox 59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: tcsc, Assigned: tcsc)
References
Details
Attachments
(2 files)
We use preferences very heavily, and bug 1425613 indicates that maybe this is not such a good idea. I spent some time playing around with Gijs's patch and found the following (all numbers are from a full xpcshell-test run of services/sync/test)
Top 20 files reading/writing preferences
171906 resource://gre/modules/Preferences.jsm
28219 resource://services-sync/engines.js
25876 resource://gre/modules/TelemetryEnvironment.jsm
15102 resource://services-sync/service.js
11250 resource://services-sync/resource.js
10846 resource://services-sync/telemetry.js
9952 resource://services-sync/policies.js
9548 resource://services-common/observers.js
7425 resource://services-sync/engines/prefs.js
6529 resource://services-sync/util.js
4597 resource://services-sync/engines/clients.js
3538 resource://services-sync/browserid_identity.js
3112 resource://services-sync/stages/enginesync.js
2746 resource://services-sync/record.js
1385 resource://gre/modules/Task.jsm
1070 resource://services-common/utils.js
995 resource://services-sync/stages/declined.js
900 resource://services-common/logmanager.js
873 resource://services-sync/status.js
Of the preferences read or written by Preferences.jsm, our top 20 are:
11352 log.logger.network.resources
5072 client.GUID
4750 sendVersionInfo
4615 globalScore
3089 clients.lastSync
2991 client.type
2376 debug.ignoreCachedAuthCredentials
2285 nextSync
2216 engine.history
1864 engine.addons
1740 clients.devices.mobile
1717 clients.devices.desktop
1410 engine.bookmarks
1380 bookmarks.lastSync
1260 engine.prefs
1245 engine.passwords
1240 engine.tabs
1131 username
1025 client.syncID
1004 syncInterval
Most of the worst ones shouldn't be that bad to handle, the first of them is especially silly though. We probably should just have a top level logger in Resource.jsm or something, instead of instantiating it each time.
I've attached a file that contains the non-truncated version of this data, as well as a breakdown of the first list containing line numbers, which would be useful for figuring out what e.g. engines.js is reading (not that it's that hard to guess)
Updated•7 years ago
|
Assignee: nobody → tchiovoloni
Priority: -- → P1
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8938160 [details]
Bug 1425960 - Optimize sync preference usage
https://reviewboard.mozilla.org/r/208874/#review215300
Looks great, thanks!
::: services/sync/modules/policies.js:35
(Diff revision 1)
> "nsICaptivePortalService");
>
> // Get the value for an interval that's stored in preferences. To save users
> // from themselves (and us from them!) the minimum time they can specify
> // is 60s.
> -function getThrottledIntervalPreference(prefName) {
> +function defineThrottledIntervalPrefGetters(object, prefs) {
I don't think these "throttled interval" changes are worthwhile - we get the pref values only once (assuming no "startOver" etc, which I think is fine)
::: services/sync/modules/util.js:669
(Diff revision 1)
> + * };
> + */
> + defineLazyIDProperty(object, propName, prefName) {
> + // An object that exists to be the target of the lazy pref getter.
> + // We can't use `object` (at least, not using `propName`) since XPCOMUtils
> + // will stop on any setter we define.
s/stop/stomp/ ?
(Neat impl though)
Attachment #8938160 -
Flags: review?(markh) → review+
Comment hidden (mozreview-request) |
Pushed by tchiovoloni@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/39686aa3f685
Optimize sync preference usage r=markh
Comment 5•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
You need to log in
before you can comment on or make changes to this bug.
Description
•