perf: Optimize cluster calculation with DFS and zero-allocation patterns #2539

scamiv · 2025-11-29T14:49:39Z

Description:

Replace BFS with DFS and eliminate GC pressure in calculateClusters():

Switch from O(N) queue.shift() to O(1) stack.pop() operations
Replace Set.has()/Set.add() with Uint8Array bitfield
Add reusable buffer management to avoid repeated allocations
Implement callback-based neighbor iteration to eliminate array allocations
Add forEachNeighborWithDiag() method to Game interface and GameImpl
Remove now unused GameImpl import from PlayerExecution

Describe the PR.

Please complete the following:

I have added screenshots for all UI updates
I process any text displayed to the user through translateText() and I've added it to the en.json file
I have added relevant tests to the test directory
I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

DISCORD_USERNAME

Replace BFS with DFS and eliminate GC pressure in calculateClusters() hot path: - Switch from O(N) queue.shift() to O(1) stack.pop() operations - Replace Set.has()/Set.add() with Uint8Array bitfield - Add reusable buffer management to avoid repeated allocations - Implement callback-based neighbor iteration to eliminate array allocations - Add forEachNeighborWithDiag() method to Game interface and GameImpl - Remove now unused GameImpl import from PlayerExecution

coderabbitai · 2025-11-29T14:50:04Z

Walkthrough

Adds a per-game traversal state with generation-tracked, buffered flood-fill refactor for cluster computation in PlayerExecution, and introduces zero-allocation neighbor iteration APIs (forEachNeighbor and forEachNeighborWithDiag) on Game implemented in GameImpl.

Changes

Cohort / File(s)	Summary
Neighbor iteration API `src/core/game/Game.ts`, `src/core/game/GameImpl.ts`	Adds `forEachNeighbor(tile: TileRef, callback: (neighbor: TileRef) => void): void` (cardinal) and `forEachNeighborWithDiag(tile: TileRef, callback: (neighbor: TileRef) => void): void` (8-way) to `Game` and implements both in `GameImpl` as zero-allocation, callback-based neighbor iterators with bounds checks.
Cluster traversal & traversal state `src/core/execution/PlayerExecution.ts`	Reworks cluster computation to use a shared per-game `ClusterTraversalState { visited: Uint32Array; gen: number }` and a new `floodFillWithGen(currentGen, visited, startTiles, neighborFn, includeFn)` buffered flood-fill. Replaces per-tile BFS/sets with generation checks, uses the new neighbor callbacks, and updates generation bumping/visit checks. No public signatures changed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Inspect correctness of generation bumping and wrap/overflow handling in ClusterTraversalState.
Verify flood-fill logic preserves previous cluster connectivity (diagonals vs cardinal where intended).
Confirm forEachNeighbor* match existing neighbor semantics and bounds checks.
Check re-entrancy and concurrent calls to calculateClusters() with shared traversal state.

Suggested reviewers

evanpelle

Poem

🧭 Gen marks roll, the tiles awake,
Callbacks call neighbors, paths they make.
Queues grow small, flood fills sweep through,
Visited flags wave — clusters born anew. 🎉

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: optimizing cluster calculation with DFS and zero-allocation patterns, which directly reflects the core improvements in the PR.
Description check	✅ Passed	The description is directly related to the changeset, providing specific details about the optimization strategy including queue.shift() replacement, bitfield usage, buffer management, callback-based iteration, and new methods added.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e77c9f2 and 1adca9c.

📒 Files selected for processing (1)

src/core/execution/PlayerExecution.ts (8 hunks)

🧰 Additional context used

🧠 Learnings (7)

📓 Common learnings

Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2539
File: src/core/game/GameImpl.ts:520-542
Timestamp: 2025-11-29T22:22:37.170Z
Learning: In GameImpl.ts, neighborsWithDiag() and forEachNeighborWithDiag() intentionally duplicate coordinate iteration logic. They serve different purposes: forEachNeighborWithDiag() is a zero-allocation hot-path optimization while neighborsWithDiag() is a convenience method that returns an array. Refactoring one to call the other would add callback/closure allocations and indirection overhead, defeating the performance goals.

📚 Learning: 2025-11-29T22:22:37.170Z

Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2539
File: src/core/game/GameImpl.ts:520-542
Timestamp: 2025-11-29T22:22:37.170Z
Learning: In GameImpl.ts, neighborsWithDiag() and forEachNeighborWithDiag() intentionally duplicate coordinate iteration logic. They serve different purposes: forEachNeighborWithDiag() is a zero-allocation hot-path optimization while neighborsWithDiag() is a convenience method that returns an array. Refactoring one to call the other would add callback/closure allocations and indirection overhead, defeating the performance goals.

Applied to files:

src/core/execution/PlayerExecution.ts

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

Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/worker/SharedTileRing.ts:55-63
Timestamp: 2025-11-26T22:27:31.808Z
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/execution/PlayerExecution.ts

📚 Learning: 2025-10-20T20:15:28.858Z

Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.

Applied to files:

src/core/execution/PlayerExecution.ts

📚 Learning: 2025-08-28T22:47:31.406Z

Learnt from: BrewedCoffee
Repo: openfrontio/OpenFrontIO PR: 1957
File: tests/core/executions/PlayerExecution.test.ts:16-39
Timestamp: 2025-08-28T22:47:31.406Z
Learning: In test files, PlayerExecution instances must be manually registered with game.addExecution() because the setup utility doesn't automatically register SpawnExecution, which would normally handle this during the spawn phase in a real game.

Applied to files:

src/core/execution/PlayerExecution.ts

📚 Learning: 2025-10-20T11:02:16.969Z

Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.

Applied to files:

src/core/execution/PlayerExecution.ts

📚 Learning: 2025-10-18T17:54:01.311Z

Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.311Z
Learning: In src/core/execution/FakeHumanExecution.ts, MIRVs and ground attacks should not be mutually exclusive. The considerMIRV() method should not short-circuit maybeAttack() - both actions can occur in the same tick.

Applied to files:

src/core/execution/PlayerExecution.ts

🧬 Code graph analysis (1)

src/core/execution/PlayerExecution.ts (3)

src/core/game/GameImpl.ts (1)

owner (183-185)

src/core/game/PlayerImpl.ts (2)

tiles (283-285)

borderTiles (287-289)

src/core/game/GameMap.ts (1)

TileRef (3-3)

🔇 Additional comments (8)

src/core/execution/PlayerExecution.ts (8)

6-12: Clean typed state for generation-tracked traversal.

The ClusterTraversalState interface with a generation counter is a solid pattern to avoid clearing the visited array on each run. Using a module-level singleton aligns with the single-Game-per-runtime invariant discussed in prior review comments.

142-161: Callback-based neighbor iteration looks correct.

The hasUnownedNeighbor flag pattern works well since forEachNeighbor cannot be short-circuited. The check at line 156 correctly handles the flag after iteration completes.

One small note: the enemies.size !== 1 check at line 159 will early-exit the loop when more than one enemy is found (enemies.size > 1) or zero enemies after a tile check (enemies.size === 0). This is fine for the logic, but the check at line 163 outside the loop handles the final validation.

175-194: LGTM on isSurrounded refactor.

The neighbor iteration callback cleanly collects enemy tiles. The shore/edge check before iteration is a good early-exit optimization.

217-223: Good use of flood fill for cluster capture.

The flood fill with generation tracking correctly expands from firstTile to find all connected player-owned tiles. Using forEachNeighbor (non-diagonal) here is appropriate for territory connectivity.

237-246: Clean refactor to callback-based iteration.

The neighbor collection logic is correct. The callback pattern avoids array allocation for the neighbor list.

276-299: Solid cluster calculation with generation-tracked traversal.

Using a single generation bump for the entire calculateClusters call and then flood-filling each unvisited border tile is efficient. The diagonal neighbor iteration (forEachNeighborWithDiag) is appropriate for finding connected border regions.

323-331: Generation rollover handling is correct.

The check at 0xffffffff triggers before the value exceeds Uint32 range. Resetting to gen = 1 after fill(0) ensures no stale matches.

One tiny note: since JS numbers exceed 32-bit range, after increment gen becomes 0x100000000 (which equals 4294967296), not wrapped. The comparison with Uint32Array elements would still work correctly since those elements are clamped to 32-bit, but the reset at 0xffffffff prevents this edge case entirely.

