Bug 1536170 - Replace Async.jankYielder r=tcsc,markh,eoger
☠☠ backed out by fd8c9a28b4d4 ☠ ☠
authorBarret Rennie <barret@brennie.ca>
Thu, 11 Apr 2019 18:39:43 +0000
changeset 469066 51a67bffd7d2670d12d87058973bc01e64f10096
parent 469065 18af3ac7686b947a75f45190c9d08f9f019d4982
child 469067 ccea2e827d9dff136b8f6ac9b357e3fabb5e3ec7
push id35856
push usercsabou@mozilla.com
push dateFri, 12 Apr 2019 03:19:48 +0000
treeherdermozilla-central@940684cd1065 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcsc, markh, eoger
bugs1536170
milestone68.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 1536170 - Replace Async.jankYielder r=tcsc,markh,eoger `Async.jankYielder` is known to, unfortunately, cause jank by creating a lot of immediately resolved promises that must be then GCed. For a collection of 50 items, it will create 50 promises and 49 of them will immediately resolve. Instead of `Async.jankYielder`, we now have `Async.yieldState`, which simply keeps track of whether or not the caller should yield to the event loop. Two higher level looping constructs are built on top of it: * `Async.yieldingIterator`, which has been rewritten to not create extraneous promises; and * `Async.yieldingForEach`, which is a replacement for awaiting `Async.jankYielder` in a loop. Instead, it accepts the loop body as a function. Each of these can share an instance of an `Async.yieldState`, which allows an object with multiple loops to yield every N iterations overall, instead of every N iterations of each loop, which keeps the behaviour of using one `Async.jankYielders` in multiple places. Differential Revision: https://phabricator.services.mozilla.com/D26229
browser/components/translation/TranslationDocument.jsm
services/common/async.js
services/sync/modules/bookmark_validator.js
services/sync/modules/collection_validator.js
services/sync/modules/engines.js
services/sync/modules/engines/bookmarks.js
toolkit/components/places/SyncedBookmarksMirror.jsm
--- a/browser/components/translation/TranslationDocument.jsm
+++ b/browser/components/translation/TranslationDocument.jsm
@@ -201,21 +201,17 @@ this.TranslationDocument.prototype = {
    * @param target   A string that is either "translation"
    *                 or "original".
    */
   _swapDocumentContent(target) {
     (async () => {
       // Let the event loop breath on every 100 nodes
       // that are replaced.
       const YIELD_INTERVAL = 100;
-      let maybeYield = Async.jankYielder(YIELD_INTERVAL);
-      for (let root of this.roots) {
-        root.swapText(target);
-        await maybeYield();
-      }
+      await Async.yieldingForEach(this.roots, root => root.swapText(target), YIELD_INTERVAL);
     })();
   },
 };
 
 /**
  * This class represents an item for translation. It's basically our
  * wrapper class around a node returned by getTranslationNode, with
  * more data and structural information on it.
--- a/services/common/async.js
+++ b/services/common/async.js
@@ -94,47 +94,98 @@ var Async = {
    * - https://bugzilla.mozilla.org/show_bug.cgi?id=1094248
    */
   promiseYield() {
     return new Promise(resolve => {
       Services.tm.currentThread.dispatch(resolve, Ci.nsIThread.DISPATCH_NORMAL);
     });
   },
 
