Refactor/Perf: Reuse OutlineFilters to reduce lategame GC overhead in StructureIconsLayer#3339
Refactor/Perf: Reuse OutlineFilters to reduce lategame GC overhead in StructureIconsLayer#3339Skigim wants to merge 2 commits intoopenfrontio:mainfrom
Conversation
WalkthroughReplaces inline Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
didn't have time to look deeper, but from a quick glance: |
So from what I can tell, the pixi library builds a programCache that covers the GPU side, but the js objects aren't cached on the CPU end so GC is still getting a workout with the memory overload. I'll run profiler when I get home to see if there's a substantial benefit - if not I'll edit title to refactor. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/client/graphics/layers/StructureIconsLayer.ts (1)
335-365: This still creates new filter arrays in the hot path.The
OutlineFilterobjects are reused now, but eachfilters = [FILTER_OUTLINE_*]here still allocates a fresh array, and the repeated writes can keep some GC pressure in the exact path this PR is trying to cool down. A small helper that skips no-op assignments and reuses the common filter-array values would make the perf win more complete.Also applies to: 640-641
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/graphics/layers/StructureIconsLayer.ts` around lines 335 - 365, The code repeatedly assigns new one-element arrays to sprite.container.filters (e.g., this.ghostUnit.container.filters = [FILTER_OUTLINE_RED] and similar for FILTER_OUTLINE_GREEN), causing garbage churn; add reusable const arrays like FILTERS_OUTLINE_RED = [FILTER_OUTLINE_RED] and FILTERS_OUTLINE_GREEN = [FILTER_OUTLINE_GREEN] and a small helper method (e.g., setFilters(container: PIXI.Container, filters: readonly any[])) that checks if container.filters === filters and only assigns when different, then replace all inline assignments in StructureIconsLayer (including the ghostUnit, potentialUpgrade.iconContainer, potentialUpgrade.dotContainer and the other occurrences mentioned) to call setFilters with the shared arrays to avoid allocating fresh arrays on the hot path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/client/graphics/layers/StructureIconsLayer.ts`:
- Around line 335-365: The code repeatedly assigns new one-element arrays to
sprite.container.filters (e.g., this.ghostUnit.container.filters =
[FILTER_OUTLINE_RED] and similar for FILTER_OUTLINE_GREEN), causing garbage
churn; add reusable const arrays like FILTERS_OUTLINE_RED = [FILTER_OUTLINE_RED]
and FILTERS_OUTLINE_GREEN = [FILTER_OUTLINE_GREEN] and a small helper method
(e.g., setFilters(container: PIXI.Container, filters: readonly any[])) that
checks if container.filters === filters and only assigns when different, then
replace all inline assignments in StructureIconsLayer (including the ghostUnit,
potentialUpgrade.iconContainer, potentialUpgrade.dotContainer and the other
occurrences mentioned) to call setFilters with the shared arrays to avoid
allocating fresh arrays on the hot path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 508f6847-9e59-4f54-9678-8f3234af88e4
📒 Files selected for processing (1)
src/client/graphics/layers/StructureIconsLayer.ts
This PR is the second slice of updates addressing #3207 by optimizing how we draw visual outlines for structures in
StructureIconsLayer.ts.The Problem:
Currently we create separate filters each time we need a new outline, which overloads GC exponentially as the game (read - number of structures) progresses.
The Fix:
The goal is to reuse OutlineFilter to reduce GC overhead instead of drawing fresh every frame by creating one outline for each color at game start and reusing them for each structure that needs them.
Please complete the following: (this is planning commit, to align with my agreement with scamiv and to ensure that I understand the bigger picture before proceeding. Will add profiler screenshots later before marking ready)
Please put your Discord username so you can be contacted if a bug or regression is found:
skigim