333-367: Well-structured DFS flood fill with generation tracking.

The implementation correctly uses:

stack.pop() for O(1) removal vs O(N) queue.shift().

Callback-based neighbor iteration to avoid intermediate arrays.

Generation-based visited tracking to skip already-processed tiles.

The local stack and result allocations per call are reasonable. The stack could be pooled for further optimization, but the current approach is a good balance between clarity and performance.

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

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

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/core/execution/PlayerExecution.ts (1)
291-291: Fix comment formatting.

Missing space after // in the comment.
-        //Use callback to avoid creating a 'neighbors' Array
+        // Use callback to avoid creating a 'neighbors' Array

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ab53ee6 and 14bf09f.

📒 Files selected for processing (3)

src/core/execution/PlayerExecution.ts (2 hunks)
src/core/game/Game.ts (1 hunks)
src/core/game/GameImpl.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

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

Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/worker/SharedTileRing.ts:55-63
Timestamp: 2025-11-26T22:27:31.808Z
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/execution/PlayerExecution.ts

🔇 Additional comments (5)

src/core/game/Game.ts (1)

667-673: LGTM! Clean zero-allocation API design.

The callback-based pattern is a solid approach for performance-critical code. The documentation clearly explains the motivation.

src/core/execution/PlayerExecution.ts (4)

13-17: LGTM! Proper buffer initialization.

Initializing the buffer to an empty Uint8Array(0) is clean. The actual sizing happens lazily when needed based on map dimensions.

264-274: LGTM! Efficient buffer management.

The early return (lines 264-265) and buffer reuse pattern (line 273) are solid optimizations. Using fill(0) to clear is significantly faster than allocating a new Set on each call.

Minor: The null check on line 269 (!this._visitedBuffer) is unnecessary since the constructor initializes the field to Uint8Array(0).

263-305: Well-designed performance optimization.

The refactored calculateClusters successfully achieves the stated goals:

✅ Replaces O(N) queue.shift() with O(1) stack.pop()

✅ Replaces Set.has()/Set.add() with Uint8Array bitfield access

✅ Uses callback-based neighbor iteration to eliminate array allocations

✅ Reuses buffer across calls to minimize GC pressure

The DFS algorithm implementation is correct and should deliver significant performance improvements.

279-303: Based on the web search results, I've found a critical connectivity mismatch. The web search confirms that the original calculateClusters algorithm uses four-neighbour connectivity (orthogonal neighbors only), but the current code uses forEachNeighborWithDiag which implements 8-connectivity (includes diagonal neighbors).

Additionally, the web search hints that there may have been a recent change in the commit history related to this.

Since the repository is currently inaccessible, I'll rewrite the review comment based on the verified findings:

Connectivity approach changed from 4-way to 8-way neighbors — verify this is intentional.

The current implementation uses forEachNeighborWithDiag() for 8-connectivity (includes diagonal neighbors), but the documented behavior of calculateClusters is to compute connected components using four-neighbour connectivity (orthogonal neighbors only). This algorithmic change affects territory clustering, which has downstream implications for troop generation, unit/building validity, and conquest logic. Confirm whether this change to diagonal connectivity was intentional or accidental. If unintentional, revert to forEachNeighbor() for orthogonal neighbors only.

Regarding the use of TileRef as an array index (lines 281, 285, 295, 298): this is a standard pattern in the codebase and appears safe, assuming TileRef represents a valid linear tile index in the range [0, width × height). This should be verified during code review or testing to ensure no out-of-bounds access occurs.

src/core/game/GameImpl.ts

evanpelle · 2025-12-01T03:31:00Z

src/core/execution/PlayerExecution.ts

