Skip to content

feat(assets): throttle concurrent asset requests#8929

Merged
mvaligursky merged 1 commit into
mainfrom
mv-asset-request-throttle
Jun 19, 2026
Merged

feat(assets): throttle concurrent asset requests#8929
mvaligursky merged 1 commit into
mainfrom
mv-asset-request-throttle

Conversation

@mvaligursky

Copy link
Copy Markdown
Contributor

Adds a limit on the number of concurrent asset requests, so large preloads no longer fail with net::ERR_INSUFFICIENT_RESOURCES — browsers reject requests once too many are in flight at once (empirically reproducible well below the asset counts some projects preload). Fixes #5258.

Changes:

  • Throttle requests at the HTTP layer — the single choke point all resource handlers go through — so it applies transparently to every asset type with no changes to the registry, handlers, or preload. Requests beyond the limit are queued and dispatched as earlier ones complete.
  • The XMLHttpRequest is still created and returned synchronously; only the .send() call is deferred, so progress/abort behaviour is unchanged.
  • Default limit is 128; 0 disables throttling (previous behaviour). It is a process-global limit, matching the browser's per-process resource limit, and covers all XHR-based loads (the large majority of assets; the <img>-element texture fallback, <audio>, <script> and bundle fetch paths are browser-managed and not affected).
  • Editor projects can set it via a new maxConcurrentRequests application property, parsed alongside the existing maxAssetRetries.

API Changes:

  • Add app.loader.maxConcurrentRequests (getter/setter) to read or override the limit at runtime:
    app.loader.maxConcurrentRequests = 50; // at most 50 requests in flight at once
    app.loader.maxConcurrentRequests = 0;  // disable throttling

Related:

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown

Public API report

This PR changes the public API surface (+2 / −0), per the docs' rules (@ignore / @Private / undocumented are excluded).

Show API diff
+ResourceLoader.get maxConcurrentRequests(): number
+ResourceLoader.set maxConcurrentRequests(value: number)

Informational only — this never fails the build.

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown

Build size report

This PR changes the size of the minified bundles.

Bundle Minified Gzip Brotli
playcanvas.min.js 2277.1 KB (+1.5 KB, +0.06%) 584.7 KB (+0.3 KB, +0.06%) 454.8 KB (+0.6 KB, +0.14%)
playcanvas.min.mjs 2274.5 KB (+1.5 KB, +0.06%) 583.8 KB (+0.4 KB, +0.07%) 454.0 KB (+0.4 KB, +0.08%)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a concurrency throttle for XHR-based asset/network requests to prevent browsers from rejecting large bursts of simultaneous requests (e.g. net::ERR_INSUFFICIENT_RESOURCES). It implements throttling at the shared HTTP layer and exposes configuration via app.loader.maxConcurrentRequests and an application property.

Changes:

  • Add a max-concurrency request throttle in Http, queuing excess requests and dispatching them as in-flight requests complete.
  • Expose the throttle as ResourceLoader#maxConcurrentRequests, and allow configuration via application_properties.maxConcurrentRequests.
  • Add unit tests covering queuing/dispatch behavior and the “0 = unlimited” mode.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
src/platform/net/http.js Implements request slot accounting, queueing, and pumping logic for concurrent XHR throttling.
src/framework/handlers/loader.js Adds app.loader.maxConcurrentRequests getter/setter to control the shared HTTP throttle.
src/framework/app-base.js Parses maxConcurrentRequests from application properties and applies it to the loader.
test/platform/net/http.test.mjs Adds tests validating request throttling, unlimited mode, and dynamic limit changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/platform/net/http.js
Comment thread src/platform/net/http.js
Comment thread src/platform/net/http.js Outdated
Comment thread src/platform/net/http.js Outdated
Comment thread src/framework/handlers/loader.js
Browsers reject requests with net::ERR_INSUFFICIENT_RESOURCES once too many
are in flight at once. The engine previously fired every asset request
immediately with no cap, so large preloads could fail.

