Global throttling

.github/workflows/ci-cd.yml

@@ -37,12 +37,19 @@ jobs:
                   GitHub head ref: ${{ github.head_ref }}
                   EOF
 
-            - name: Setup & Test
+            - run: npm ci
+
+            - name: Pre-build tests
               run: |
-                  npm ci
-                  npm run build
+                  npm run clean
                   npm run test
 
+            - name: Build
+              run: npm run build
+
+            - name: Post-build tests
+              run: npm run test:build
+
             - name: Semantic release (configured to run dry if branch is other than 'develop')
               env:
                   NPM_TOKEN: ${{ secrets.NPM_TOKEN }}

package.json

@@ -13,12 +13,14 @@
   "browser": "./dist/web/scratch-storage.js",
   "types": "./dist/types/index.d.ts",
   "scripts": {
-    "build": "rimraf dist && webpack --mode=production",
+    "build": "npm run clean && webpack --mode=production",
+    "clean": "rimraf dist",
     "commitmsg": "commitlint -e $GIT_PARAMS",
     "coverage": "tap ./test/{unit,integration}/*.js --coverage --coverage-report=lcov",
     "prepare": "husky",
     "semantic-release": "semantic-release",
     "test": "npm run test:lint && jest \"test[\\\\/](unit|integration)\"",
+    "test:build": "jest \"test[\\\\/]build\"",
     "test:clearCache": "jest --clearCache",
     "test:integration": "jest \"test[\\\\/]integration\"",
     "test:lint": "eslint",
@@ -34,7 +36,6 @@
     "arraybuffer-loader": "^1.0.3",
     "base64-js": "^1.3.0",
     "buffer": "6.0.3",
-    "cross-fetch": "^4.1.0",
     "fastestsmallesttextencoderdecoder": "^1.0.7",
     "js-md5": "^0.7.3",
     "minilog": "^3.1.0"

src/FetchTool.ts

@@ -1,3 +1,4 @@
+import {AssetQueueOptions} from './HostQueues';
 import {scratchFetch} from './scratchFetch';
 import {ScratchGetRequest, ScratchSendRequest, Tool} from './Tool';
 
@@ -24,7 +25,7 @@ export class FetchTool implements Tool {
      * @returns {Promise.<Uint8Array?>} Resolve to Buffer of data from server.
      */
     get ({url, ...options}: ScratchGetRequest): Promise<Uint8Array | null> {
-        return scratchFetch(url, Object.assign({method: 'GET'}, options))
+        return scratchFetch(url, Object.assign({method: 'GET'}, options), {queueOptions: AssetQueueOptions})
             .then((result: Response) => {
                 if (result.ok) return result.arrayBuffer().then(b => new Uint8Array(b));
                 if (result.status === 404) return null;
@@ -49,7 +50,7 @@ export class FetchTool implements Tool {
     send ({url, withCredentials = false, ...options}: ScratchSendRequest): Promise<string> {
         return scratchFetch(url, Object.assign({
             credentials: withCredentials ? 'include' : 'omit'
-        }, options))
+        }, options), {queueOptions: AssetQueueOptions})
             .then(response => {
                 if (response.ok) return response.text();
                 return Promise.reject(response.status);

src/FetchWorkerTool.worker.ts

@@ -0,0 +1,94 @@
+/* eslint-env worker */
+/* eslint-disable-next-line spaced-comment */
+/// <reference lib="webworker" />
+
+// This worker won't share the same queue as the main thread, but throttling should be okay
+// as long as we don't use FetchTool and FetchWorkerTool at the same time.
+// TODO: Communicate metadata from the main thread to workers or move the worker boundary "into" `scratchFetch`.
+// Make sure to benchmark any changes to avoid performance regressions, especially for large project loads.
+import {AssetQueueOptions} from './HostQueues';
+import {scratchFetch} from './scratchFetch';
+
+interface JobMessage {
+    id: string,
+    url: string;
+    options: RequestInit | undefined;
+}
+
+interface CompletionMessage {
+    id: string,
+    buffer?: ArrayBuffer | null;
+    error?: string;
+}
+
+let jobsActive = 0;
+const complete: CompletionMessage[] = [];
+
+let intervalId: ReturnType<typeof setInterval> | undefined = void 0;
+
+/**
+ * Register a step function.
+ *
+ * Step checks if there are completed jobs and if there are sends them to the
+ * parent. Then it checks the jobs count. If there are no further jobs, clear
+ * the step.
+ */
+const registerStep = function () {
+    intervalId = setInterval(() => {
+        if (complete.length) {
+            // Send our chunk of completed requests and instruct postMessage to
+            // transfer the buffers instead of copying them.
+            postMessage(
+                complete.slice(),
+                // Instruct postMessage that these buffers in the sent message
+                // should use their Transferable trait. After the postMessage
+                // call the "buffers" will still be in complete if you looked,
+                // but they will all be length 0 as the data they reference has
+                // been sent to the window. This lets us send a lot of data
+                // without the normal postMessage behaviour of making a copy of
+                // all of the data for the window.
+                complete.map(response => response.buffer).filter(Boolean) as Transferable[]
+            );
+            complete.length = 0;
+        }
+        if (jobsActive === 0) {
+            clearInterval(intervalId);
+            intervalId = void 0;
+        }
+    }, 1);
+};
+
+/**
+ * Receive a job from the parent and fetch the requested data.
+ * @param message The message from the parent.
+ * @param message.data A job id, url, and options descriptor to perform.
+ */
+const onMessage = async ({data: job}: MessageEvent<JobMessage>) => {
+    if (jobsActive === 0 && !intervalId) {
+        registerStep();
+    }
+
+    jobsActive++;
+
+    try {
+        const response = await scratchFetch(job.url, job.options, {queueOptions: AssetQueueOptions});
+
+        const result:CompletionMessage = {id: job.id};
+        if (response.ok) {
+            result.buffer = await response.arrayBuffer();
+        } else if (response.status === 404) {
+            result.buffer = null;
+        } else {
+            throw response.status;
+        }
+        complete.push(result);
+    } catch (error) {
+        complete.push({id: job.id, error: ((error as Error)?.message) || `Failed request: ${job.url}`});
+    } finally {
+        jobsActive--;
+    }
+};
+
+// "fetch" is supported in Node.js as of 16.15 and our target browsers as of ~2017
+postMessage({support: {fetch: true}});
+self.addEventListener('message', onMessage);

src/HostQueues.ts

@@ -0,0 +1,33 @@
+import {QueueManager, type QueueOptions} from '@scratch/task-herder';
+
+/**
+ * @summary A set of generous limits, for things like downloading assets from CDN.
+ * @description
+ * In practice, these limits seem to lead to slightly better performance than no limits at all, mostly due to the
+ * concurrency limit. For example, on my development computer & my relatively fast residential connection, a
+ * concurrency limit of 4 loads a particular test project in 21 seconds, as opposed to 25 seconds when I bypass the
+ * queue and call `fetch` directly. In that test, my setup downloads about 50 assets per second, so this set of options
+ * only affects concurrency and doesn't actually throttle the downloads. Limiting concurrency also fixes the issue
+ * where very large projects (thousands of assets) can lead to browser failures like `net::ERR_INSUFFICIENT_RESOURCES`.
+ * The exact concurrency limit doesn't seem to matter much since the browser limits parallel connections itself. It
+ * just needs to be high enough to avoid bubbles in the download pipeline and low enough to avoid resource exhaustion.
+ * @see {@link https://github.com/scratchfoundation/scratch-gui/issues/7111}
+ */
+export const AssetQueueOptions: QueueOptions = {
+    burstLimit: 64,
+    sustainRate: 64,
+    // WARNING: asset download concurrency >=5 can lead to corrupted buffers on Chrome (December 2025, Chrome 142.0)
+    // when using Scratch's bitmap load pipeline. Marking the canvas context as `{willReadFrequently: true}` seems to
+    // eliminate that issue, so maybe the problem is related to hardware acceleration.
+    concurrency: 64
+};
+
+/**
+ * Central registry of per-host queues.
+ * Uses strict limits by default. Override these strict limits as needed for specific hosts.
+ */
+export const hostQueueManager = new QueueManager({
+    burstLimit: 5,
+    sustainRate: 1,
+    concurrency: 1
+});

src/ScratchStorage.ts

@@ -6,7 +6,7 @@
 import _Asset, {AssetData, AssetId} from './Asset';
 import {AssetType as _AssetType, AssetType} from './AssetType';
 import {DataFormat as _DataFormat, DataFormat} from './DataFormat';
-import _scratchFetch from './scratchFetch';
+import * as _scratchFetch from './scratchFetch';
 import Helper from './Helper';
 
 interface HelperWithPriority {
@@ -130,7 +130,7 @@
      * @param {DataFormat} dataFormat - The dataFormat of the data for the cached asset.
      * @param {Buffer} data - The data for the cached asset.
      * @param {string} [id] - The id for the cached asset.
      * @param {bool} [generateId] - flag to set id to an md5 hash of data if `id` isn't supplied
      * @returns {Asset} generated Asset with `id` attribute set if not supplied
      */
     createAsset (
@@ -171,7 +171,7 @@
         this.addWebStore(types, urlFunction);
     }
 
     /**
      * TODO: Should this be removed in favor of requesting an asset with `null` as the ID?
      * @param {AssetType} type - Get the default ID for assets of this type.
      * @returns {?string} The ID of the default asset of the given type, if any.