+    if (!this._visitedBuffer || this._visitedBuffer.length < mapSize) {
+      this._visitedBuffer = new Uint8Array(mapSize);
+    } else {


let's do the visitedBuffer initialization in the init() method

I’ve removed the buffer from PlayerExecution and replaced it with a Game scoped Uint32Array

evanpelle · 2025-12-01T03:33:53Z

src/core/execution/PlayerExecution.ts

+      this._visitedBuffer = new Uint8Array(mapSize);
+    } else {
+      // Fast clear (much faster than creating a new Set)
+      this._visitedBuffer.fill(0);


I'm little worried about this, we'll be filling a 2 million+ large array for each player. This could negate the gains made?

maybe instead we store player small id in the array so players can reuse the same array (like this: this._visitedBuffer[neighbor] === player.getSmallID()), then we only need to clear the array once each tick?

I think we should run a performance profile, or create a perf test, see AstarPerf.test

I’ve switched to a single Uint32Array visited buffer with a generational counter, so we go from O(players * mapSize) to O(mapSize) and avoid clearing the array.

- Introduce ClusterTraversalState and a WeakMap<Game, ClusterTraversalState> in PlayerExecution.ts to store visited: Uint32Array and gen per game instance. - Remove the per-player _visitedBuffer - Update calculateClusters() to allocate/resize a single Uint32Array(totalTiles) per Game, use a generation counter instead of calling fill(0) to clear. - Switch visited checks to visited[tile] === currentGen.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/core/execution/PlayerExecution.ts (1)

269-317: DFS cluster calculation looks correct and keeps allocations low

The new calculateClusters logic matches the intended behavior:

Early-return on empty borderTiles avoids unnecessary work.

Buffer sizing is tied to width * height, and you grow the buffer only when the map grows.

The generational scheme (gen + Uint32Array of tile refs) ensures each tile is visited at most once per call without any O(N) clear.

The stack-based DFS over borderTiles plus forEachNeighborWithDiag produces connected components as before, while avoiding neighbor array allocations.

If you want to squeeze allocations even further, you could store the stack array in ClusterTraversalState and reuse it across calls (just set stack.length = 0 at the start of calculateClusters()), instead of creating a new array each time. That would be a small, optional win and keeps the code shape similar.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 14bf09f and a10b3aa.

📒 Files selected for processing (1)

src/core/execution/PlayerExecution.ts (2 hunks)

🧰 Additional context used

🧠 Learnings (6)

📓 Common learnings

Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2539
File: src/core/game/GameImpl.ts:520-542
Timestamp: 2025-11-29T22:22:37.170Z
Learning: In GameImpl.ts, neighborsWithDiag() and forEachNeighborWithDiag() intentionally duplicate coordinate iteration logic. They serve different purposes: forEachNeighborWithDiag() is a zero-allocation hot-path optimization while neighborsWithDiag() is a convenience method that returns an array. Refactoring one to call the other would add callback/closure allocations and indirection overhead, defeating the performance goals.

📚 Learning: 2025-11-29T22:22:37.170Z

Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2539
File: src/core/game/GameImpl.ts:520-542
Timestamp: 2025-11-29T22:22:37.170Z
Learning: In GameImpl.ts, neighborsWithDiag() and forEachNeighborWithDiag() intentionally duplicate coordinate iteration logic. They serve different purposes: forEachNeighborWithDiag() is a zero-allocation hot-path optimization while neighborsWithDiag() is a convenience method that returns an array. Refactoring one to call the other would add callback/closure allocations and indirection overhead, defeating the performance goals.

Applied to files:

src/core/execution/PlayerExecution.ts

📚 Learning: 2025-10-20T20:15:28.858Z

Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.

Applied to files:

src/core/execution/PlayerExecution.ts

📚 Learning: 2025-08-28T22:47:31.406Z

Learnt from: BrewedCoffee
Repo: openfrontio/OpenFrontIO PR: 1957
File: tests/core/executions/PlayerExecution.test.ts:16-39
Timestamp: 2025-08-28T22:47:31.406Z
Learning: In test files, PlayerExecution instances must be manually registered with game.addExecution() because the setup utility doesn't automatically register SpawnExecution, which would normally handle this during the spawn phase in a real game.

Applied to files:

src/core/execution/PlayerExecution.ts

📚 Learning: 2025-10-20T11:02:16.969Z

Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.

Applied to files:

src/core/execution/PlayerExecution.ts

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

Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/worker/SharedTileRing.ts:55-63
Timestamp: 2025-11-26T22:27:31.808Z
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/execution/PlayerExecution.ts

🧬 Code graph analysis (1)

src/core/execution/PlayerExecution.ts (4)

src/core/game/Game.ts (1)

Game (659-752)

src/core/game/PlayerImpl.ts (1)

borderTiles (287-289)

src/core/game/GameMap.ts (1)

TileRef (3-3)

src/core/game/UnitImpl.ts (1)

tile (176-178)

🔇 Additional comments (1)

src/core/execution/PlayerExecution.ts (1)

6-13: Per-game traversal state with generational visited is a good fit

Using a WeakMap<Game, ClusterTraversalState> plus a single Uint32Array per game removes the need for one visited buffer per player, and the gen counter means you avoid clearing the full array on each run. This directly addresses the earlier concern about filling multi-million entry arrays per player without adding complexity. I don’t see functional problems here.

evanpelle · 2025-12-01T19:21:55Z

src/core/execution/PlayerExecution.ts

+// Per-game traversal state used by calculateClusters() to avoid per-player buffers.
+const traversalStates = new WeakMap<Game, ClusterTraversalState>();


when would we ever have multiple game instances?

I could turn it into a static, but this seemed cleaner to ensure isolation in case we ever have different Game instances in the same thread (e.g. unit tests). Want it changed?

It's an invariant we'll only have a single game object at a time.

- Convert neighbor loops in surroundedBySamePlayer, isSurrounded, getCapturingPlayer to forEachNeighbor callbacks - Implement floodFillWithGen() method for configurable zero-allocation flood fill - Replace BFS in removeCluster() with floodFillWithGen using cardinal neighbors - Refactor calculateClusters() to use floodFillWithGen with diagonal neighbors - Add generational state management and forEachNeighbor interface method

…red traversal state variable. - Update traversal state management

scamiv requested a review from a team as a code owner November 29, 2025 14:49

github-actions bot assigned scamiv Nov 29, 2025

coderabbitai bot requested changes Nov 29, 2025

View reviewed changes

src/core/game/GameImpl.ts Show resolved Hide resolved

evanpelle reviewed Dec 1, 2025

View reviewed changes

coderabbitai bot previously approved these changes Dec 1, 2025

View reviewed changes

scamiv dismissed coderabbitai[bot]’s stale review via a10b3aa December 1, 2025 17:38

coderabbitai bot reviewed Dec 1, 2025

View reviewed changes

coderabbitai bot previously approved these changes Dec 1, 2025

View reviewed changes

evanpelle reviewed Dec 1, 2025

View reviewed changes

scamiv dismissed coderabbitai[bot]’s stale review via e77c9f2 December 1, 2025 23:52

coderabbitai bot previously approved these changes Dec 1, 2025

View reviewed changes

- Replace the WeakMap for per-game traversal states with a single sha…

1adca9c

…red traversal state variable. - Update traversal state management

scamiv dismissed coderabbitai[bot]’s stale review via 1adca9c December 2, 2025 13:38

coderabbitai bot approved these changes Dec 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

perf: Optimize cluster calculation with DFS and zero-allocation patterns #2539

perf: Optimize cluster calculation with DFS and zero-allocation patterns #2539

Uh oh!

scamiv commented Nov 29, 2025

Uh oh!

coderabbitai bot commented Nov 29, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

evanpelle Dec 1, 2025

Uh oh!

scamiv Dec 1, 2025

Uh oh!

evanpelle Dec 1, 2025

Uh oh!

scamiv Dec 1, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

evanpelle Dec 1, 2025

Uh oh!

scamiv Dec 1, 2025

Uh oh!

evanpelle Dec 2, 2025

Uh oh!

scamiv Dec 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// Per-game traversal state used by calculateClusters() to avoid per-player buffers.
		const traversalStates = new WeakMap<Game, ClusterTraversalState>();

perf: Optimize cluster calculation with DFS and zero-allocation patterns #2539

Are you sure you want to change the base?

perf: Optimize cluster calculation with DFS and zero-allocation patterns #2539

Uh oh!

Conversation

scamiv commented Nov 29, 2025

Description:

Please complete the following:

Please put your Discord username so you can be contacted if a bug or regression is found:

Uh oh!

coderabbitai bot commented Nov 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

Pre-merge checks

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

evanpelle Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

scamiv Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

evanpelle Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

scamiv Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

evanpelle Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

scamiv Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

evanpelle Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

scamiv Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai bot commented Nov 29, 2025 •

edited

Loading

scamiv Dec 1, 2025 •

edited

Loading