Closed Bug 1425960 Opened 7 years ago Closed 7 years ago

Sync shouldn't hammer preferences so much.

Categories

(Firefox :: Sync, enhancement, P1)

enhancement

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)
Assignee: nobody → tchiovoloni
Priority: -- → P1
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+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: