From 35d31f23246161847c64a93967484a96960a17fe Mon Sep 17 00:00:00 2001
From: Ingvar Stepanyan <rreverser@google.com>
Date: Wed, 29 Apr 2020 16:03:04 +0100
Subject: [PATCH] Add some comments to explain Rust thread glue

---
 codecs/oxipng/spawn.ts        | 23 +++++++++++++++++++++++
 codecs/oxipng/src/parallel.rs | 26 ++++++++++++++++++++++++++
 codecs/oxipng/worker.ts       |  4 ++++
 3 files changed, 53 insertions(+)

diff --git a/codecs/oxipng/spawn.ts b/codecs/oxipng/spawn.ts
index bf62cf7b..3abecb97 100644
--- a/codecs/oxipng/spawn.ts
+++ b/codecs/oxipng/spawn.ts
@@ -15,11 +15,34 @@ function initWorker(worker: Worker, workerInit: WorkerInit) {
 
 async function startMainThread() {
   const num = navigator.hardwareConcurrency;
+
+  // First, let browser fetch and spawn Workers for our pool in the background.
+  // This is fairly expensive, so we want to start it as early as possible.
   const workers = Array.from({ length: num }, () => new Worker('./worker', { type: 'module' }));
+
+  // Meanwhile, asynchronously compile, instantiate and initialise Wasm on our main thread.
   await initOxiPNG(fetch(wasmUrl), undefined as any);
+
+  // Get module+memory from the Wasm instance.
+  //
+  // Ideally we wouldn't go via Wasm bindings here, since both are just JS variables, but memory is
+  // currently not exposed on the Wasm instance correctly by wasm-bindgen.
   const workerInit: WorkerInit = worker_initializer(num);
+
+  // Once done, we want to send module+memory to each Worker so that they instantiate Wasm too.
+  // While doing so, we need to wait for Workers to acknowledge that they have received our message.
+  // Ideally this shouldn't be necessary, but Chromium currently doesn't conform to the spec:
+  // https://bugs.chromium.org/p/chromium/issues/detail?id=1075645
+  //
+  // If we didn't do this ping-pong game, the `start_main_thread` below would block the current
+  // thread on an atomic before even *sending* the `postMessage` containing memory,
+  // so Workers would never be able to unblock us back.
   await Promise.all(workers.map(worker => initWorker(worker, workerInit)));
+
+  // Finally, instantiate rayon pool - this will use shared Wasm memory to send tasks to the
+  // Workers and then block until they're all ready.
   start_main_thread();
+
   return {
     optimise,
   };
diff --git a/codecs/oxipng/src/parallel.rs b/codecs/oxipng/src/parallel.rs
index e511f596..e5ced704 100644
--- a/codecs/oxipng/src/parallel.rs
+++ b/codecs/oxipng/src/parallel.rs
@@ -9,6 +9,32 @@ extern "C" {
     fn array_of_2(a: JsValue, b: JsValue) -> JsValue;
 }
 
+// This is one of the parts that work around Chromium incorrectly implementing postMessage:
+// https://bugs.chromium.org/p/chromium/issues/detail?id=1075645
+//
+// rayon::ThreadPoolBuilder (used below) executes spawn handler to populate the worker pool,
+// and then blocks the current thread until each worker unblocks its (opaque) lock.
+//
+// Normally, we could use postMessage directly inside the spawn handler to
+// post module + memory + threadPtr to each worker, and the block the current thread.
+//
+// However, that bug means that postMessage is currently delayed until the next event loop,
+// which will never spin since we block the current thread, and so the other workers will
+// never be able to unblock us.
+//
+// To work around this problem, we:
+// 1) Expose `worker_initializer` that returns module + memory pair (without threadPtr)
+//    that workers can be initialised with to become native threads.
+//    JavaScript can postMessage this pair in advance, and asynchronously wait for workers
+//    to acknowledge the receipt.
+// 2) Create a global communication channel on the Rust side using crossbeam.
+//    It will be used to send threadPtr to the pre-initialised workers
+//    instead of postMessage.
+// 3) Provide a separate `start_main_thread` that expects all workers to be ready,
+//    and just uses the provided channel to send `threadPtr`s using the
+//    shared memory and blocks the current thread until they're all grabbed.
+// 4) Provide a `worker_initializer` that is expected to be invoked from various workers,
+//    reads one `threadPtr` from the shared channel and starts running it.
 static CHANNEL: OnceCell<(Sender<rayon::ThreadBuilder>, Receiver<rayon::ThreadBuilder>)> = OnceCell::new();
 
 #[wasm_bindgen]
diff --git a/codecs/oxipng/worker.ts b/codecs/oxipng/worker.ts
index 9a08f4c9..69f60501 100644
--- a/codecs/oxipng/worker.ts
+++ b/codecs/oxipng/worker.ts
@@ -11,6 +11,10 @@ addEventListener(
     //
     // At this point, the "main" thread can run Wasm that
     // will synchronously block waiting on other atomics.
+    //
+    // Note that we don't need to wait for Wasm instantiation here - it's
+    // better to start main thread as early as possible, and then it blocks
+    // on a shared atomic anyway until Worker is fully ready.
     postMessage(null);
 
     await initOxiPNG(...(event.data as WorkerInit));