perf: fetch the Genesis world manifest concurrently with realm /about - #9540
perf: fetch the Genesis world manifest concurrently with realm /about#9540eordano wants to merge 2 commits into
Conversation
The Genesis manifest URL is a compile-time constant, so the fetch does not need realmName from the /about response. Start it alongside that request and have the Genesis branch await the same task, keeping the manifest off the boot critical path. World realms are unaffected: their manifest URL embeds realmName, so they still resolve it from the response. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AwLgyC5zggzth1oggwnBd4
decentraland-bot
added
the
ext-contribution
|
Build skipped — no changes detected under |
|
Lint did not finish ( |
|
Slack notification sent to #explorer-ext-contributions for external review. |
|
|
Local scene development targets a local realm, so the prefetch would reach an external host for a manifest that entrypoint never consumes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AwLgyC5zggzth1oggwnBd4
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
PR Review: perf: fetch the Genesis world manifest concurrently with realm /about
STEP 2 — Root-cause check: PASS
The PR correctly identifies that genesis manifest URLs are compile-time constants and don't depend on realmName from the /about response. Parallelizing the fetch addresses the real cause of the boot-path latency.
STEP 3 — Design & integration: PASS
No new long-lived unit is introduced. inFlightMainManifest and PrefetchGenesisManifest are added to the existing WorldManifestProvider (constructed in StaticContainer.cs:350, injected into RealmController). The prefetch responsibility belongs here — WorldManifestProvider already owns FetchGenesisManifestAsync and cachedMainManifest.
Lifecycle owners searched: StaticContainer creates WorldManifestProvider (line 350). RealmController consumes it (constructor injection, line 104). RealmContainer wires them together (line 79). No other owner manages the genesis manifest fetch.
Teardown / consumption trace: inFlightMainManifest is written in PrefetchGenesisManifest (line 36) and consumed via await in FetchWorldManifestAsync (line 50). No subscription/event/connection is opened — no teardown needed. The Preserve()d UniTask is a value type holding a reference to the underlying completion source; it doesn't need disposal.
STEP 4 — Member audit
PrefetchGenesisManifest(public): 2 consumers —RealmController.SetRealmAsync(line 138) andWorldManifestProvider.FetchWorldManifestAsync(line 49). Meaningful for both sites; not single-use.inFlightMainManifest(private field): written inPrefetchGenesisManifest, read inFetchWorldManifestAsync. 2 access points. Correctly scoped as private.
STEP 5 — Line-level findings
See inline comments (1× P1, 2× P2).
Security review
No security issues found. The change only reorders an existing HTTP fetch to a hardcoded S3 URL. No new user input handling, auth changes, secrets, or injection surfaces.
STEP 8 — Non-blocking warnings
None. Main.unity is not in the changed files.
REVIEW_RESULT: FAIL ❌
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Touches realm infrastructure (WorldManifestProvider async flow) but no ECS systems, components, queries, or structural changes.
QA_REQUIRED: YES
Reviewed by Jarvis 🤖 · Requested via Slack
| public void PrefetchGenesisManifest(DecentralandEnvironment environment, CancellationToken ct) => | ||
| inFlightMainManifest ??= FetchGenesisManifestAsync(environment, ct).Preserve(); |
There was a problem hiding this comment.
[P1] Stale task after cancellation poisons genesis manifest for the rest of the session.
??= never retries once inFlightMainManifest is set. If the CancellationToken passed to SetRealmAsync fires before the HTTP request completes, FetchGenesisManifestAsync catches OperationCanceledException and returns WorldManifest.Empty. The UniTask completes successfully (status Succeeded) with Empty, cachedMainManifest stays null, and ??= skips every subsequent call — the genesis manifest remains empty for the rest of the session with no recovery path.
This is especially insidious because PrefetchGenesisManifest is called unconditionally in SetRealmAsync (even for world realms). If a world-realm connection is cancelled, the genesis manifest prefetch is also cancelled via the shared ct, poisoning future genesis connections the user never navigated to.
Fix: Guard on cache hit and pending status instead of ??=, so a cancelled/failed fetch is retried with a fresh token.
| public void PrefetchGenesisManifest(DecentralandEnvironment environment, CancellationToken ct) => | |
| inFlightMainManifest ??= FetchGenesisManifestAsync(environment, ct).Preserve(); | |
| public void PrefetchGenesisManifest(DecentralandEnvironment environment, CancellationToken ct) | |
| { | |
| if (cachedMainManifest.HasValue) return; | |
| if (inFlightMainManifest.HasValue && inFlightMainManifest.Value.Status == UniTaskStatus.Pending) | |
| return; | |
| inFlightMainManifest = FetchGenesisManifestAsync(environment, ct).Preserve(); | |
| } |
| // Genesis manifest URLs are compile-time constants, so this runs concurrently with | ||
| // the realm /about request instead of waiting on realmName. Idempotent; Preserve() | ||
| // lets the Genesis branch await this same task rather than issue a second request. |
There was a problem hiding this comment.
[P2] Comment narrates caller/external behavior (CLAUDE.md §11 anti-patterns). "runs concurrently with the realm /about request" and "lets the Genesis branch await this same task" describe what callers do, not what this method guarantees. The caller pattern can change without this code changing, silently making the comment wrong.
| // Genesis manifest URLs are compile-time constants, so this runs concurrently with | |
| // the realm /about request instead of waiting on realmName. Idempotent; Preserve() | |
| // lets the Genesis branch await this same task rather than issue a second request. | |
| // Idempotent while a fetch is pending; Preserve()d for multi-await. |
| return await FetchGenesisManifestAsync(environment, ct); | ||
| { | ||
| PrefetchGenesisManifest(environment, ct); | ||
| return await inFlightMainManifest!.Value; |
There was a problem hiding this comment.
[P2] Null-forgiving operator ! without justifying comment (CLAUDE.md nullable reference types). The ! is correct — PrefetchGenesisManifest guarantees inFlightMainManifest is non-null on return — but the rule requires a comment when using !.
| return await inFlightMainManifest!.Value; | |
| return await inFlightMainManifest!.Value; // set by PrefetchGenesisManifest above |
2 participants
The Genesis manifest URL is a compile-time constant, so the fetch does not need
realmNamefrom the/aboutresponse. It now starts alongside that request, and the Genesis branch awaits the same task — keeping the manifest off the boot critical path.PrefetchGenesisManifestis idempotent (??=), and the task isPreserve()d so the Genesis branch can await it rather than issue a second request.FetchGenesisManifestAsyncalready funnels every exception intoWorldManifest.Empty, so a failed prefetch cannot fail the boot.Scope and cost
The prefetch starts before the realm type is known, so it is speculative:
*.dcl.eth) and custom realms — resolve their manifest fromrealmNameas before, and additionally pay one discarded Genesis fetch, once per provider lifetime. It is async and fail-soft, but it is network the entrypoint does not consume.SetRealmAsyncalso runs on realm changes, not only at boot;??=bounds the speculative fetch to one per session.🤖 Generated with Claude Code
https://claude.ai/code/session_01AwLgyC5zggzth1oggwnBd4