Add a concurrency limit at the HTTP layer - the single choke point all
resource handlers go through. Requests beyond the limit are queued and
dispatched as earlier ones complete. The XHR is still created and returned
synchronously; only the .send() call is deferred, so progress/abort
behaviour is unchanged.

- Http.maxConcurrentRequests (internal): default 128, 0 = unlimited
- ResourceLoader#maxConcurrentRequests: public getter/setter (app.loader)
- maxConcurrentRequests application property parsed in app-base, mirroring
  the existing maxAssetRetries
@mvaligursky

Copy link
Copy Markdown
Contributor Author

Thanks for the review — addressed all five points in 50f2efc2e:

  1. Per-request slot state stored on options → moved to the per-request xhr (xhr._slotHeld). Reusing an options object across requests (the retry: false path, which isn't cloned) can no longer corrupt accounting. Added a regression test that fires 4 concurrent requests sharing a single options object and asserts _activeRequests returns cleanly to 0.
  2. Unconditional options.error(...) in the send-catch → guarded with typeof options.error === 'function'.
  3. _activeRequests doc → clarified it only counts slot-accounted (throttled async) requests and stays 0 when throttling is disabled.
  4. _pump() O(n²) Array#shift → now dequeues via a _sendQueueHead index with periodic compaction, so draining a large queue is O(n).
  5. Setter validationResourceLoader#maxConcurrentRequests clamps to a non-negative integer (Math.max(0, Math.floor(value)) || 0), with Infinity still supported, matching enableRetry().

Full test suite passes (1850) including the new per-request slot-tracking test.

@copilot review

Copilot AI commented Jun 19, 2026

Copy link
Copy Markdown

Thanks for the review — addressed all five points in 50f2efc2e:

  1. Per-request slot state stored on options → moved to the per-request xhr (xhr._slotHeld). Reusing an options object...

All five fixes look good. The tests pass (4/4) and lint is clean.

A few minor observations on the current state:

  • _acquire / _releaseSlot now correctly track slot state on xhr._slotHeld rather than options. The shared-options regression test confirms no leak.
  • The send catch block is correctly guarded with typeof options.error === 'function'.
  • _activeRequests docs clearly reflect that only throttled async requests are counted.
  • _pump uses _sendQueueHead + periodic compaction — O(n) overall. One small fragility: afterEach resets http._sendQueue.length = 0 but not http._sendQueueHead. In the current tests this is harmless because every test that queues requests also drains the queue fully (triggering the _sendQueueHead = 0 compaction path), but explicitly resetting it in afterEach (and beforeEach) would make the test harness more robust against future additions.
  • The setter clamping (Math.max(0, Math.floor(value)) || 0) correctly handles NaN, negatives, fractional values, and preserves Infinity.

@mvaligursky mvaligursky requested a review from Copilot June 19, 2026 11:18
@mvaligursky mvaligursky merged commit 6b14aae into main Jun 19, 2026
12 checks passed
@mvaligursky mvaligursky deleted the mv-asset-request-throttle branch June 19, 2026 11:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment on lines +165 to +168
// start each test from a known, clean throttle state on the shared singleton
http.maxConcurrentRequests = 128;
http._activeRequests = 0;
http._sendQueue.length = 0;
Comment on lines +173 to +176
// restore defaults so state doesn't leak into other test files
http.maxConcurrentRequests = 128;
http._activeRequests = 0;
http._sendQueue.length = 0;
Comment thread src/platform/net/http.js
Comment on lines +680 to +693
if (!throttled) {
// unthrottled (0 or Infinity): send everything immediately, with no slot accounting
while (this._sendQueueHead < this._sendQueue.length) {
this._sendQueue[this._sendQueueHead++].send();
}
} else {
// throttled: keep the number of in-flight requests under the limit
while (this._sendQueueHead < this._sendQueue.length && this._activeRequests < limit) {
const { xhr, send } = this._sendQueue[this._sendQueueHead++];
this._activeRequests++;
xhr._slotHeld = true;
send();
}
}
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.

Throttle large number of assets requests?

3 participants