Skip to content

perf(ai): Run AI iterations using setImmediate#999

Merged
delucis merged 2 commits into
mainfrom
delucis/faster-ai-iterations
Sep 4, 2021
Merged

perf(ai): Run AI iterations using setImmediate#999
delucis merged 2 commits into
mainfrom
delucis/faster-ai-iterations

Conversation

@delucis
Copy link
Copy Markdown
Member

@delucis delucis commented Aug 30, 2021

Use setImmediate to run each async iteration of the MCTS bot. In most scenarios this will use a polyfill as setImmediate is not widely supported or on a web standards track. See https://github.com/YuzuJS/setImmediate for browser support details. This polyfill is pretty old and covers some use cases we don’t need — like Node < 0.9 (!!) — so maybe it would be preferable to roll our own smaller, simpler postMessage-based alternative? I tried this library as a proof of concept test, and it did seem to result in faster AI result times using the Tic-Tac-Toe example by maybe 30% (and actually more in later game stages with fewer game branches although at that point the perceived difference is not so significant).

Checklist

  • Use a separate branch in your local repo (not main).
  • Test coverage is 100% (or you have a story for why it's ok).

Use `setImmediate` to run each async iteration of the MCTS bot. In most scenarios this will use a polyfill as `setImmediate` is not widely supported or on a web standards track. See <https://github.com/YuzuJS/setImmediate> for browser support details.
@delucis delucis requested a review from nicolodavis August 30, 2021 15:45
@delucis
Copy link
Copy Markdown
Member Author

delucis commented Sep 1, 2021

Here’s a refactored version of that setImmediate polyfill which returns the implementation instead of attaching it to the global scope (and uses a more modern code style).

Code snippet
type TaskID = number;
type Task = () => void;

const win =
  typeof self !== 'undefined'
    ? self
    : typeof global !== 'undefined'
    ? global
    : this;
const isNode = {}.toString.call(win.process) === '[object process]';

/**
 * Task runner that will run queued tasks as soon as possible, but which won’t
 * block other high priority work (like the browser render pipeline).
 * Effectively an alternative to the poorly supported `setImmediate`.
 */
class NonblockingRunner {
  private nextID = 1;
  private tasks: Record<TaskID, Task> = {};
  private currentlyRunning = false;
  private run: (handle: TaskID) => void;

  constructor() {
    this.run = isNode
      ? // For Node environments without `setImmediate` (like JSDOM).
        this.nextTickImplementation()
      : NonblockingRunner.canUsePostMessage()
      ? // For non-IE10 modern browsers.
        this.postMessageImplementation()
      : // For older browsers.
        this.setTimeoutImplementation();
  }

  /** Register a task to run as soon as possible. */
  public enqueue(task: Task): TaskID {
    this.tasks[this.nextID] = task;
    this.run(this.nextID);
    return this.nextID++;
  }

  private runIfPresent(id: TaskID) {
    // From the spec: "Wait until any invocations of this algorithm started before this one have completed."
    // So if we're currently running a task, we'll need to delay this invocation.
    if (this.currentlyRunning) {
      // Delay by doing a setTimeout. setImmediate was tried instead, but in Firefox 7 it generated a
      // "too much recursion" error.
      setTimeout(this.runIfPresent, 0, id);
    } else {
      const task = this.tasks[id];
      if (task) {
        this.currentlyRunning = true;
        try {
          task();
        } finally {
          delete this.tasks[id];
          this.currentlyRunning = false;
        }
      }
    }
  }

  private setTimeoutImplementation() {
    return (id: TaskID) => setTimeout(() => this.runIfPresent(id), 0);
  }

  private nextTickImplementation() {
    return (id: TaskID) => process.nextTick(() => this.runIfPresent(id));
  }

  private postMessageImplementation() {
    // Installs an event handler on `global` for the `message` event: see
    // * https://developer.mozilla.org/en/DOM/window.postMessage
    // * http://www.whatwg.org/specs/web-apps/current-work/multipage/comms.html#crossDocumentMessages

    const messagePrefix = `setImmediate$${Math.random()}$`;
    const onGlobalMessage = ({ source, data }: MessageEvent) => {
      if (
        source === (win as any) &&
        typeof data === 'string' &&
        // Avoid `startsWith` for better backwards compatibility.
        data.indexOf(messagePrefix) === 0
      ) {
        const id = Number.parseInt(data.slice(messagePrefix.length), 10);
        this.runIfPresent(id);
      }
    };

    if (win.addEventListener) {
      win.addEventListener('message', onGlobalMessage, false);
    } else {
      (win as any).attachEvent('onmessage', onGlobalMessage);
    }

    return (id: TaskID) => win.postMessage(messagePrefix + id, '*');
  }

  private static canUsePostMessage() {
    // The test against `importScripts` prevents `postMessage` from being used inside a web worker,
    // where `postMessage` means something completely different and can't be used for this purpose.
    if (!win.postMessage || win.importScripts) return false;

    let postMessageIsAsynchronous = true;
    const oldOnMessage = win.onmessage;
    win.onmessage = () => {
      postMessageIsAsynchronous = false;
    };
    win.postMessage('', '*');
    win.onmessage = oldOnMessage;
    return postMessageIsAsynchronous;
  }
}

/** Global `NonblockingRunner` instance. */
const runner = new NonblockingRunner();

/** Run a task callback as soon as possible without blocking rendering. */
export const runWithoutBlocking =
  typeof setImmediate !== 'undefined'
    ? setImmediate
    : (task: Task) => runner.enqueue(task);

Which can then be used as you would setImmediate

runWithoutBlocking(() => {
  // next task step
});

There’s actually very little that can be removed from the existing polyfill. Even the nextTick implementation that is advertised as being needed for Node < 0.9 is actually still needed, because in the JSDOM environment Jest uses, setImmediate is disabled for consistency with most browsers, but the browser workarounds like postMessage don’t work. So the trade-offs here would mainly be: not polluting the global namespace vs using our own fork which would be hard to test in all its use scenarios.

I’d vote for using the existing polyfill.

@vdfdev
Copy link
Copy Markdown
Contributor

vdfdev commented Sep 2, 2021

This is great! AI performance is something that needs improvement indeed!

@vdfdev
Copy link
Copy Markdown
Contributor

vdfdev commented Sep 2, 2021

How big is the polyfill?

@vdfdev
Copy link
Copy Markdown
Contributor

vdfdev commented Sep 2, 2021

+1 on using the polyfill

@delucis
Copy link
Copy Markdown
Member Author

delucis commented Sep 2, 2021

How big is the polyfill?

~1k gzipped — https://bundlephobia.com/package/[email protected]

@delucis
Copy link
Copy Markdown
Member Author

delucis commented Sep 2, 2021

Regarding performance gains, it is worth noting that Tic-Tac-Toe’s logic is on just about the simplest possible end of things, which means that the iteration overhead is going to be a more significant chunk of work, so gains from this change are probably maximised. I’d imagine that the more complex the game, the less of an impact this will have. Also I measured those performance changes in Firefox & Chrome (latest) on a Mac. The benefit in Safari seemed to be smaller. Didn’t try testing in other environments.

(My intuitions may also be wrong here. One other thing I noticed was that the current setTimeout( ..., 0) approach produced more inconsistent timings, presumably because more often something else would get in the way of the iteration task, so perhaps the benefits of more consistent chunk scheduling hold up even for more complex games.)

@delucis delucis merged commit ad78ede into main Sep 4, 2021
@delucis delucis deleted the delucis/faster-ai-iterations branch September 4, 2021 14:46
@delucis
Copy link
Copy Markdown
Member Author

delucis commented Sep 4, 2021

My testing suggests this is an improvement and I want to get this into a patch release before merging #985, so merging without review. Still be curious if this matched up to Nicolo’s original idea.

Copy link
Copy Markdown
Member

@nicolodavis nicolodavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @delucis, this is what I had in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants