Bug 1425960 - Optimize sync preference usage r=markh
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Wed, 20 Dec 2017 12:36:18 -0500
changeset 449446 39686aa3f685fd5204001cd18a4a009be90843de
parent 449445 ba4c8c662479f6880654525b6fcc0a706ec975b0
child 449447 96be4341e6a383968d0ba12aa20499d970e1a9c6
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1425960
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1425960 - Optimize sync preference usage r=markh MozReview-Commit-ID: AMwkvF7Dy3G
services/sync/modules/browserid_identity.js
services/sync/modules/engines.js
services/sync/modules/engines/clients.js
services/sync/modules/policies.js
services/sync/modules/resource.js
services/sync/modules/service.js
services/sync/modules/util.js
--- a/services/sync/modules/browserid_identity.js
+++ b/services/sync/modules/browserid_identity.js
@@ -31,16 +31,19 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://gre/modules/FxAccounts.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "log", function() {
   let log = Log.repository.getLogger("Sync.BrowserIDManager");
   log.manageLevelFromPref("services.sync.log.logger.identity");
   return log;
 });
 
+XPCOMUtils.defineLazyPreferenceGetter(this, "IGNORE_CACHED_AUTH_CREDENTIALS",
+                                      "services.sync.debug.ignoreCachedAuthCredentials");
+
 // FxAccountsCommon.js doesn't use a "namespace", so create one here.
 var fxAccountsCommon = {};
 Cu.import("resource://gre/modules/FxAccountsCommon.js", fxAccountsCommon);
 
 const OBSERVER_TOPICS = [
   fxAccountsCommon.ONLOGIN_NOTIFICATION,
   fxAccountsCommon.ONLOGOUT_NOTIFICATION,
   fxAccountsCommon.ON_ACCOUNT_STATE_CHANGE_NOTIFICATION,
@@ -165,16 +168,17 @@ this.BrowserIDManager = function Browser
   // NOTE: _fxaService and _tokenServerClient are replaced with mocks by
   // the test suite.
   this._fxaService = fxAccounts;
   this._tokenServerClient = new TokenServerClient();
   this._tokenServerClient.observerPrefix = "weave:service";
   // will be a promise that resolves when we are ready to authenticate
   this.whenReadyToAuthenticate = null;
   this._log = log;
+  XPCOMUtils.defineLazyPreferenceGetter(this, "_username", "services.sync.username");
 };
 
 this.BrowserIDManager.prototype = {
   _fxaService: null,
   _tokenServerClient: null,
   // https://docs.services.mozilla.com/token/apis.html
   _token: null,
   _signedInUser: null, // the signedinuser we got from FxAccounts.
@@ -427,17 +431,17 @@ this.BrowserIDManager.prototype = {
     return this._fxaService.localtimeOffsetMsec;
   },
 
   get syncKeyBundle() {
     return this._syncKeyBundle;
   },
 
   get username() {
-    return Svc.Prefs.get("username", null);
+    return this._username;
   },
 
   /**
    * Set the username value.
    *
    * Changing the username has the side-effect of wiping credentials.
    */
   set username(value) {
@@ -581,23 +585,17 @@ this.BrowserIDManager.prototype = {
 
   /**
    * Do we have a non-null, not yet expired token for the user currently
    * signed in?
    */
   hasValidToken() {
     // If pref is set to ignore cached authentication credentials for debugging,
     // then return false to force the fetching of a new token.
-    let ignoreCachedAuthCredentials = false;
-    try {
-      ignoreCachedAuthCredentials = Svc.Prefs.get("debug.ignoreCachedAuthCredentials");
-    } catch (e) {
-      // Pref doesn't exist
-    }
-    if (ignoreCachedAuthCredentials) {
+    if (IGNORE_CACHED_AUTH_CREDENTIALS) {
       return false;
     }
     if (!this._token) {
       return false;
     }
     if (this._token.expiration < this._now()) {
       return false;
     }
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -64,17 +64,16 @@ this.Tracker = function Tracker(name, en
     dataPostProcessor: json => this._dataPostProcessor(json),
     beforeSave: () => this._beforeSave(),
   });
   this.ignoreAll = false;
 
   Svc.Obs.add("weave:engine:start-tracking", this);
   Svc.Obs.add("weave:engine:stop-tracking", this);
 
-  Svc.Prefs.observe("engine." + this.engine.prefName, this);
 };
 
 Tracker.prototype = {
   /*
    * Score can be called as often as desired to decide which engines to sync
    *
    * Valid values for score:
    * -1: Do not sync unless the user specifically requests it (almost disabled)
@@ -243,30 +242,24 @@ Tracker.prototype = {
         }
         return;
       case "weave:engine:stop-tracking":
         this._log.trace("Got stop-tracking.");
         if (this._isTracking) {
           this.stopTracking();
           this._isTracking = false;
         }
-        return;
-      case "nsPref:changed":
-        if (data == PREFS_BRANCH + "engine." + this.engine.prefName) {
-          this.onEngineEnabledChanged(this.engine.enabled);
-        }
     }
   },
 
   async finalize() {
     // Stop listening for tracking and engine enabled change notifications.
     // Important for tests where we unregister the engine during cleanup.
     Svc.Obs.remove("weave:engine:start-tracking", this);
     Svc.Obs.remove("weave:engine:stop-tracking", this);
-    Svc.Prefs.ignore("engine." + this.engine.prefName, this);
 
     // Persist all pending tracked changes to disk, and wait for the final write
     // to finish.
     this._saveChangedIDs();
     await this._storage.finalize();
   },
 };
 
@@ -658,16 +651,21 @@ this.Engine = function Engine(name, serv
 
   this._notify = Utils.notify("weave:engine:");
   this._log = Log.repository.getLogger("Sync.Engine." + this.Name);
   this._log.manageLevelFromPref(`services.sync.log.logger.engine.${this.name}`);
 
   this._modified = this.emptyChangeset();
   this._tracker; // initialize tracker to load previously changed IDs
   this._log.debug("Engine constructed");
+
+  XPCOMUtils.defineLazyPreferenceGetter(this, "_enabled",
+    `services.sync.engine.${this.prefName}`, false,
+    (data, previous, latest) =>
+      this._tracker.onEngineEnabledChanged(latest));
 };
 Engine.prototype = {
   // _storeObj, and _trackerObj should to be overridden in subclasses
   _storeObj: Store,
   _trackerObj: Tracker,
 
   // Override this method to return a new changeset type.
   emptyChangeset() {
@@ -684,21 +682,23 @@ Engine.prototype = {
   // than the record upload limit.
   allowSkippedRecord: true,
 
   get prefName() {
     return this.name;
   },
 
   get enabled() {
-    return Svc.Prefs.get("engine." + this.prefName, false);
+    return this._enabled;
   },
 
   set enabled(val) {
-    Svc.Prefs.set("engine." + this.prefName, !!val);
+    if (!!val != this._enabled) {
+      Svc.Prefs.set("engine." + this.prefName, !!val);
+    }
   },
 
   get score() {
     return this._tracker.score;
   },
 
   get _store() {
     let store = new this._storeObj(this.Name, this);
@@ -771,17 +771,26 @@ this.SyncEngine = function SyncEngine(na
     beforeSave: () => this._beforeSaveMetadata(),
   });
 
   this._previousFailedStorage = new JSONFile({
     path: Utils.jsonFilePath("failed/" + this.name),
     dataPostProcessor: json => this._metadataPostProcessor(json),
     beforeSave: () => this._beforeSaveMetadata(),
   });
+  Utils.defineLazyIDProperty(this, "syncID", `services.sync.${this.name}.syncID`);
 
+  XPCOMUtils.defineLazyPreferenceGetter(this, "_lastSync",
+                                        `services.sync.${this.name}.lastSync`,
+                                        "0", null,
+                                        v => parseFloat(v));
+  XPCOMUtils.defineLazyPreferenceGetter(this, "_lastSyncLocal",
+                                        `services.sync.${this.name}.lastSyncLocal`,
+                                        "0", null,
+                                        v => parseInt(v, 10));
   // Async initializations can be made in the initialize() method.
 
   // The map of ids => metadata for records needing a weak upload.
   //
   // Currently the "metadata" fields are:
   //
   // - forceTombstone: whether or not we should ignore the local information
   //   about the record, and write a tombstone for it anyway -- e.g. in the case
@@ -875,40 +884,28 @@ SyncEngine.prototype = {
   get cryptoKeysURL() {
     return this.storageURL + "crypto/keys";
   },
 
   get metaURL() {
     return this.storageURL + "meta/global";
   },
 
-  get syncID() {
-    // Generate a random syncID if we don't have one
-    let syncID = Svc.Prefs.get(this.name + ".syncID", "");
-    return syncID == "" ? this.syncID = Utils.makeGUID() : syncID;
-  },
-  set syncID(value) {
-    Svc.Prefs.set(this.name + ".syncID", value);
-  },
-
   /*
    * lastSync is a timestamp in server time.
    */
   get lastSync() {
-    return parseFloat(Svc.Prefs.get(this.name + ".lastSync", "0"));
+    return this._lastSync;
   },
   set lastSync(value) {
-    // Reset the pref in-case it's a number instead of a string
-    Svc.Prefs.reset(this.name + ".lastSync");
     // Store the value as a string to keep floating point precision
     Svc.Prefs.set(this.name + ".lastSync", value.toString());
   },
   resetLastSync() {
     this._log.debug("Resetting " + this.name + " last sync time");
-    Svc.Prefs.reset(this.name + ".lastSync");
     Svc.Prefs.set(this.name + ".lastSync", "0");
     this.lastSyncLocal = 0;
   },
 
   get toFetch() {
     this._toFetchStorage.ensureDataReady();
     return this._toFetchStorage.data.ids;
   },
@@ -926,17 +923,17 @@ SyncEngine.prototype = {
     this._previousFailedStorage.data = { ids };
     this._previousFailedStorage.saveSoon();
   },
 
   /*
    * lastSyncLocal is a timestamp in local time.
    */
   get lastSyncLocal() {
-    return parseInt(Svc.Prefs.get(this.name + ".lastSyncLocal", "0"), 10);
+    return this._lastSyncLocal;
   },
   set lastSyncLocal(value) {
     // Store as a string because pref can only store C longs as numbers.
     Svc.Prefs.set(this.name + ".lastSyncLocal", value.toString());
   },
 
   /*
    * Returns a changeset for this sync. Engine implementations can override this
--- a/services/sync/modules/engines/clients.js
+++ b/services/sync/modules/engines/clients.js
@@ -86,24 +86,26 @@ Utils.deferGetSet(ClientsRec,
 
 this.ClientEngine = function ClientEngine(service) {
   SyncEngine.call(this, "Clients", service);
 
   // Reset the last sync timestamp on every startup so that we fetch all clients
   this.resetLastSync();
   this.fxAccounts = fxAccounts;
   this.addClientCommandQueue = Promise.resolve();
+  Utils.defineLazyIDProperty(this, "localID", "services.sync.client.GUID");
 };
 ClientEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _storeObj: ClientStore,
   _recordObj: ClientsRec,
   _trackerObj: ClientsTracker,
   allowSkippedRecord: false,
   _knownStaleFxADeviceIds: null,
+  _lastDeviceCounts: null,
 
   // These two properties allow us to avoid replaying the same commands
   // continuously if we cannot manage to upload our own record.
   _localClientLastModified: 0,
   get _lastModifiedOnProcessCommands() {
     return Services.prefs.getIntPref(LAST_MODIFIED_ON_PROCESS_COMMAND_PREF, -1);
   },
 
@@ -181,38 +183,24 @@ ClientEngine.prototype = {
       }
 
       counts.set(type, counts.get(type) + 1);
     }
 
     return counts;
   },
 
-  get localID() {
-    // Generate a random GUID id we don't have one
-    let localID = Svc.Prefs.get("client.GUID", "");
-    return localID == "" ? this.localID = Utils.makeGUID() : localID;
-  },
-  set localID(value) {
-    Svc.Prefs.set("client.GUID", value);
-  },
-
   get brandName() {
     let brand = Services.strings.createBundle(
       "chrome://branding/locale/brand.properties");
     return brand.GetStringFromName("brandShortName");
   },
 
   get localName() {
-    let name = Utils.getDeviceName();
-    // If `getDeviceName` returns the default name, set the pref. FxA registers
-    // the device before syncing, so we don't need to update the registration
-    // in this case.
-    Svc.Prefs.set("client.name", name);
-    return name;
+    return Utils.getDeviceName();
   },
   set localName(value) {
     Svc.Prefs.set("client.name", value);
     // Update the registration in the background.
     this.fxAccounts.updateDeviceRegistration().catch(error => {
       this._log.warn("failed to update fxa device registration", error);
     });
   },
@@ -557,35 +545,41 @@ ClientEngine.prototype = {
     }
   },
 
   async _syncFinish() {
     // Record histograms for our device types, and also write them to a pref
     // so non-histogram telemetry (eg, UITelemetry) and the sync scheduler
     // has easy access to them, and so they are accurate even before we've
     // successfully synced the first time after startup.
-    for (let [deviceType, count] of this.deviceTypes) {
+    let deviceTypeCounts = this.deviceTypes;
+    for (let [deviceType, count] of deviceTypeCounts) {
       let hid;
       let prefName = this.name + ".devices.";
       switch (deviceType) {
         case DEVICE_TYPE_DESKTOP:
           hid = "WEAVE_DEVICE_COUNT_DESKTOP";
           prefName += "desktop";
           break;
         case DEVICE_TYPE_MOBILE:
           hid = "WEAVE_DEVICE_COUNT_MOBILE";
           prefName += "mobile";
           break;
         default:
           this._log.warn(`Unexpected deviceType "${deviceType}" recording device telemetry.`);
           continue;
       }
       Services.telemetry.getHistogramById(hid).add(count);
-      Svc.Prefs.set(prefName, count);
+      // Optimization: only write the pref if it changed since our last sync.
+      if (this._lastDeviceCounts == null ||
+          this._lastDeviceCounts.get(prefName) != count) {
+        Svc.Prefs.set(prefName, count);
+      }
     }
+    this._lastDeviceCounts = deviceTypeCounts;
     return SyncEngine.prototype._syncFinish.call(this);
   },
 
   async _reconcile(item) {
     // Every incoming record is reconciled, so we use this to track the
     // contents of the collection on the server.
     this._incomingClients[item.id] = item.modified;
 
@@ -1072,14 +1066,14 @@ ClientsTracker.prototype = {
       case "weave:engine:stop-tracking":
         if (this._enabled) {
           Svc.Prefs.ignore("client.name", this);
           this._enabled = false;
         }
         break;
       case "nsPref:changed":
         this._log.debug("client.name preference changed");
-        this.addChangedID(Svc.Prefs.get("client.GUID"));
+        this.addChangedID(this.engine.localID);
         this.score += SCORE_INCREMENT_XLARGE;
         break;
     }
   }
 };
--- a/services/sync/modules/policies.js
+++ b/services/sync/modules/policies.js
@@ -80,42 +80,45 @@ SyncScheduler.prototype = {
   get nextSync() {
     return Svc.Prefs.get("nextSync", 0) * 1000;
   },
   set nextSync(value) {
     Svc.Prefs.set("nextSync", Math.floor(value / 1000));
   },
 
   get syncInterval() {
-    return Svc.Prefs.get("syncInterval", this.singleDeviceInterval);
+    return this._syncInterval;
   },
   set syncInterval(value) {
-    Svc.Prefs.set("syncInterval", value);
+    if (value != this._syncInterval) {
+      Services.prefs.setIntPref("services.sync.syncInterval", value);
+    }
   },
 
   get syncThreshold() {
-    return Svc.Prefs.get("syncThreshold", SINGLE_USER_THRESHOLD);
+    return this._syncThreshold;
   },
   set syncThreshold(value) {
-    Svc.Prefs.set("syncThreshold", value);
+    if (value != this._syncThreshold) {
+      Services.prefs.setIntPref("services.sync.syncThreshold", value);
+    }
   },
 
   get globalScore() {
-    return Svc.Prefs.get("globalScore", 0);
+    return this._globalScore;
   },
   set globalScore(value) {
-    Svc.Prefs.set("globalScore", value);
+    if (this._globalScore != value) {
+      Services.prefs.setIntPref("services.sync.globalScore", value);
+    }
   },
 
-  // The number of clients we have is maintained in preferences via the
-  // clients engine, and only updated after a successsful sync.
+  // Managed by the clients engine (by way of prefs)
   get numClients() {
-    return Svc.Prefs.get("clients.devices.desktop", 0) +
-           Svc.Prefs.get("clients.devices.mobile", 0);
-
+    return this.numDesktopClients + this.numMobileClients;
   },
   set numClients(value) {
     throw new Error("Don't set numClients - the clients engine manages it.");
   },
 
   get offline() {
     // Services.io.offline has slowly become fairly useless over the years - it
     // no longer attempts to track the actual network state by default, but one
@@ -132,19 +135,44 @@ SyncScheduler.prototype = {
         return true;
       }
     } catch (ex) {
       this._log.warn("Could not determine network status.", ex);
     }
     return false;
   },
 
+  _initPrefGetters() {
+
+    XPCOMUtils.defineLazyPreferenceGetter(this,
+      "idleTime", "services.sync.scheduler.idleTime");
+    XPCOMUtils.defineLazyPreferenceGetter(this,
+      "maxResyncs", "services.sync.maxResyncs", 0);
+
+    // The number of clients we have is maintained in preferences via the
+    // clients engine, and only updated after a successsful sync.
+    XPCOMUtils.defineLazyPreferenceGetter(this,
+      "numDesktopClients", "services.sync.clients.devices.desktop", 0);
+    XPCOMUtils.defineLazyPreferenceGetter(this,
+      "numMobileClients", "services.sync.clients.devices.mobile", 0);
+
+    // Scheduler state that seems to be read more often than it's written.
+    // We also check if the value has changed before writing in the setters.
+    XPCOMUtils.defineLazyPreferenceGetter(this,
+      "_syncThreshold", "services.sync.syncThreshold", SINGLE_USER_THRESHOLD);
+    XPCOMUtils.defineLazyPreferenceGetter(this,
+      "_syncInterval", "services.sync.syncInterval", this.singleDeviceInterval);
+    XPCOMUtils.defineLazyPreferenceGetter(this,
+      "_globalScore", "services.sync.globalScore", 0);
+  },
+
   init: function init() {
     this._log.manageLevelFromPref("services.sync.log.logger.service.main");
     this.setDefaults();
+    this._initPrefGetters();
     Svc.Obs.add("weave:engine:score:updated", this);
     Svc.Obs.add("network:offline-status-changed", this);
     Svc.Obs.add("network:link-status-changed", this);
     Svc.Obs.add("captive-portal-detected", this);
     Svc.Obs.add("weave:service:sync:start", this);
     Svc.Obs.add("weave:service:sync:finish", this);
     Svc.Obs.add("weave:engine:sync:finish", this);
     Svc.Obs.add("weave:engine:sync:error", this);
@@ -157,17 +185,17 @@ SyncScheduler.prototype = {
     Svc.Obs.add("weave:service:setup-complete", this);
     Svc.Obs.add("weave:service:start-over", this);
     Svc.Obs.add("FxA:hawk:backoff:interval", this);
 
     if (Status.checkSetup() == STATUS_OK) {
       Svc.Obs.add("wake_notification", this);
       Svc.Obs.add("captive-portal-login-success", this);
       Svc.Obs.add("sleep_notification", this);
-      IdleService.addIdleObserver(this, Svc.Prefs.get("scheduler.idleTime"));
+      IdleService.addIdleObserver(this, this.idleTime);
     }
   },
 
   // eslint-disable-next-line complexity
   observe: function observe(subject, topic, data) {
     this._log.trace("Handling " + topic);
     switch (topic) {
       case "weave:engine:score:updated":
@@ -303,25 +331,25 @@ SyncScheduler.prototype = {
           this.hasIncomingItems = true;
         }
         if (subject.newFailed) {
           this._log.error(`Engine ${data} found ${subject.newFailed} new records that failed to apply`);
         }
         break;
       case "weave:service:setup-complete":
          Services.prefs.savePrefFile(null);
-         IdleService.addIdleObserver(this, Svc.Prefs.get("scheduler.idleTime"));
+         IdleService.addIdleObserver(this, this.idleTime);
          Svc.Obs.add("wake_notification", this);
          Svc.Obs.add("captive-portal-login-success", this);
          Svc.Obs.add("sleep_notification", this);
          break;
       case "weave:service:start-over":
          this.setDefaults();
          try {
-           IdleService.removeIdleObserver(this, Svc.Prefs.get("scheduler.idleTime"));
+           IdleService.removeIdleObserver(this, this.idleTime);
          } catch (ex) {
            if (ex.result != Cr.NS_ERROR_FAILURE) {
              throw ex;
            }
            // In all likelihood we didn't have an idle observer registered yet.
            // It's all good.
          }
          break;
@@ -407,23 +435,24 @@ SyncScheduler.prototype = {
     } else {
       this._log.trace("Adjusting syncInterval to activeInterval.");
       this.syncInterval = this.activeInterval;
     }
   },
 
   updateGlobalScore() {
     let engines = [this.service.clientsEngine].concat(this.service.engineManager.getEnabled());
+    let globalScore = this.globalScore;
     for (let i = 0;i < engines.length;i++) {
       this._log.trace(engines[i].name + ": score: " + engines[i].score);
-      this.globalScore += engines[i].score;
+      globalScore += engines[i].score;
       engines[i]._tracker.resetScore();
     }
-
-    this._log.trace("Global score updated: " + this.globalScore);
+    this.globalScore = globalScore;
+    this._log.trace("Global score updated: " + globalScore);
   },
 
   calculateScore() {
     this.updateGlobalScore();
     this.checkSyncStatus();
   },
 
   /**
@@ -511,21 +540,21 @@ SyncScheduler.prototype = {
     // Ensure the interval is set to no less than the backoff.
     if (Status.backoffInterval && interval < Status.backoffInterval) {
       this._log.trace("Requested interval " + interval +
                       " ms is smaller than the backoff interval. " +
                       "Using backoff interval " +
                       Status.backoffInterval + " ms instead.");
       interval = Status.backoffInterval;
     }
-
-    if (this.nextSync != 0) {
+    let nextSync = this.nextSync;
+    if (nextSync != 0) {
       // There's already a sync scheduled. Don't reschedule if there's already
       // a timer scheduled for sooner than requested.
-      let currentInterval = this.nextSync - Date.now();
+      let currentInterval = nextSync - Date.now();
       this._log.trace("There's already a sync scheduled in " +
                       currentInterval + " ms.");
       if (currentInterval < interval && this.syncTimer) {
         this._log.trace("Ignoring scheduling request for next sync in " +
                         interval + " ms.");
         return;
       }
     }
@@ -622,19 +651,16 @@ SyncScheduler.prototype = {
     this._log.debug("Clearing sync triggers and the global score.");
     this.globalScore = this.nextSync = 0;
 
     // Clear out any scheduled syncs
     if (this.syncTimer)
       this.syncTimer.clear();
   },
 
-  get maxResyncs() {
-    return Svc.Prefs.get("maxResyncs", 0);
-  },
 };
 
 this.ErrorHandler = function ErrorHandler(service) {
   this.service = service;
   this.init();
 };
 ErrorHandler.prototype = {
   MINIMUM_ALERT_INTERVAL_MSEC: 604800000,   // One week.
--- a/services/sync/modules/resource.js
+++ b/services/sync/modules/resource.js
@@ -32,16 +32,21 @@ Cu.importGlobalProperties(["fetch"]);
 this.Resource = function Resource(uri) {
   this._log = Log.repository.getLogger(this._logName);
   this._log.manageLevelFromPref("services.sync.log.logger.network.resources");
   this.uri = uri;
   this._headers = {};
 };
 // (static) Caches the latest server timestamp (X-Weave-Timestamp header).
 Resource.serverTime = null;
+
+XPCOMUtils.defineLazyPreferenceGetter(Resource,
+                                      "SEND_VERSION_INFO",
+                                      "services.sync.sendVersionInfo",
+                                      true);
 Resource.prototype = {
   _logName: "Sync.Resource",
 
   /**
    * Callback to be invoked at request time to add authentication details.
    *
    * By default, a global authenticator is provided. If this is set, it will
    * be used instead of the global one.
@@ -86,17 +91,17 @@ Resource.prototype = {
 
   /**
    * @param {string} method HTTP method
    * @returns {Headers}
    */
   _buildHeaders(method) {
     const headers = new Headers(this._headers);
 
-    if (Svc.Prefs.get("sendVersionInfo", true)) {
+    if (Resource.SEND_VERSION_INFO) {
       headers.append("user-agent", Utils.userAgent);
     }
 
     if (this.authenticator) {
       const result = this.authenticator(this, method);
       if (result && result.headers) {
         for (const [k, v] of Object.entries(result.headers)) {
           headers.append(k.toLowerCase(), v);
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -64,16 +64,17 @@ function getEngineModules() {
 
 // A unique identifier for this browser session. Used for logging so
 // we can easily see whether 2 logs are in the same browser session or
 // after the browser restarted.
 XPCOMUtils.defineLazyGetter(this, "browserSessionID", Utils.makeGUID);
 
 function Sync11Service() {
   this._notify = Utils.notify("weave:service:");
+  Utils.defineLazyIDProperty(this, "syncID", "services.sync.client.syncID");
 }
 Sync11Service.prototype = {
 
   _lock: Utils.lock,
   _locked: false,
   _loggedIn: false,
 
   infoURL: null,
@@ -90,25 +91,16 @@ Sync11Service.prototype = {
   set clusterURL(value) {
     if (value != null && typeof value != "string") {
       throw new Error("cluster must be a string, got " + (typeof value));
     }
     this._clusterURL = value;
     this._updateCachedURLs();
   },
 
-  get syncID() {
-    // Generate a random syncID id we don't have one
-    let syncID = Svc.Prefs.get("client.syncID", "");
-    return syncID == "" ? this.syncID = Utils.makeGUID() : syncID;
-  },
-  set syncID(value) {
-    Svc.Prefs.set("client.syncID", value);
-  },
-
   get isLoggedIn() { return this._loggedIn; },
 
   get locked() { return this._locked; },
   lock: function lock() {
     if (this._locked)
       return false;
     this._locked = true;
     return true;
--- a/services/sync/modules/util.js
+++ b/services/sync/modules/util.js
@@ -22,16 +22,24 @@ XPCOMUtils.defineLazyGetter(this, "FxAcc
   Cu.import("resource://gre/modules/FxAccountsCommon.js", FxAccountsCommon);
   return FxAccountsCommon;
 });
 
 XPCOMUtils.defineLazyServiceGetter(this, "cryptoSDR",
                                    "@mozilla.org/login-manager/crypto/SDR;1",
                                    "nsILoginManagerCrypto");
 
+XPCOMUtils.defineLazyPreferenceGetter(this, "localDeviceName",
+                                      "services.sync.client.name",
+                                      "");
+
+XPCOMUtils.defineLazyPreferenceGetter(this, "localDeviceType",
+                                      "services.sync.client.type",
+                                      DEVICE_TYPE_DESKTOP);
+
 /*
  * Custom exception types.
  */
 class LockException extends Error {
   constructor(message) {
     super(message);
     this.name = "LockException";
   }
@@ -75,17 +83,17 @@ this.Utils = {
       /* eslint-disable no-multi-spaces */
       this._userAgent =
         Services.appinfo.name + "/" + Services.appinfo.version +  // Product.
         " (" + hph.oscpu + ")" +                                  // (oscpu)
         " FxSync/" + WEAVE_VERSION + "." +                        // Sync.
         Services.appinfo.appBuildID + ".";                        // Build.
       /* eslint-enable no-multi-spaces */
     }
-    return this._userAgent + Svc.Prefs.get("client.type", "desktop");
+    return this._userAgent + localDeviceType;
   },
 
   /**
    * Wrap a [promise-returning] function to catch all exceptions and log them.
    *
    * @usage MyObj._catch = Utils.catch;
    *        MyObj.foo = function() { this._catch(func)(); }
    *
@@ -621,27 +629,70 @@ this.Utils = {
       // fall back on ua info string
       Cc["@mozilla.org/network/protocol;1?name=http"].getService(Ci.nsIHttpProtocolHandler).oscpu;
 
     let syncStrings = Services.strings.createBundle("chrome://weave/locale/sync.properties");
     return syncStrings.formatStringFromName("client.name2", [user, brandName, system], 3);
   },
 
   getDeviceName() {
-    const deviceName = Svc.Prefs.get("client.name", "");
+    let deviceName = localDeviceName;
 
     if (deviceName === "") {
-      return this.getDefaultDeviceName();
+      deviceName = this.getDefaultDeviceName();
+      Svc.Prefs.set("client.name", deviceName);
     }
 
     return deviceName;
   },
 
+  /**
+   * Helper to implement a more efficient version of fairly common pattern:
+   *
+   * Utils.defineLazyIDProperty(this, "syncID", "services.sync.client.syncID")
+   *
+   * is equivalent to (but more efficient than) the following:
+   *
+   * Foo.prototype = {
+   *   ...
+   *   get syncID() {
+   *     let syncID = Svc.Prefs.get("client.syncID", "");
+   *     return syncID == "" ? this.syncID = Utils.makeGUID() : syncID;
+   *   },
+   *   set syncID(value) {
+   *     Svc.Prefs.set("client.syncID", value);
+   *   },
+   *   ...
+   * };
+   */
+  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 stomp on any setter we define.
+    const storage = {};
+    XPCOMUtils.defineLazyPreferenceGetter(storage, "value", prefName, "");
+    Object.defineProperty(object, propName, {
+      configurable: true,
+      enumerable: true,
+      get() {
+        let value = storage.value;
+        if (!value) {
+          value = Utils.makeGUID();
+          Services.prefs.setStringPref(prefName, value);
+        }
+        return value;
+      },
+      set(value) {
+        Services.prefs.setStringPref(prefName, value);
+      }
+    });
+  },
+
   getDeviceType() {
-    return Svc.Prefs.get("client.type", DEVICE_TYPE_DESKTOP);
+    return localDeviceType;
   },
 
   formatTimestamp(date) {
     // Format timestamp as: "%Y-%m-%d %H:%M:%S"
     let year = String(date.getFullYear());
     let month = String(date.getMonth() + 1).padStart(2, "0");
     let day = String(date.getDate()).padStart(2, "0");
     let hours = String(date.getHours()).padStart(2, "0");