Gcpressure

[scamiv]

Examples of reducing GC churn

Game performance suffers from excessive garbage collection pressure.
Implement targeted optimizations to reduce per-frame allocations in hot code paths.
These are straightforward examples showing how small changes can significantly reduce GC churn.

Changes

1. GameRunner Tick Packing Buffer Reuse

File: src/core/GameRunner.ts

• Before: Each game tick created intermediate arrays via map() and new BigUint64Array instances
• After: Added reusable tileUpdateBuffer that grows dynamically and writes data directly
• Impact: Eliminates per-tick array allocations and typed array creations

Before (lines ~179-184):

// Many tiles are updated to pack it into an array
const packedTileUpdates = updates[GameUpdateType.Tile].map((u) => u.update);
updates[GameUpdateType.Tile] = [];

this.callBack({
  tick: this.game.ticks(),
  packedTileUpdates: new BigUint64Array(packedTileUpdates), // New allocation each tick

After (lines 179-199):

// Many tiles are updated to pack it into an array
const tileUpdates = updates[GameUpdateType.Tile];
const numUpdates = tileUpdates.length;

// Grow buffer if needed
if (numUpdates > this.tileUpdateBuffer.length) {
  this.tileUpdateBuffer = new BigUint64Array(
    Math.max(numUpdates, this.tileUpdateBuffer.length * 2),
  );
}

// Write directly to buffer
for (let i = 0; i < numUpdates; i++) {
  this.tileUpdateBuffer[i] = tileUpdates[i].update;
}

updates[GameUpdateType.Tile] = [];

this.callBack({
  tick: this.game.ticks(),
  packedTileUpdates: this.tileUpdateBuffer.subarray(0, numUpdates), // Reuse buffer

2. StructureIconsLayer Filter Caching

File: src/client/graphics/layers/StructureIconsLayer.ts

• Before: OutlineFilter instances created repeatedly (~50ms intervals during ghost structure rendering)
• After: Cached filter instances and reusable empty arrays
• Impact: Eliminates frequent filter object creation and array allocations

Before (line ~275):

this.ghostUnit.container.filters = [
  new OutlineFilter({ thickness: 2, color: "rgba(255, 0, 0, 1)" }),
];

Usage example (line ~276):

this.ghostUnit.container.filters = [this.redOutlineFilter];

*After (lines 80-93):

// Cached filter instances to reduce GC pressure
private readonly redOutlineFilter = new OutlineFilter({
  thickness: 2,
  color: "rgba(255, 0, 0, 1)",
});
private readonly greenOutlineFilter = new OutlineFilter({
  thickness: 2,
  color: "rgba(0, 255, 0, 1)",
});
private readonly whiteOutlineFilter = new OutlineFilter({
  thickness: 2,
  color: "rgb(255, 255, 255)",
});
private readonly emptyFilters: PIXI.Filter[] = [];

Result: new OutlineFilter(...) → this.redOutlineFilter, this.emptyFilters

[scamiv]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Walkthrough

This PR optimizes memory allocation across two core components by caching and reusing objects instead of creating new instances repeatedly. StructureIconsLayer caches outline filters as private fields; GameRunner reuses a tile-update buffer with dynamic growth.

Changes

Cohort / File(s)	Summary
Outline filter caching src/client/graphics/layers/StructureIconsLayer.ts	Introduced three cached OutlineFilter instances (redOutlineFilter, greenOutlineFilter, whiteOutlineFilter) and emptyFilters array as private fields. Replaced per-event filter instantiations with references to cached filters across multiple code paths (invalid builds, highlight upgrades, focus structure).
Tile update buffer reuse src/core/GameRunner.ts	Added reusable tileUpdateBuffer (BigUint64Array with dynamic growth) to pack tile updates instead of allocating new arrays each tick. Callback receives a subarray view of the buffer containing only written updates.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Verify all code paths in StructureIconsLayer correctly reference cached filters and that visual semantics are preserved
• Check buffer growth logic in GameRunner for edge cases and confirm subarray slicing is memory-safe
• Confirm no unintended behavioral changes to rendering or tile-update delivery

Possibly related PRs

• Improve icons readability #1321 — Modifies StructureIconsLayer for structure icon and construction-state rendering logic
• Add new icon shapes and filter for filtering icons on the layer #1348 — Modifies StructureIconsLayer white outline filter handling and visibility

Suggested labels

Performance Optimization

Suggested reviewers

• evanpelle
• scottanderson
• tnhnblgl

Poem

🔄 Objects cached, no more waste,
Memory reused at quickened pace,
Filters glow from private store,
Buffers flow forevermore! ✨

Pre-merge checks

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Gcpressure' is vague and does not clearly convey what the PR does; it uses a non-descriptive abbreviation that lacks context.	Use a clear, descriptive title like 'Reduce GC pressure with buffer reuse and filter caching' or 'Optimize hot paths to reduce garbage collection churn'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description clearly explains the purpose, changes, and impact of the PR with detailed before/after examples for both modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/core/GameRunner.ts (1)

183-193: Good buffer growth + direct write; consider capping growth to avoid “spike” memory retention.
If numUpdates spikes once, tileUpdateBuffer may stay huge for the whole match. Optional: cap to a max, or shrink on map change / new game.

src/client/graphics/layers/StructureIconsLayer.ts (1)

271-277: You still allocate 1-element filter arrays in hot paths; cache those too.
container.filters = [this.redOutlineFilter] allocates each time. You already solved the “clear” case with emptyFilters; you can do the same for set cases.

 class StructureIconsLayer implements Layer {
   // Cached filter instances to reduce GC pressure
   private readonly redOutlineFilter = new OutlineFilter({ thickness: 2, color: "rgba(255, 0, 0, 1)" });
   private readonly greenOutlineFilter = new OutlineFilter({ thickness: 2, color: "rgba(0, 255, 0, 1)" });
   private readonly whiteOutlineFilter = new OutlineFilter({ thickness: 2, color: "rgb(255, 255, 255)" });
   private readonly emptyFilters: PIXI.Filter[] = [];
+  private readonly redFilters: PIXI.Filter[] = [this.redOutlineFilter];
+  private readonly greenFilters: PIXI.Filter[] = [this.greenOutlineFilter];
+  private readonly whiteFilters: PIXI.Filter[] = [this.whiteOutlineFilter];
 }

Then replace [this.redOutlineFilter] → this.redFilters, etc.

Also applies to: 289-314, 510-515

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 427e462 and 3c45aaa.

📒 Files selected for processing (2)

• src/client/graphics/layers/StructureIconsLayer.ts (5 hunks)
• src/core/GameRunner.ts (2 hunks)

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-11-26T20:49:29.140Z

Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/game/GameView.ts:516-525
Timestamp: 2025-11-26T20:49:29.140Z
Learning: In GameView.ts, when usesSharedTileState is true (SAB mode), packedTileUpdates contains unpacked tile references as BigInt(tileRef) only, because all tile state lives in the shared Uint16Array. In non-SAB mode, packedTileUpdates contains packed TileUpdate bigints in the format (tileRef << 16n | state), which must be decoded via updateTile(tu). Therefore, Number(tu) is correct in SAB mode and shifting right by 16 bits would be wrong.

Applied to files:

• src/core/GameRunner.ts

📚 Learning: 2025-11-26T22:27:31.844Z

Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/worker/SharedTileRing.ts:55-63
Timestamp: 2025-11-26T22:27:31.844Z
Learning: In SharedTileRing.ts, the ring buffer is sized to width * height (the map dimensions). Combined with the dirty flag deduplication mechanism (each tile can only be enqueued once until the consumer clears its dirty flag), the number of pending entries is naturally bounded by the map size and drained every render frame. Therefore, ring buffer overflow should be extremely rare or effectively impossible, and any theoretical race condition between producer and consumer modifying the read index would only manifest as redundant tile refs being rendered, not memory corruption, which is acceptable.
<!-- </add_learning>

Applied to files:

• src/core/GameRunner.ts

📚 Learning: 2025-10-08T17:14:49.369Z

Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: In `ClientGameRunner.ts`, the `myPlayer` field is always set when `shouldPreventWindowClose()` is called, so the null check in that method is sufficient without needing to fetch it again from `gameView.playerByClientID()`.

Applied to files:

• src/core/GameRunner.ts

🔇 Additional comments (2)

src/core/GameRunner.ts (1)

99-100: The concern about subarray() causing data corruption does not apply here.

The callback receives packedTileUpdates as a view and passes it to GameView.update(), which immediately iterates it synchronously (line 520: forEach) and decodes the data into updatedTiles (line 521: this.updateTile(tu)). The view itself is never stored or accessed again. Each new tick generates a fresh view into the reused buffer, and stale views are automatically discarded—there is no deferred or async processing that would cause the view to outlive its validity.

The buffer reuse is a safe optimization here because consumption is synchronous and immediate within the same call stack.

Likely an incorrect or invalid review comment.

src/client/graphics/layers/StructureIconsLayer.ts (1)

79-94: The cached filter pattern is safe and a solid GC win. OutlineFilter's color option accepts CSS strings like "rgba(255, 0, 0, 1)", and sharing one immutable filter instance across multiple containers is safe as long as the filter's properties (thickness, color, etc.) are never changed after creation. Your code does exactly this—each filter is created once with fixed config and only assigned to container filter arrays, never mutated. The private readonly declarations reinforce this pattern.

No action needed; this is idiomatic and efficient.

[scamiv]

🧹 Nitpick comments (2)

src/core/GameRunner.ts (1)> 183-193: Good buffer growth + direct write; consider capping growth to avoid “spike” memory retention.

If numUpdates spikes once, tileUpdateBuffer may stay huge for the whole match. Optional: cap to a max, or shrink on map change / new game.

src/client/graphics/layers/StructureIconsLayer.ts (1)> 271-277: You still allocate 1-element filter arrays in hot paths; cache those too.

container.filters = [this.redOutlineFilter] allocates each time. You already solved the “clear” case with emptyFilters; you can do the same for set cases.

 class StructureIconsLayer implements Layer {
   // Cached filter instances to reduce GC pressure
   private readonly redOutlineFilter = new OutlineFilter({ thickness: 2, color: "rgba(255, 0, 0, 1)" });
   private readonly greenOutlineFilter = new OutlineFilter({ thickness: 2, color: "rgba(0, 255, 0, 1)" });
   private readonly whiteOutlineFilter = new OutlineFilter({ thickness: 2, color: "rgb(255, 255, 255)" });
   private readonly emptyFilters: PIXI.Filter[] = [];
+  private readonly redFilters: PIXI.Filter[] = [this.redOutlineFilter];
+  private readonly greenFilters: PIXI.Filter[] = [this.greenOutlineFilter];
+  private readonly whiteFilters: PIXI.Filter[] = [this.whiteOutlineFilter];
 }

Very valid