Bug 1425960 - Optimize sync preference usage r?markh draft
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Wed, 20 Dec 2017 12:36:18 -0500
changeset 715330 d47b3a0774f76ad0983741b1862186ab49e20d37
parent 715271 ac93fdadf1022211eec62258ad22b42cb37a6d14
child 744769 4a461c90c971349acc86c99fc1857644d4e8401f
push id94136
push userbmo:tchiovoloni@mozilla.com
push dateWed, 03 Jan 2018 18:07:15 +0000
reviewersmarkh
bugs1425960
milestone59.0a1
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");