-  // Returns a method that yields every X calls.
-  // Common case is calling the returned method every iteration in a loop.
-  jankYielder(yieldEvery = 50) {
+  /* Shared state for yielding every N calls.
+   *
+   * Can be passed to multiple Async.yieldingForEach to have them overall yield
+   * every N iterations.
+   */
+  yieldState(yieldEvery = 50) {
     let iterations = 0;
-    return async () => {
-      Async.checkAppReady(); // Let it throw!
-      if (++iterations % yieldEvery === 0) {
-        await Async.promiseYield();
-      }
+
+    return {
+      shouldYield() {
+        ++iterations;
+        return iterations % yieldEvery === 0;
+      },
     };
   },
 
   /**
-   * Turn a synchronous iterator/iterable into an async iterator that calls a
-   * Async.jankYielder at each step.
+   * Apply the given function to each element of the iterable, yielding the
+   * event loop every yieldEvery iterations.
    *
    * @param iterable {Iterable}
-   *        Iterable or iterator that should be wrapped. (Anything usable in
-   *        for..of should work)
+   *        The iterable or iterator to iterate through.
+   *
+   * @param fn {(*) -> void|boolean}
+   *        The function to be called on each element of the iterable.
+   *
+   *        Returning true from the function will stop the iteration.
    *
-   * @param [maybeYield = 50] {number|() => Promise<void>}
-   *        Either an existing jankYielder to use, or a number to provide as the
-   *        argument to Async.jankYielder.
+   * @param [yieldEvery = 50] {number|object}
+   *        The number of iterations to complete before yielding back to the event
+   *        loop.
+   *
+   * @return {boolean}
+   *         Whether or not the function returned early.
    */
-  async* yieldingIterator(iterable, maybeYield = 50) {
-    if (typeof maybeYield == "number") {
-      maybeYield = Async.jankYielder(maybeYield);
+  async yieldingForEach(iterable, fn, yieldEvery = 50) {
+    const yieldState = typeof yieldEvery === "number" ? Async.yieldState(yieldEvery) : yieldEvery;
+    let iteration = 0;
+
+    for (const item of iterable) {
+      let result = fn(item, iteration++);
+      if (typeof result !== "undefined" && typeof result.then !== "undefined") {
+        // If we await result when it is not a Promise, we create an
+        // automatically resolved promise, which is exactly the case that we
+        // are trying to avoid.
+        result = await result;
+      }
+
+      if (result === true) {
+        return true;
+      }
+
+      if (yieldState.shouldYield()) {
+        await Async.promiseYield();
+        Async.checkAppReady();
+      }
     }
-    for (let item of iterable) {
-      await maybeYield();
+
+    return false;
+  },
+
+  /**
+   * Turn a synchronous iterator/iterable into an async iterator that yields in
+   * the same manner as Async.yieldingForEach.
+   *
+   * @param iterable {Iterable}
+   *        The iterable or iterator that should be wrapped.
+   *
+   * @param yieldEvery {number|object}
+   *        Either an existing Async.yieldState to use, or a number to provide as
+   *        the argument to async.yieldState.
+   */
+  async* yieldingIterator(iterable, yieldEvery = 50) {
+    const yieldState = typeof yieldEvery === "number" ? Async.yieldState(yieldEvery) : yieldEvery;
+
+    for (const item of iterable) {
       yield item;
+
+      if (yieldState.shouldYield()) {
+        await Async.promiseYield();
+        Async.checkAppReady();
+      }
     }
   },
 
   asyncQueueCaller(log) {
     return new AsyncQueueCaller(log);
   },
 
   asyncObserver(log, obj) {
--- a/services/sync/modules/bookmark_validator.js
+++ b/services/sync/modules/bookmark_validator.js
@@ -235,19 +235,19 @@ XPCOMUtils.defineLazyGetter(this, "ROOT_
 async function detectCycles(records) {
   // currentPath and pathLookup contain the same data. pathLookup is faster to
   // query, but currentPath gives is the order of traversal that we need in
   // order to report the members of the cycles.
   let pathLookup = new Set();
   let currentPath = [];
   let cycles = [];
   let seenEver = new Set();
-  const maybeYield = Async.jankYielder();
-  const traverse = async node => {
-    await maybeYield();
+  const yieldState = Async.yieldState();
+
+  const traverse = async (node) => {
     if (pathLookup.has(node)) {
       let cycleStart = currentPath.lastIndexOf(node);
       let cyclePath = currentPath.slice(cycleStart).map(n => n.id);
       cycles.push(cyclePath);
       return;
     } else if (seenEver.has(node)) {
       // If we're checking the server, this is a problem, but it should already be reported.
       // On the client, this could happen due to including `node.concrete` in the child list.
@@ -256,28 +256,27 @@ async function detectCycles(records) {
     seenEver.add(node);
     let children = node.children || [];
     if (node.concreteItems) {
       children.push(...node.concreteItems);
     }
     if (children.length) {
       pathLookup.add(node);
       currentPath.push(node);
-      for (let child of children) {
-        await traverse(child);
-      }
+      await Async.yieldingForEach(children, traverse, yieldState);
       currentPath.pop();
       pathLookup.delete(node);
     }
   };
-  for (let record of records) {
+
+  await Async.yieldingForEach(records, async (record) => {
     if (!seenEver.has(record)) {
       await traverse(record);
     }
-  }
+  }, yieldState);
 
   return cycles;
 }
 
 class ServerRecordInspection {
   constructor() {
     this.serverRecords = null;
     this.liveRecords = [];
@@ -292,17 +291,17 @@ class ServerRecordInspection {
     this.deletedRecords = [];
 
     this.problemData = new BookmarkProblemData();
 
     // These are handled outside of problemData
     this._orphans = new Map();
     this._multipleParents = new Map();
 
-    this.maybeYield = Async.jankYielder();
+    this.yieldState = Async.yieldState();
   }
 
   static async create(records) {
     return new ServerRecordInspection().performInspection(records);
   }
 
   async performInspection(records) {
     await this._setRecords(records);
@@ -346,51 +345,50 @@ class ServerRecordInspection {
     if (this.serverRecords) {
       // In general this class is expected to be created, have
       // `performInspection` called, and then only read from from that point on.
       throw new Error("Bug: ServerRecordInspection can't `setRecords` twice");
     }
     this.serverRecords = records;
     let rootChildren = [];
 
-    for (let record of this.serverRecords) {
-      await this.maybeYield();
+    await Async.yieldingForEach(this.serverRecords, async (record) => {
       if (!record.id) {
         ++this.problemData.missingIDs;
-        continue;
+        return;
       }
 
       if (record.deleted) {
         this.deletedIds.add(record.id);
       }
       if (this.idToRecord.has(record.id)) {
         this.problemData.duplicates.push(record.id);
-        continue;
+        return;
       }
 
       this.idToRecord.set(record.id, record);
 
       if (!record.deleted) {
         this.liveRecords.push(record);
 
         if (record.parentid == "places") {
           rootChildren.push(record);
         }
       }
 
       if (!record.children) {
-        continue;
+        return;
       }
 
       if (record.type != "folder") {
         // Due to implementation details in engines/bookmarks.js, (Livemark
         // subclassing BookmarkFolder) Livemarks will have a children array,
         // but it should still be empty.
         if (!record.children.length) {
-          continue;
+          return;
         }
         // Otherwise we mark it as an error and still try to resolve the children
         this.problemData.childrenOnNonFolder.push(record.id);
       }
 
       this.folders.push(record);
 
       if (new Set(record.children).size !== record.children.length) {
@@ -408,22 +406,23 @@ class ServerRecordInspection {
       //   still have deleted, duplicate, mismatching, etc. children.
       //
       // - children: This is the 'cleaned' version of the child records that are
       //   safe to iterate over, etc.. If there are no reported problems, it should
       //   be identical to unfilteredChildren.
       //
       // The last two are left alone until later `this._linkChildren`, however.
       record.childGUIDs = record.children;
-      for (let id of record.childGUIDs) {
-        await this.maybeYield();
+
+      await Async.yieldingForEach(record.childGUIDs, id => {
         this.noteParent(id, record.id);
-      }
+      }, this.yieldState);
+
       record.children = [];
-    }
+    }, this.yieldState);
 
     // Finish up some parts we can easily do now that we have idToRecord.
     this.deletedRecords = Array.from(this.deletedIds,
       id => this.idToRecord.get(id));
 
     this._initRoot(rootChildren);
   }
 
@@ -450,130 +449,128 @@ class ServerRecordInspection {
       title: "",
     };
     this.liveRecords.push(this.root);
     this.idToRecord.set("places", this.root);
   }
 
   // Adds `parent` to all records it can that have `parentid`
   async _linkParentIDs() {
-    for (let [id, record] of this.idToRecord) {
-      await this.maybeYield();
+    await Async.yieldingForEach(this.idToRecord, ([id, record]) => {
       if (record == this.root || record.deleted) {
-        continue;
+        return false;
       }
 
       // Check and update our orphan map.
       let parentID = record.parentid;
       let parent = this.idToRecord.get(parentID);
       if (!parentID || !parent) {
         this._noteOrphan(id, parentID);
-        continue;
+        return false;
       }
 
       record.parent = parent;
 
       if (parent.deleted) {
         this.problemData.deletedParents.push(id);
-        return;
+        return true;
       } else if (parent.type != "folder") {
         this.problemData.parentNotFolder.push(record.id);
-        return;
+        return true;
       }
 
       if (parent.id !== "place" || this.problemData.rootOnServer) {
         if (!parent.childGUIDs.includes(record.id)) {
           this.noteMismatch(record.id, parent.id);
         }
       }
 
       if (parent.deleted && !record.deleted) {
         this.problemData.deletedParents.push(record.id);
       }
 
       // Note: We used to check if the parentName on the server matches the
       // actual local parent name, but given this is used only for de-duping a
       // record the first time it is seen and expensive to keep up-to-date, we
       // decided to just stop recording it. See bug 1276969 for more.
-    }
+      return false;
+    }, this.yieldState);
   }
 
   // Build the children and unfilteredChildren arrays, (which are of record
   // objects, not ids)
   async _linkChildren() {
     // Check that we aren't missing any children.
-    for (let folder of this.folders) {
-      await this.maybeYield();
-
+    await Async.yieldingForEach(this.folder, async (folder) => {
       folder.children = [];
       folder.unfilteredChildren = [];
 
       let idsThisFolder = new Set();
 
-      for (let i = 0; i < folder.childGUIDs.length; ++i) {
-        await this.maybeYield();
-        let childID = folder.childGUIDs[i];
-
+      await Async.yieldingForEach(folder.childGUIDs, async (childID) => {
         let child = this.idToRecord.get(childID);
 
         if (!child) {
           this.problemData.missingChildren.push({ parent: folder.id, child: childID });
-          continue;
+          return;
         }
 
         if (child.deleted) {
           this.problemData.deletedChildren.push({ parent: folder.id, child: childID });
-          continue;
+          return;
         }
 
         if (child.parentid != folder.id) {
           this.noteMismatch(childID, folder.id);
-          continue;
+          return;
         }
 
         if (idsThisFolder.has(childID)) {
           // Already recorded earlier, we just don't want to mess up `children`
-          continue;
+          return;
         }
         folder.children.push(child);
-      }
-    }
+      }, this.yieldState);
+    }, this.yieldState);
   }
 
   // Finds the orphans in the tree using something similar to a `mark and sweep`
   // strategy. That is, we iterate over the children from the root, remembering
   // which items we've seen. Then, we iterate all items, and know the ones we
   // haven't seen are orphans.
   async _findOrphans() {
     let seen = new Set([this.root.id]);
-    for (let [node] of Utils.walkTree(this.root)) {
-      await this.maybeYield();
+
+    const inCycle = await Async.yieldingForEach(Utils.walkTree(this.root), ([node]) => {
       if (seen.has(node.id)) {
         // We're in an infloop due to a cycle.
         // Return early to avoid reporting false positives for orphans.
-        return;
+        return true;
       }
       seen.add(node.id);
+
+      return false;
+    }, this.yieldState);
+
+    if (inCycle) {
+      return;
     }
 
-    for (let i = 0; i < this.liveRecords.length; ++i) {
-      await this.maybeYield();
-      let record = this.liveRecords[i];
+    await Async.yieldingForEach(this.liveRecords, (record, i) => {
       if (!seen.has(record.id)) {
         // We intentionally don't record the parentid here, since we only record
         // that if the record refers to a parent that doesn't exist, which we
         // have already handled (when linking parentid's).
         this._noteOrphan(record.id);
       }
-    }
+    }, this.yieldState);
 
-    for (const [id, parent] of this._orphans) {
-      await this.maybeYield();
+    await Async.yieldingForEach(this._orphans, ([id, parent]) => {
       this.problemData.orphans.push({id, parent});
-    }
+    }, this.yieldState);
   }
 
   async _finish() {
     this.problemData.cycles = await detectCycles(this.liveRecords);
 
     for (const [child, recordedParents] of this._multipleParents) {
       let parents = new Set(recordedParents);
       if (parents.size > 1) {
@@ -592,61 +589,60 @@ class ServerRecordInspection {
     for (let prop of idArrayProps) {
       this.problemData[prop] = [...new Set(this.problemData[prop])];
     }
   }
 }
 
 class BookmarkValidator {
   constructor() {
-    this.maybeYield = Async.jankYielder();
+    this.yieldState = Async.yieldState();
   }
 
   async canValidate() {
     return !await PlacesSyncUtils.bookmarks.havePendingChanges();
   }
 
   async _followQueries(recordsByQueryId) {
-    for (let entry of recordsByQueryId.values()) {
-      await this.maybeYield();
+    await Async.yieldingForEach(recordsByQueryId.values(), entry => {
       if (entry.type !== "query" && (!entry.bmkUri || !entry.bmkUri.startsWith(QUERY_PROTOCOL))) {
-        continue;
+        return;
       }
       let params = new URLSearchParams(entry.bmkUri.slice(QUERY_PROTOCOL.length));
       // Queries with `excludeQueries` won't form cycles because they'll
       // exclude all queries, including themselves, from the result set.
       let excludeQueries = params.get("excludeQueries");
       if (excludeQueries === "1" || excludeQueries === "true") {
         // `nsNavHistoryQuery::ParseQueryBooleanString` allows `1` and `true`.
-        continue;
+        return;
       }
       entry.concreteItems = [];
       let queryIds = params.getAll("folder");
       for (let queryId of queryIds) {
         let concreteItem = recordsByQueryId.get(queryId);
         if (concreteItem) {
           entry.concreteItems.push(concreteItem);
         }
       }
-    }
+    }, this.yieldState);
   }
 
   async createClientRecordsFromTree(clientTree) {
     // Iterate over the treeNode, converting it to something more similar to what
     // the server stores.
     let records = [];
     // A map of local IDs and well-known query folder names to records. Unlike
     // GUIDs, local IDs aren't synced, since they're not stable across devices.
     // New Places APIs use GUIDs to refer to bookmarks, but the legacy APIs
     // still use local IDs. We use this mapping to parse `place:` queries that
     // refer to folders via their local IDs.
     let recordsByQueryId = new Map();
     let syncedRoots = SYNCED_ROOTS;
+
     const traverse = async (treeNode, synced) => {
-      await this.maybeYield();
       if (!synced) {
         synced = syncedRoots.includes(treeNode.guid);
       }
       let localId = treeNode.id;
       let guid = PlacesSyncUtils.bookmarks.guidToRecordId(treeNode.guid);
       let itemType = "item";
       treeNode.ignored = !synced;
       treeNode.id = guid;
@@ -698,25 +694,28 @@ class BookmarkValidator {
         // `place:folder=MOBILE_BOOKMARKS` are equivalent.
         recordsByQueryId.set(localId.toString(10), treeNode);
       }
       if (treeNode.type === "folder") {
         treeNode.childGUIDs = [];
         if (!treeNode.children) {
           treeNode.children = [];
         }
-        for (let child of treeNode.children) {
+
+        await Async.yieldingForEach(treeNode.children, async (child) => {
           await traverse(child, synced);
           child.parent = treeNode;
-          child.parentid = guid;
+          child.parentId = guid;
           treeNode.childGUIDs.push(child.guid);
-        }
+        }, this.yieldState);
       }
     };
-    await traverse(clientTree, false);
+
+    await Async.yieldingForEach(clientTree, item => traverse(item, false), this.yieldState);
+
     clientTree.id = "places";
     await this._followQueries(recordsByQueryId);
     return records;
   }
 
   /**
    * Process the server-side list. Mainly this builds the records into a tree,
    * but it also records information about problems, and produces arrays of the
@@ -761,33 +760,32 @@ class BookmarkValidator {
       if (!record || record.parentid !== "places") {
         problemData.badClientRoots.push(rootGUID);
       }
     }
   }
 
   async _computeUnifiedRecordMap(serverRecords, clientRecords) {
     let allRecords = new Map();
-    for (let sr of serverRecords) {
-      await this.maybeYield();
+    await Async.yieldingForEach(serverRecords, sr => {
       if (sr.fake) {
-        continue;
+        return;
       }
       allRecords.set(sr.id, {client: null, server: sr});
-    }
+    }, this.yieldState);
 
-    for (let cr of clientRecords) {
-      await this.maybeYield();
+    await Async.yieldingForEach(clientRecords, cr => {
       let unified = allRecords.get(cr.id);
       if (!unified) {
         allRecords.set(cr.id, {client: cr, server: null});
       } else {
         unified.client = cr;
       }
-    }
+    }, this.yieldState);
+
     return allRecords;
   }
 
   _recordMissing(problems, id, clientRecord, serverRecord, serverTombstones) {
     if (!clientRecord && serverRecord) {
       problems.clientMissing.push(id);
     }
     if (!serverRecord && clientRecord) {
@@ -897,53 +895,53 @@ class BookmarkValidator {
     serverRecords = inspectionInfo.records;
     let problemData = inspectionInfo.problemData;
 
     await this._validateClient(problemData, clientRecords);
 
     let allRecords = await this._computeUnifiedRecordMap(serverRecords, clientRecords);
 
     let serverDeleted = new Set(inspectionInfo.deletedRecords.map(r => r.id));
-    for (let [id, {client, server}] of allRecords) {
-      await this.maybeYield();
+
+    await Async.yieldingForEach(allRecords, ([id, {client, server}]) => {
       if (!client || !server) {
         this._recordMissing(problemData, id, client, server, serverDeleted);
-        continue;
+        return;
       }
       if (server && client && client.ignored) {
         problemData.serverUnexpected.push(id);
       }
       let { differences, structuralDifferences } = this._compareRecords(client, server);
 
       if (differences.length) {
         problemData.differences.push({ id, differences });
       }
       if (structuralDifferences.length) {
         problemData.structuralDifferences.push({ id, differences: structuralDifferences });
       }
-    }
+    }, this.yieldState);
+
     return inspectionInfo;
   }
 
   async _getServerState(engine) {
     // XXXXX - todo - we need to capture last-modified of the server here and
     // ensure the repairer only applys with if-unmodified-since that date.
     let collection = engine.itemSource();
     let collectionKey = engine.service.collectionKeys.keyForCollection(engine.name);
     collection.full = true;
     let result = await collection.getBatched();
     if (!result.response.success) {
       throw result.response;
     }
     let cleartexts = [];
-    for (let record of result.records) {
-      await this.maybeYield();
+    await Async.yieldingForEach(result.records, async (record) => {
       await record.decrypt(collectionKey);
       cleartexts.push(record.cleartext);
-    }
+    }, this.yieldState);
     return cleartexts;
   }
 
   async validate(engine) {
     let start = Date.now();
     let clientTree = await PlacesUtils.promiseBookmarksTree("", {
       includeItemIds: true,
     });
--- a/services/sync/modules/collection_validator.js
+++ b/services/sync/modules/collection_validator.js
@@ -71,23 +71,23 @@ class CollectionValidator {
   async getServerItems(engine) {
     let collection = engine.itemSource();
     let collectionKey = engine.service.collectionKeys.keyForCollection(engine.name);
     collection.full = true;
     let result = await collection.getBatched();
     if (!result.response.success) {
       throw result.response;
     }
-    let maybeYield = Async.jankYielder();
     let cleartexts = [];
-    for (let record of result.records) {
-      await maybeYield();
+
+    await Async.yieldingForEach(result.records, async (record) => {
       await record.decrypt(collectionKey);
       cleartexts.push(record.cleartext);
-    }
+    });
+
     return cleartexts;
   }
 
   // Should return a promise that resolves to an array of client items.
   getClientItems() {
     return Promise.reject("Must implement");
   }
 
@@ -139,27 +139,29 @@ class CollectionValidator {
   }
 
   // Returns an object containing
   //   problemData: an instance of the class returned by emptyProblemData(),
   //   clientRecords: Normalized client records
   //   records: Normalized server records,
   //   deletedRecords: Array of ids that were marked as deleted by the server.
   async compareClientWithServer(clientItems, serverItems) {
-    let maybeYield = Async.jankYielder();
+    const yieldState = Async.yieldState();
+
     const clientRecords = [];
-    for (let item of clientItems) {
-      await maybeYield();
+
+    await Async.yieldingForEach(clientItems, item => {
       clientRecords.push(this.normalizeClientItem(item));
-    }
+    }, yieldState);
+
     const serverRecords = [];
-    for (let item of serverItems) {
-      await maybeYield();
+    await Async.yieldingForEach(serverItems, async (item) => {
       serverRecords.push((await this.normalizeServerItem(item)));
-    }
+    }, yieldState);
+
     let problems = this.emptyProblemData();
     let seenServer = new Map();
     let serverDeleted = new Set();
     let allRecords = new Map();
 
     for (let record of serverRecords) {
       let id = record[this.idProp];
       if (!id) {
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -303,35 +303,35 @@ Store.prototype = {
    * applyIncoming(). Store implementations may overwrite this function
    * if desired.
    *
    * @param  records Array of records to apply
    * @return Array of record IDs which did not apply cleanly
    */
   async applyIncomingBatch(records) {
     let failed = [];
-    let maybeYield = Async.jankYielder();
-    for (let record of records) {
-      await maybeYield();
+
+    await Async.yieldingForEach(records, async (record) => {
       try {
         await this.applyIncoming(record);
       } catch (ex) {
         if (ex.code == SyncEngine.prototype.eEngineAbortApplyIncoming) {
           // This kind of exception should have a 'cause' attribute, which is an
           // originating exception.
           // ex.cause will carry its stack with it when rethrown.
           throw ex.cause;
         }
         if (Async.isShutdownException(ex)) {
           throw ex;
         }
         this._log.warn("Failed to apply incoming record " + record.id, ex);
         failed.push(record.id);
       }
-    }
+    });
+
     return failed;
   },
 
   /**
    * Apply a single record against the store.
    *
    * This takes a single record and makes the local changes required so the
    * local state matches what's in the record.
@@ -1179,37 +1179,35 @@ SyncEngine.prototype = {
     // Stage 1: Fetch new records from the server, up to the download limit.
     if (this.lastModified == null || this.lastModified > lastSync) {
       let { response, records } = await newitems.getBatched(this.downloadBatchSize);
       if (!response.success) {
         response.failureCode = ENGINE_DOWNLOAD_FAIL;
         throw response;
       }
 
-      let maybeYield = Async.jankYielder();
-      for (let record of records) {
-        await maybeYield();
+      await Async.yieldingForEach(records, async (record) => {
         downloadedIDs.add(record.id);
 
         if (record.modified < oldestModified) {
           oldestModified = record.modified;
         }
 
         let { shouldApply, error } = await this._maybeReconcile(record);
         if (error) {
           failedInCurrentSync.add(record.id);
           count.failed++;
-          continue;
+          return;
         }
         if (!shouldApply) {
           count.reconciled++;
-          continue;
+          return;
         }
         recordsToApply.push(record);
-      }
+      });
 
       let failedToApply = await this._applyRecords(recordsToApply);
       Utils.setAddAll(failedInCurrentSync, failedToApply);
 
       // `applied` is a bit of a misnomer: it counts records that *should* be
       // applied, so it also includes records that we tried to apply and failed.
       // `recordsToApply.length - failedToApply.length` is the number of records
       // that we *successfully* applied.
@@ -1274,35 +1272,33 @@ SyncEngine.prototype = {
       backfilledItems.ids = ids;
 
       let {response, records} = await backfilledItems.getBatched(this.downloadBatchSize);
       if (!response.success) {
         response.failureCode = ENGINE_DOWNLOAD_FAIL;
         throw response;
       }
 
-      let maybeYield = Async.jankYielder();
       let backfilledRecordsToApply = [];
       let failedInBackfill = [];
 
-      for (let record of records) {
-        await maybeYield();
-
+      await Async.yieldingForEach(records, async (record) => {
         let { shouldApply, error } = await this._maybeReconcile(record);
         if (error) {
           failedInBackfill.push(record.id);
           count.failed++;
-          continue;
+          return;
         }
         if (!shouldApply) {
           count.reconciled++;
-          continue;
+          return;
         }
         backfilledRecordsToApply.push(record);
-      }
+      });
+
       let failedToApply = await this._applyRecords(backfilledRecordsToApply);
       failedInBackfill.push(...failedToApply);
 
       count.failed += failedToApply.length;
       count.applied += backfilledRecordsToApply.length;
 
       this.toFetch = Utils.setDeleteAll(this.toFetch, ids);
       this.previousFailed = Utils.setAddAll(this.previousFailed, failedInBackfill);
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -494,19 +494,17 @@ BookmarksEngine.prototype = {
     function* walkBookmarksRoots(tree) {
       for (let child of tree.children) {
         if (isSyncedRootNode(child)) {
           yield* Utils.walkTree(child, tree);
         }
       }
     }
 
-    let maybeYield = Async.jankYielder();
-    for (let [node, parent] of walkBookmarksRoots(tree)) {
-      await maybeYield();
+    await Async.yieldingForEach(walkBookmarksRoots(tree), ([node, parent]) => {
       let {guid, type: placeType} = node;
       guid = PlacesSyncUtils.bookmarks.guidToRecordId(guid);
       let key;
       switch (placeType) {
         case PlacesUtils.TYPE_X_MOZ_PLACE:
           // Bookmark
           key = "b" + node.uri + ":" + (node.title || "");
           break;
@@ -515,34 +513,34 @@ BookmarksEngine.prototype = {
           key = "f" + (node.title || "");
           break;
         case PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR:
           // Separator
           key = "s" + node.index;
           break;
         default:
           this._log.error("Unknown place type: '" + placeType + "'");
-          continue;
+          return;
       }
 
       let parentName = parent.title || "";
       if (guidMap[parentName] == null) {
         guidMap[parentName] = {};
       }
 
       // If the entry already exists, remember that there are explicit dupes.
       let entry = {
         guid,
         hasDupe: guidMap[parentName][key] != null,
       };
 
       // Remember this item's GUID for its parent-name/key pair.
       guidMap[parentName][key] = entry;
       this._log.trace("Mapped: " + [parentName, key, entry, entry.hasDupe]);
-    }
+    });
 
     return guidMap;
   },
 
   // Helper function to get a dupe GUID for an item.
   async _mapDupe(item) {
     // Figure out if we have something to key with.
     let key;
--- a/toolkit/components/places/SyncedBookmarksMirror.jsm
+++ b/toolkit/components/places/SyncedBookmarksMirror.jsm
@@ -122,19 +122,19 @@ const SQLITE_MAX_VARIABLE_NUMBER = 999;
 
 // The current mirror database schema version. Bump for migrations, then add
 // migration code to `migrateMirrorSchema`.
 const MIRROR_SCHEMA_VERSION = 4;
 
 const DEFAULT_MAX_FRECENCIES_TO_RECALCULATE = 400;
 
 // Use a shared jankYielder in these functions
-XPCOMUtils.defineLazyGetter(this, "maybeYield", () => Async.jankYielder());
+XPCOMUtils.defineLazyGetter(this, "yieldState", () => Async.yieldState());
 function yieldingIterator(collection) {
-  return Async.yieldingIterator(collection, maybeYield);
+  return Async.yieldingIterator(collection, yieldState);
 }
 
 /** Adapts a `Log.jsm` logger to a `mozISyncedBookmarksMirrorLogger`. */
 class MirrorLoggerAdapter {
   constructor(log) {
     this.log = log;
   }