chore: opti: perf-hunt roll-up (#9693 #9697 #9700 #9701 #9702 #9703 #9705 #9706) - #9707
Merged
Conversation
… bone count BoneMatrixCalculationJob recomputed all MAX_BONE_COUNT matrices per avatar every frame regardless of how many ComputeSkinning actually uploads; refresh and pass through the authoritative per-avatar count so the job only computes the uploaded range.
Transform.SetPositionAndRotation is world-authoritative, so the resulting world pose equals the arguments passed in; skip the native position/rotation readback and assign the cache from them directly.
…stem RenderGroundInternal allocated and disposed a NativeArray<int> and a NativeList<Matrix4x4> with Allocator.TempJob every frame the ground was visible; hold them as Allocator.Persistent fields instead, clearing them each frame and disposing once in OnDispose.
…cally ReceiveData rented exactly PartialData.Length + dataLength and copied the full old buffer on every regrow, causing O(N^2) rent+memcpy churn over a chunked download. Pre-size the buffer from the Content-Length header when available, and otherwise double the buffer on regrow, copying only the bytes written so far. Exercising the growth path requires driving DownloadHandlerScript's native callbacks, which has no stable unit-test seam in headless batch mode.
RemoteEntitiesExtensions.Remove ran every frame and unconditionally built an OwnedBunch<RemoveIntention>, whose ctor acquires MutexSync even when there is nothing to remove. Add a racy, lock-free NewBunchAvailable() pre-check (mirrors RemoteProfiles.NewBunchAvailable()) so the lock is skipped entirely on empty frames.
A cached absolute point-at world hit-point survived a teleport or same-realm scene reload, leaving an avatar's arm pinned at a stale position. Reset the point-at target from both the PlayerTeleportIntent path and the scene-reload path (ECSReloadScene, both overloads). Verified on real Unity 6000.4.0f1 (compile + EditMode/PlayMode regression tests).
…line query GetAvatarsVisibleWithOutline recalculated the camera's frustum planes and re-fetched the camera component for every avatar; both are identical across a frame, so compute them once in Update and pass the camera in via [Data].
… writes SetPointAtLayerWeight and SetRotationLayerWeight now shadow the last value written and skip the native Animator.SetLayerWeight call when it hasn't changed; ResetState clears the shadow so a rebind isn't skipped.
…ring-based lookups Completes the layer-index scaffolding introduced with the weight guard: - AnimatorEmoteLayers gains the BASE_LAYER_INDEX constant the scaffolding referenced (fixes compilation) and loses GetFromEmoteMask plus the unused ALL_LAYERS / NON_BASE_LAYERS arrays. - GetAnimatorCurrentStateTag(string) and SetLayerWeight(string) resolved the layer via a native Animator.GetLayerIndex call on every invocation, per frame in HeadIKSystem and HandsIKSystem. All callers now use the cached index API (UpperBodyLayerIndex, GetEmoteLayerIndex, int overloads) and the string overloads are removed. - Removes the unused DclAvatarMask using alias.
…oveIntentions.cs Co-authored-by: Muna <44584806+decentraland-bot@users.noreply.github.com> Signed-off-by: Nick Khalow <71646502+NickKhalow@users.noreply.github.com>
… pass CameraComponent as query data
…container-reuse' into opti/perf-hunt/rollup
…tions-fast-path' into opti/perf-hunt/rollup
…-guard' into opti/perf-hunt/rollup
…sform-cache' into opti/perf-hunt/rollup
…ad-handler-growth' into opti/perf-hunt/rollup
…lculation-job' into opti/perf-hunt/rollup
…-frustum-hoist' into opti/perf-hunt/rollup
Contributor
🚦 CI Status
Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below. Warnings count reduced: 13292 => 13202 Warnings/errors in files changed by this PR (104)All Unity tests passed ✅
|
decentraland-bot
left a comment
decentraland-bot
left a comment
Contributor
There was a problem hiding this comment.
STEP 2 — Root-cause check
PASS. This roll-up addresses eight specific performance bottlenecks, each fixing the cause rather than a symptom:
- Redundant bone-matrix computation beyond actual bone counts (#9705)
- Per-avatar frustum-plane extraction repeated inside the query (#9706)
- Redundant
Animator.SetLayerWeightnative interop calls (#9701) - Stale hand point-at surviving teleport / scene teardown (#9693)
- Per-frame
Allocator.TempJobnative-container churn in landscape rendering (#9697) - Unnecessary managed→native round-trip in
SetWorldTransformcache (#9702) - Linear buffer growth in
PartialDownloadHandler(#9703) - Mutex acquisition on empty remove-intention frames (#9700)
STEP 3 — Design & integration
PASS with observations.
HandPointAtSystem subscription (#9693): The system subscribes to scenesCache.CurrentScene.OnUpdate in its constructor and unsubscribes in OnDispose(). This is the correct lifecycle pairing — the event signals scene teardown, which is the right moment to clear stale hand-pointing state. The currentSceneLost latch avoids component mutation inside the callback. One fragility: the code accesses scenesCache.CurrentScene at both subscription and unsubscription time rather than capturing the reactive property into a field. If the property ever returned different instances across the system’s lifetime (e.g., after a realm switch that recreates the reactive property), the -= in OnDispose would silently no-op, leaking the subscription and preventing GC of the system. See inline comment.
Owner search — HandPointAtSystem: The point-at lifecycle is already owned by HandPointAtSystem. The new subscription adds scene-loss detection to this same owner. No parallel lifecycle introduced. Checked: HandPointAtSystem is the sole creator/manager of HandPointAtComponent. The PlayerTeleportIntent query correctly uses an ECS component that exists only during teleport.
RefreshBoneCounts per-frame query (#9705): StartAvatarMatricesCalculationSystem.RefreshBoneCounts writes skinningComponent.BoneCount into the pipeline every frame. Bone counts change only on wearable re-equip, so this is technically “reconciling every frame what is known at an explicit moment.” However, the cost is one int write per avatar per frame (~100–200 iterations at peak), negligible vs. the subsequent Burst-compiled matrix job. The event-driven alternative would require cross-assembly wiring through the wearable equip pipeline. The query correctly filters [None(typeof(DeleteEntityIntention))]. Acceptable.
RenderGroundSystem persistent containers (#9697): The NativeArray<int> and NativeList<Matrix4x4> are rendering data containers, NOT entity/component collections. They do not violate CLAUDE.md §1. Disposal is correctly handled in OnDispose(). Lazy initialization via the nullable tuple is clean.
STEP 4 — Member audit
New public/internal members:
| # | Member | Consumers | Verdict |
|---|---|---|---|
| 1 | AvatarTransformMatrixJobWrapper.SetBoneCount(ref…, int) |
1 (RefreshBoneCounts query) |
Data-push setter, not a derived predicate — OK |
| 2 | MainPlayerPipeline.SetBoneCount(int) |
1 (wrapper above) | Pipeline-internal forwarding — OK |
| 3 | RemoteAvatarPipeline.SetBoneCount(int, int) |
1 (wrapper above) | Has bounds check — OK |
| 4 | CalculateFrustumPlanes(Camera) |
2 (Update + tests) |
Internal, narrowly scoped — OK |
| 5 | IsVisibleInCamera(Bounds) |
1 (outline query) | Signature narrowed from (Camera, Bounds) — OK |
| 6 | HandPointAtComponent.StopPointing() |
2 (teleport + scene-lost) | Encapsulates two operations — OK |
| 7 | IRemoveIntentions.NewBunchAvailable() |
4 implementations | Lock-free pre-check, well-documented — OK |
| 8 | IAvatarView.UpperBodyLayerIndex |
2 (HandsIKSystem, HeadIKSystem) |
Replaces per-call GetLayerIndex(string) — OK |
| 9 | IAvatarView.GetEmoteLayerIndex(AvatarEmoteMask) |
5 call sites | Replaces AnimatorEmoteLayers.GetFromEmoteMask — OK |
| 10 | SetHiddenComponent (private→internal) |
Same call sites + tests | Widened for testability — see inline note on single-flag invariant |
No single-use→merge, absent≠false, or redundant-guard issues found.
STEP 5 — Line-level findings
All findings are P2 (no P0 or P1). See inline comments with suggestion blocks.
| # | Severity | File | Issue |
|---|---|---|---|
| 1 | P2 | HandPointAtSystem.cs |
Subscription identity: capture the reactive property in a field to guarantee unsubscribe targets the same instance |
| 2 | P2 | AvatarShapeVisibilitySystem.cs |
HasFlag→bitwise assumes single-flag callers; method is now internal. Add a debug assert |
| 3 | P2 | PartialDownloadHandler.cs |
PartialData.Length * 2 can overflow int for buffers > ~1 GB. Use long arithmetic |
| 4 | P2 | AvatarShapeVisibilitySystemShould.cs |
Test fields have double indentation (16 spaces instead of 8) on null!-annotated lines |
| 5 | P2 | AvatarOutlineFrustumHoistPerformanceTest.cs |
Same indentation issue on null!-annotated fields |
Security review: No security issues found. The ReceiveContentLengthHeader pre-allocation is correctly capped by PartialDownloadingRange.CHUNK_SIZE. The lock-free NewBunchAvailable() reads use atomic int reads with bounded one-frame staleness. No secrets, credentials, or injection surfaces.
STEP 6 — Complexity
COMPLEX. Touches the avatar rendering pipeline (GPU skinning, bone-matrix compute job), ECS systems, comms/multiplayer profile removal, landscape rendering with native containers, and scene lifecycle subscriptions across multiple assemblies.
STEP 7 — QA assessment
QA_REQUIRED: YES. Changes affect runtime avatar rendering (skinning, outlines, IK layers), landscape ground rendering, hand point-at behavior on teleport, and web-request buffer management.
STEP 8 — Non-blocking warnings
⚠️ CI: Test (playmode) failed — The failure is in “Log in to GitHub Container Registry,” an infrastructure issue unrelated to code changes. Edit-mode tests were cancelled as a cascade. Recommend re-running CI.
⚠️ CI: semantic / title-matches-convention failed — PR titleopti: perf-hunt roll-up (…)may not match the repo’s allowed semantic types. Verifyoptiis an allowed type or adjust the title.
⚠️ Missing newline at end of file —AvatarOutlineFrustumHoistPerformanceTest.csandRenderGroundContainerReusePerformanceTest.csare missing a trailing newline.
STEP 9 — Verdict
REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Touches avatar rendering pipeline (GPU skinning, bone-matrix jobs), ECS systems, comms profile removal, landscape native containers, and scene lifecycle subscriptions across multiple assemblies.
QA_REQUIRED: YES
Reviewed by Jarvis 🤖 · Requested by decentraland-bot via GitHub
lorenzo-ranciaffi
approved these changes
Aug 12, 2026
balducciv
approved these changes
Aug 13, 2026
balducciv
left a comment
balducciv
left a comment
There was a problem hiding this comment.
Build: v0.167.0-alpha-opti/perf-hunt/rollup-74cdef8
OS: macOS — M3, Windows 11
Scope: Roll-up of 8 perf/opti PRs (#9693, #9697, #9700, #9701, #9702, #9703, #9705, #9706). No behavioral changes expected — this is a smoke test to confirm no regressions across avatar rendering, landscape, teleport, web requests, and comms.
1. Avatar rendering — bone matrices, outlines, layer weights (#9701, #9705, #9706)
- [ ✅ ] Enter a crowded scene (Genesis Plaza or similar) with 10+ avatars visible
- [ ✅ ] Own avatar animates correctly — no skinning glitches or mesh distortion
- [ ✅ ] Remote avatars animate correctly — no skinning glitches or mesh distortion
- [ ✅ ] Avatar outlines render correctly (own + remote) when applicable (e.g. hover/selection outline if in build)
- [ ✅ ] Equip/swap a wearable with spring bones (hair, cape, accessory) — no bone glitches after swap
- [ ✅ ] Equip/swap a wearable with a different bone count than the previous one — no visual glitch on the transition
- [ ✅ ] Play an emote — animator layers (point-at, rotation) transition smoothly, no stuck poses
2. Hand point-at on teleport/reload (#9693)
- [ ✅ ] Point at an object/avatar with hand point-at active, then teleport — hand resets to neutral (no stale pinned arm)
- [ ✅ ] Point at an object, then reload the current scene (same realm) — hand resets to neutral
- [ ✅ ] Point at an object, then switch realms — hand resets to neutral
- [ ✅ ] Repeat point-at → teleport a few times in a row to check for leaked subscriptions (no degraded behavior after repeated cycles)
3. Landscape rendering (#9697)
- [ ✅ ] Roam across open ground/terrain outside scenes for a few minutes
- [ ✅ ] No landscape rendering artifacts, flickering, or missing ground tiles
- [ ✅ ] No visible hitching when ground comes in/out of view repeatedly (walk in and out of render distance)
4. Web requests / asset loading (#9703)
- [ ✅ ] Load an asset-heavy scene (lots of textures/models) — assets load completely, no stalled/partial downloads
- [ ✅ ] No broken/missing textures or models after load completes
- [ ✅ ] Repeat with a slow/throttled connection if possible, to exercise the buffer-growth path
5. Comms / multiplayer profiles (#9700)
- [ ✅ ] Join a scene with multiple other players present
- [ ✅ ] Other players' profiles (names, wearables) load and update correctly
- [ ✅ ] Players leaving/joining doesn't cause errors or stuck profile data
- [ ✅ ] No increase in profile-related lag on idle frames (subjective — watch for stutter when no profile changes are happening)
6. General regression / log check
- [ ✅ ] Player.log shows
Loading stage: Completedfor each scene load - [ ✅ ] No new exceptions tied to:
HandPointAtSystem,RenderGroundSystem,PartialDownloadHandler,RemoteEntitiesExtensions,BoneMatrixCalculationJob,AvatarShapeVisibilitySystem,SetWorldTransform - [ ✅ ] No FPS/performance regression noticed subjectively during the above steps (bonus: profiler check if available)
Unrelated errors noted (do not affect verdict):
[List any noise per the standard exclusion list — GPUInstancerPro, DOTween, Curl error 42/23, shutdown ObjectDisposedException, etc.]
Verdict: PASS ✅
Note: a issue was found while testing this PR and will be filed as a separate issue
Context here
Player test 1-4.log
Player prod GP OK.log
Player second test.log
Player build test 1.log
Player build test 2.log
Player prod.log
Windows
NickKhalow
changed the title
NickKhalow
enabled auto-merge (squash)
August 13, 2026 13:45
eordano
added a commit
that referenced
this pull request
Aug 13, 2026
Both this branch and dev's perf-hunt roll-up (#9707) optimized the same systems. Resolution: dev's shipped versions win where they subsume or refine the branch's copy of the same optimization (AvatarShapeVisibilitySystem + test, AvatarBase, HandPointAtSystem, RenderGroundSystem, TransformComponent, PartialDownloadHandler); branch-unique changes kept (CharacterEmoteSystem UpdateEmoteTags early-out, IRemoveIntentions doc). AnimatorEmoteLayers keeps dev's trimmed shape plus BASE_LAYER, still needed by the branch's layer-index perf test; that test's string-path arm now inlines the raw Animator calls the deleted wrapper made, so the baseline measurement is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Description
What does this PR change?
Roll-up of the perf-hunt branches into a single integration PR for combined testing and merge. It includes the following PRs (each remains open for individual review; this PR supersedes them on merge):
fix/9578)opti/perf-hunt/08-render-ground-container-reuse)opti/multiplayer/06-remove-intentions-fast-path)opti/chat/07-avatar-layer-weight-guard)opti/perf-hunt/07-set-world-transform-cache)opti/perf-hunt/16-partial-download-handler-growth)opti/perf-hunt/04-bone-matrix-calculation-job)opti/perf-hunt/03-avatar-outline-frustum-hoist)Also in the perf-hunt series but not part of this roll-up:
dev, so it's in this branch's base.opti/perf-hunt/11-atlas-tile-compression— rejected: it compresses atlas tiles at the consumer instead of fixing the texture pipeline (the KTX2 media-converter route), its satellite half is dead code (JPEGs load as RGB24, failing the RGBA32 guard), and main-threadCompress()trades a memory symptom for frame hitches while the CPU-side copy stays resident.See the individual PRs for per-change details, benchmarks, and test notes.
Test Instructions
Steps (standard run):
Expected result: No behavioral changes — the roll-up is fixes and work-reduction optimizations only. Key areas to sanity-check in one session:
Test Steps
AvatarShapeVisibilitySystem,BoneMatrixCalculationJob, and comms profile removal on idle frames (chore: opti(comms-profiles): skip the remove-intentions lock on empty frames #9700).Quality Checklist
Code Review Reference
Please review our Branch & PR Standards before submitting. It explains the automated review flow, QA/DEV approval requirements, and what each label does — especially useful for first-time contributors.