Yield with MessageChannel instead of setTimeout in async mode

release-notes.md

@@ -10,6 +10,7 @@
   Besides preventing jsdiff from emitting patches that jsdiff itself cannot parse, other compatibility impacts should be fairly limited - essentially only occurring when one generates patches using `includeIndex: true` but `includeFileHeaders: false` (since virtually every tool that works with patches will prefer to use the `---`/`+++` file headers to determine the filenames of files to patch, rather than the `Index:` header. To the extent that this change affects compatibility at all, it should mostly be beneficial; GNU `patch` attempts to unquote quoted index headers if it sees them, so this change makes jsdiff's output more compatible with GNU `patch`.
 - [#707](https://github.com/kpdecker/jsdiff/pull/707) **objects with `toJSON` methods that return arrays are supported properly in `diffJson` now**. (Previously, these would be serialized as objects with numeric keys instead of arrays.)
 - [#712](https://github.com/kpdecker/jsdiff/pull/712) **Big performance improvement.** The core diffing loop indexed its array of furthest-reaching paths by diagonal number, and half of the diagonal numbers are negative. JavaScript engines store negative array indices as ordinary named properties rather than as array elements, so every lookup in the innermost loop was a slow dictionary access. The indices are now shifted to be non-negative. Results are unchanged; most non-trivial diffs now seem to be about 4x faster.
+- [#710](https://github.com/kpdecker/jsdiff/pull/710) **async mode is now nearly as fast as sync mode**. Async mode works by breaking the diff computation into small steps, with each step enqueuing the next; this enqueuing was originally done using `setTimeout`. However, nested `setTimeout` timeouts are "clamped" to a minimum delay of 4ms (see e.g. the "Timers" section of the HTML spec, as well as https://stackoverflow.com/a/9647221/1709587, https://stackoverflow.com/q/18826570/1709587, and https://www.belshe.com/2010/06/04/chrome-cranking-up-the-clock/ for some relevant discussion). This meant async mode on a long diff was spending more time waiting for the next step to begin running than it was spending actually computing the diff. This is now fixed and async mode is almost as fast as sync mode.
 
 ## 9.0.0
 

src/diff/base.ts

@@ -1,4 +1,5 @@
 import type {ChangeObject, AllDiffOptions, AbortableDiffOptions, DiffCallbackNonabortable, CallbackOptionAbortable, CallbackOptionNonabortable, DiffCallbackAbortable, TimeoutOption, MaxEditLengthOption} from '../types.js';
+import {enqueueTask} from '../util/yield.js';
 
 /**
  * Like a ChangeObject, but with no value and an extra `previousComponent` property.
@@ -83,7 +84,7 @@ export default class Diff<
     const done = (value: ChangeObject<ValueT>[]) => {
       value = this.postProcess(value, options);
       if (callback) {
-        setTimeout(function() { callback(value); }, 0);
+        enqueueTask(function() { callback(value); });
         return undefined;
       } else {
         return value;
@@ -210,15 +211,15 @@ export default class Diff<
     // in which case it will return undefined.
     if (callback) {
       (function exec() {
-        setTimeout(function() {
+        enqueueTask(function() {
           if (editLength > maxEditLength || Date.now() > abortAfterTimestamp) {
             return (callback as DiffCallbackAbortable<ValueT>)(undefined);
           }
 
           if (!execEditLength()) {
             exec();
           }
-        }, 0);
+        });
       }());
     } else {
       while (editLength <= maxEditLength && Date.now() <= abortAfterTimestamp) {

src/util/yield.ts

@@ -0,0 +1,26 @@
+let enqueueTaskCalls = 0;
+
+// Adds `callback` to the event queue as a macrotask, much like setTimeout(callback, 0) would.
+// Unlike setTimeout(callback, 0), this MessageChannel-based approach isn't delayed by timer
+// clamping (at least 4ms per nested timer in browsers, 1ms per timer in Node). We want to avoid
+// timer clamping because it would make jsdiff's async mode much much slower than sync mode for
+// no good reason.
+export function enqueueTask(callback: () => void): void {
+  // The occasional setTimeout calls are a compromise to mitigate
+  // https://bugs.webkit.org/show_bug.cgi?id=327531.
+  // Without this workaround, async mode would be effectively sync inside a web worker in Safari.
+  // Instead, we very occasionally yield just so that really long-running async diffs will
+  // occasionally yield in Safari, while still eliminating 90% of the clamping in all environments.
+  if (++enqueueTaskCalls % 10 == 0) {
+    setTimeout(callback, 0);
+    return;
+  }
+
+  const {port1, port2} = new MessageChannel();
+  port1.onmessage = () => {
+    // An open port would keep a Node process alive
+    port1.close();
+    callback();
+  };
+  port2.postMessage(undefined);
+}

test/diff/character.js

@@ -84,10 +84,37 @@ describe('diff/character', function() {
       diffChars('whatever', 'whatever', {ignoreCase: true});
       diffChars('whatever', 'whatever', {ignoreCase: true, callback: () => {}});
     });
+  });
 
+  describe('async mode', function() {
     it('should return undefined when called in async mode', function() {
       expect(diffChars('whatever', 'whatever', {callback: () => {}})).to.be.undefined;
       expect(diffChars('whatever', 'whatever else', {callback: () => {}})).to.be.undefined;
     });
+
+    it('should not wait for a timer between iterations in async mode', function(done) {
+      // 1000 insertions take 1000 iterations. Waiting for setTimeout between them used to take
+      // at least 1s in Node and 4s in browsers, due to timer clamping.
+      const start = Date.now();
+      diffChars('', 'a'.repeat(1000), function(diffResult) {
+        expect(diffResult).to.have.length(1);
+        expect(Date.now() - start).to.be.below(500);
+        done();
+      });
+    });
+
+    it('does not starve timers while it runs', function(done) {
+      // Confirm that we're actually yielding control back to the event loop in a way that allows
+      // other stuff to run - that is after all the point of async mode!
+      const oldText = 'ax'.repeat(1000);
+      const newText = 'ay'.repeat(1000);
+      let ticks = 0;
+      const interval = setInterval(() => { ticks++; }, 1);
+      diffChars(oldText, newText, {callback: () => {
+        clearInterval(interval);
+          expect(ticks).to.be.at.least(100);
+          done();
+      }});
+    });
   });
 });