chore: perf 2026 08 06 - #9655
Conversation
…CS, streaming
Output of a scan -> filter -> review pipeline over the perf-critical
subsystems, each fix independently re-verified against the code and passed
through the repo's Jarvis review protocol (.github/prompts/review-instructions.md).
20 candidates surfaced; 19 land here, 1 (ApplyMaterialSystem throttle) was
dropped as a wrong-tool approach. Each fix ships a [Performance][Category("Performance")]
test designed to falsify it (assert the metric delta, fail on revert).
Rendering/GPU: drop redundant per-frame DrawArgs upload (GPUInstancingService);
group-shared LOD-compaction reduction (LODBuffersEvaluation.compute); reflection
probe IndividualFaces time-slicing; hoist grass compute uploads; reuse ground
native containers.
Avatars: bound the bone-matrix job by the consumer's authoritative BoneCount
(refreshed per-frame for main-player re-equip); gate RecalculateTangents on the
real _BumpMap source; restore the texture-array mip chain; hoist per-avatar
frustum extraction; early-out the emote Mecanim poll.
ECS: jobify PartitionAssetEntities; dirty-gate proxy transforms against the live
world transform (covers parent scene-load reposition); assign the transform world
cache from arguments; split visibility event systems from the throttled
dirty-query path.
Streaming/map: skip Sentry transaction alloc for non-whitelisted requests; cache
the artificial-delay toggle; pre-size PartialDownloadHandler; block-compress atlas
tiles; budget the map became-visible burst through the existing dirtyObjects drainer.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ts + add Landscape/MapRenderer refs Assembly references aren't transitive here, so each new perf-test folder must directly reference Unity.PerformanceTesting (carried only by DCL.PlayMode.Tests) plus its code-under-test. 14 folders get a .asmref into DCL.PlayMode.Tests; that assembly gains DCL.Landscape + MapRenderer references. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ob-system refs Moving the Culling tests into DCL.PlayMode.Tests lost MapRenderer InternalsVisibleTo access (CS0122) and Unity.Burst/Collections/Mathematics/Jobs refs (CS0246 on the partition test's BurstCompile job). Grant PlayMode.Tests the internals and add the 8 job-system assembly refs the code-under-test already uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… not a real API) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… runtime) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
/#17 harnesses #6: assert bucket+isBehind bit-exact (behaviour) but compare the raw diagnostic float within tolerance -- Burst vs Mono differ by 1 ULP on raw, which never flips the LOD/priority bucket (verified: all logged mismatches had identical bucket). #16/#17: measure allocations via ProfilerRecorder(GC.Alloc)+Measure.Method().GC() instead of GC.GetAllocatedBytesForCurrentThread (unsupported on Unity runtime). #14: create ROOT_POOL_CONTAINER in SetUp so EmotePlayer ctor doesn't NRE. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below. |
|
Warnings not reduced: 13729 => 13783 — remove at least 55 warnings to merge. Warnings/errors in files changed by this PR (216) |
|
Lint in progress, come back later! |
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
STEP 1 — Context
PR: #9655 — chore: perf 2026 08 06
Branch: perf/hunt-2026-08-06 → dev
Author: eordano
Scope: 85 files, +3991 / −202 — a batch of ~20 targeted performance optimizations across avatar rendering, ECS infrastructure, GPU rendering, map renderer, landscape, web requests, and diagnostics. Each optimization ships with a dedicated performance/falsification test.
Subsystems touched: avatar GPU skinning pipeline (bone matrix job, compute skinning, texture arrays), ECS systems (partition, visibility, transforms, emotes), compute shaders (LOD buffer accumulation), GPU instancing (draw args), reflection probes, map renderer culling, landscape ground rendering, web request analytics (Sentry), download handlers, and debug UI bindings.
STEP 2 — Root-cause check
Every optimization addresses a genuine root cause identified via profiling:
- Per-frame allocations replaced with persistent container reuse (ground rendering, download handler)
- Redundant per-entity computation hoisted out of hot loops (frustum planes, camera fetch, emote tag polling)
- Contended GPU atomics replaced with group-shared reduction (LOD accumulation shader)
- Unconditional work gated on actual need (tangent recompute, Sentry transaction setup, visibility dirty scans)
- Burst-jobified partition math replacing main-thread loops
- Texture mip chains restored for correct GPU LOD filtering
Root-cause verdict: PASS — no symptom masking, no swallowed exceptions, no disabled checks.
STEP 3 — Design & integration
New systems: GltfContainerVisibilityEventSystem, PrimitivesVisibilityEventSystem
Owner search: The lifecycle these handle (one-shot create events produced by FinalizeGltfContainerLoadingSystem / InstantiatePrimitiveRenderingSystem) is already managed by the existing VisibilitySystemBase<T> hierarchy — specifically its eventsBuffer.ForEach(forEachEvent) call. The split moves the every-frame event drain into an un-throttled companion so the throttled parent system can skip idle-frame scans without dropping one-shot events that ClearEntityEventsSystem clears the same frame.
This is not a parallel lifecycle reconciler — it's a deliberate cadence split of an existing owner's responsibilities. The event buffer is the same instance passed to both systems via the plugin. The un-throttled system runs only ApplyNewlyCreatedRenderables(), the throttled system runs only UpdateDirtyDrivenVisibility() + HandleRemovedVisibilityComponents(). The base class default Update() still calls all three for unsplit consumers (text-shape, NFT-shape). PASS.
PartitionAssetEntitiesSystem staging buffers
The system adds persistent stagedPositions/stagedComponents managed arrays and entityPositions/rawIn/outBucket/outIsBehind/outRaw NativeArrays. These are per-frame scratch buffers reused across frames to avoid re-allocation — contents are filled and consumed within the same Update() call, with managed references nulled out after writeback. This falls under CLAUDE.md §1's exception for "temporary collections for per-frame aggregation." NativeArrays are properly disposed in OnDispose. PASS.
VisibilitySystemBase refactoring
The monolithic Update() is split into three protected methods (UpdateDirtyDrivenVisibility, ApplyNewlyCreatedRenderables, HandleRemovedVisibilityComponents). The base default Update() preserves the original call order for unsplit consumers. The split allows the throttled/un-throttled halves to call the appropriate subset. PASS.
Teardown trace
MainPlayerPipeline.perAvatarBoneCount→ disposed inDispose()✓RemoteAvatarPipeline.perAvatarBoneCount→ disposed inDispose()✓PartitionAssetEntitiesSystemNativeArrays → disposed inOnDispose()✓RenderGroundSystem.instanceCounts/transforms→ disposed inOnDispose()✓PersistentElementBinding.OnValueChangedsubscription → same lifetime as subscriber, no leak ✓
STEP 4 — Member audit
| Member | Consumers | Verdict |
|---|---|---|
ComputeShaderSkinning.MeshNeedsTangents |
1 (SetupComputeShader) | Encodes complex condition (facial feature + normal map check). Named by intent. ✓ |
ComputeShaderSkinning.TANGENT_SOURCE_TEXTURE_ID |
1 (MeshNeedsTangents) | Centralizes the texture ID constant. ✓ |
AvatarMaterialConfiguration.IsFacialFeature |
1 (MeshNeedsTangents) | "Single source of truth" for facial-feature detection, preventing callers from re-deriving the suffix check. ✓ |
VisibilitySystemBase.UpdateDirtyDrivenVisibility/ApplyNewlyCreatedRenderables/HandleRemovedVisibilityComponents |
4+ (base Update, GltfContainerVisibilitySystem, PrimitivesVisibilitySystem, event systems) | Proper split of responsibilities. ✓ |
SentryWebRequestSampler.IsWhitelisted |
1 (SentryWebRequestHandler) | Reuses existing matcher. Justified single-consumer for encapsulation. ✓ |
PersistentElementBinding.OnValueChanged |
1 (ArtificialDelayOptions) | General-purpose event enabling the caching pattern. ✓ |
AvatarShapeVisibilitySystem.CalculateFrustumPlanes |
2 (Update, tests) | Hoisted out of per-avatar loop. ✓, but see P2 below. |
MapCullingController.EnqueueDirtyObject |
2 (SetTrackedStateDirty, ResolveDirtyCameras) | Extracted from SetTrackedStateDirty for reuse. ✓ |
PartitionAssetEntitiesSystem.ComputePartition |
3 (PartitionJob, RePartition, tests) | Shared pure math. ✓ |
STEP 5 — Line-level findings
See inline comments below.
STEP 6 — Complexity
COMPLEX — touches avatar GPU skinning pipeline, ECS systems (partition, visibility, transforms), compute shaders (LOD accumulation), GPU instancing, rendering (reflections), map renderer culling, and web request analytics across 15+ subsystems. Includes Burst-jobified code, HLSL compute shaders, and plugin registration changes.
STEP 7 — QA assessment
QA_REQUIRED: YES — changes affect runtime rendering (avatar skinning, visibility, ground, map, reflections), GPU instancing, and web request behavior in the Unity player.
STEP 8 — Non-blocking warnings
None. Main scene not modified.
Security review
No security issues found. All changes are client-side rendering/compute optimizations with no credential, auth, or input-validation surface.
Summary
This is an exceptionally well-crafted performance optimization batch. Each of the ~20 fixes:
- Targets a root cause identified by profiling
- Is behavior-preserving (verified by parity tests against the old code path)
- Ships with a dedicated performance/falsification test that fails if the fix is reverted
- Has thorough XML documentation explaining the rationale
Two minor P2 findings noted in inline comments — neither blocks merge.
REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Touches avatar GPU skinning pipeline, ECS visibility/partition/transform systems, HLSL compute shaders, GPU instancing, reflection probes, map culling, and web request analytics across 15+ subsystems.
QA_REQUIRED: YES
Reviewed by Jarvis 🤖 · Requested by Esteban Ordano (<@U9DLN0485>) via Slack
| { | ||
| Camera cam = camera.GetCameraComponent(World).Camera; | ||
| CalculateFrustumPlanes(cam); | ||
| GetAvatarsVisibleWithOutlineQuery(World, cam); |
There was a problem hiding this comment.
[P2] IsVisibleInCamera is a public method whose contract now silently depends on CalculateFrustumPlanes being called first. Without that call, the planes array contains stale data and the AABB test returns a wrong result. The XML doc on IsVisibleInCamera should document this precondition so future callers don't misuse it.
| GetAvatarsVisibleWithOutlineQuery(World, cam); | |
| public void CalculateFrustumPlanes(Camera camera) => GeometryUtility.CalculateFrustumPlanes(camera, planes); | |
| /// <summary> | |
| /// Tests whether the given bounds are inside the cached frustum planes. | |
| /// <see cref="CalculateFrustumPlanes"/> must be called first in the same frame to populate the planes array. | |
| /// </summary> |
| FastPathSqrDistance = partitionSettings.FastPathSqrDistance, | ||
| SceneBucket = scenePartition.Bucket, | ||
| SceneIsBehind = scenePartition.IsBehind, | ||
| SqrDistanceBuckets = sqrBuckets, | ||
| EntityPositions = entityPositions, | ||
| RawIn = rawIn, |
There was a problem hiding this comment.
[P2] ComputePartition is called from a [BurstCompile(FloatMode = FloatMode.Strict)] job. While Burst can compile it transitively, adding the [BurstCompile] attribute to the method itself makes the intent explicit and lets Burst apply its full optimization pipeline directly.
| FastPathSqrDistance = partitionSettings.FastPathSqrDistance, | |
| SceneBucket = scenePartition.Bucket, | |
| SceneIsBehind = scenePartition.IsBehind, | |
| SqrDistanceBuckets = sqrBuckets, | |
| EntityPositions = entityPositions, | |
| RawIn = rawIn, | |
| /// <summary> | |
| /// Pure, Burst-compatible partition math shared verbatim by the job and the managed | |
| /// fallback. Mirrors the historic RePartition/ResolvePartitionFromDistance evaluation | |
| /// order exactly (component-wise subtract, left-to-right square sum, int bucket compare, | |
| /// dot-product behind test) so both paths agree bit-for-bit. On the fast path the raw | |
| /// square distance is intentionally left untouched (passed through <paramref name="rawIn"/>), | |
| /// matching the previous behaviour where far entities inherited the scene bucket without | |
| /// rewriting RawSqrDistance. | |
| /// </summary> | |
| [BurstCompile] | |
| public static void ComputePartition( |
|
All Unity tests passed ✅
|
2 participants
No description provided.