Skip to content

Perf alloc#3241

Open
scamiv wants to merge 7 commits intoopenfrontio:mainfrom
scamiv:perf-alloc
Open

Perf alloc#3241
scamiv wants to merge 7 commits intoopenfrontio:mainfrom
scamiv:perf-alloc

Conversation

@scamiv
Copy link
Contributor

@scamiv scamiv commented Feb 19, 2026

If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #(issue number)

Description:

PR Title

perf(core): reduce hot-path allocations & safe optimizations

This PR brings in a set of allocation-focused optimizations in core hot paths

Scope

  • src/core/execution/NukeExecution.ts
  • src/core/execution/WarshipExecution.ts
  • src/core/game/UnitGrid.ts
  • src/core/game/PlayerImpl.ts
  • src/core/configuration/DefaultConfig.ts
  • src/core/execution/SAMLauncherExecution.ts

What Changed

  • NukeExecution.detonate: reduced call overhead/allocations by caching mg/config, avoiding repeated lookups, and using allocation-free loops (no forEach closures) in the diminishing-effect pass.
  • WarshipExecution.findTargetUnit: replaced allocate+sort flow with single-pass best-target selection.
  • UnitGrid.nearbyUnits: reduced call overhead and allocations via single-type fast path and cached query coordinates.
  • PlayerImpl.units: added fast paths for common small-arity type queries (1-3 unit types).
  • DefaultConfig.unitInfo: cached UnitInfo objects per UnitType to avoid repeated object/closure creation.
  • SAMLauncherExecution targeting: removed sort churn and streamlined target selection with single-pass hydrogen prioritization.

Rebase

  • One conflict was resolved in NukeExecution.detonate by keeping main's diminishing-effect-per-impacted-tile behavior, while retaining the allocation-reduction refactors.

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

Goal: Optimized NukeExecution.detonate() to cut per-tile allocations by caching config, per-tile numTilesOwned, and per-owner TransportShip lists via a Map (src/core/execution/NukeExecution.ts:250).

Microbench: tests/perf/NukeDetonatePerf.ts (PERF_WARMUP=0 PERF_RUNS=2 PERF_OUTGOING_ATTACKS=200 PERF_TRANSPORT_SHIPS=200)

avgMs 264.463 -> 83.049 (p50 264.463 -> 83.049)
Goal: Avoid per-tick potentialTargets allocation + sort by selecting best target in one pass (src/core/execution/WarshipExecution.ts:78).

Microbench: tests/perf/WarshipFindTargetPerf.ts (PERF_WARMUP=0 PERF_RUNS=3 PERF_ITERS=2000 PERF_SHIPS=1500)

avgMs 1978.341 -> 431.092 (p50 1968.576 -> 420.816)
Goal: Avoid per-call Set creation and reduce call overhead by fast-pathing single-type queries + caching query coords (src/core/game/UnitGrid.ts:135).

Microbench: tests/perf/UnitGridNearbyUnitsPerf.ts (PERF_WARMUP=0 PERF_RUNS=3 PERF_ITERS=5000 PERF_UNITS=2000)

avgMs 395.661 -> 371.690 (p50 405.020 -> 365.366)
Goal: Avoid Set allocation and filter callback overhead for common 1–3 type queries (src/core/game/PlayerImpl.ts:216).

Microbench: tests/perf/PlayerUnitsPerf.ts (PERF_WARMUP=0 PERF_RUNS=3 PERF_ITERS=20000 PERF_UNITS=5000)

avgMs 5355.536 -> 3142.176 (p50 4939.107 -> 3152.285)
Goal: Reuse per-UnitType UnitInfo objects to avoid repeated object + closure creation in hot paths (src/core/configuration/DefaultConfig.ts:349).

Microbench: tests/perf/UnitInfoPerf.ts (PERF_WARMUP=0 PERF_RUNS=3 PERF_ITERS=20000)

avgMs 13.857 -> 3.240 (p50 12.118 -> 2.233)
Goal: Avoid targets.sort + reduce per-trajectory overhead by precomputing SAM rangeSquared and using single-pass Hydrogen priority (src/core/execution/SAMLauncherExecution.ts:27).

Microbench: tests/perf/SAMTargetingPerf.ts (PERF_WARMUP=0 PERF_RUNS=3 PERF_ITERS=2000 PERF_TRAJ_LEN=80)

avgMs 4518.498 -> 4363.653 (p50 4597.366 -> 4356.109)
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 19, 2026

Walkthrough

Introduces caching for unit info, several performance optimizations in execution and game layers (reducing allocations and repeated property access), and refactors targeting logic to pick a single best target instead of building/sorting candidate lists.

Changes

Cohort / File(s) Summary
Unit Info Caching
src/core/configuration/DefaultConfig.ts
Added private unitInfoCache: Map<UnitType, UnitInfo> and refactored unitInfo to consult/cache results. Reworked switch to assign a local info and store it. Port costWrapper now includes Factory; Train handled explicitly.
Nuke Execution
src/core/execution/NukeExecution.ts
Introduced local aliases (mg, config) and local variables to avoid repeated property access. Rewrote loops to use for-of and local captures, clarified unit deletion/filtering, and centralized distance checks via mg. No exported API changes.
SAM Launcher Targeting Refactor
src/core/execution/SAMLauncherExecution.ts
Reworked interception flow to reduce allocations: conditional Set creation for nearby nukes, removed isInRange, updated computeInterceptionTile signature to accept samTile and rangeSquared, and replaced target-array accumulation with a single best-target selection (HydrogenBomb prioritized).
Warship Target Selection
src/core/execution/WarshipExecution.ts
Replaced array-and-sort target selection with single bestUnit tracking using typePriority and distance. Cached local mg, config, owner, and patrolTile; added explicit TradeShip handling and patrol-range checks.
Player Unit Filtering
src/core/game/PlayerImpl.ts
Added fast paths for units(...) when requesting 0–3 types to avoid Set allocations; preserved Set-based logic for >3 types. Semantics unchanged.
Unit Grid Lookup
src/core/game/UnitGrid.ts
nearbyUnits now branches for single vs. multi-type queries, inlines distance computation, and uses local gm, x, y to reduce repeated lookups. Adds optimized multi-type aggregation while keeping single-type behavior equivalent.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🛠️ Small caches hum, old loops unwind,
Targets chosen sharp, no lists to bind,
Fast paths skip the extra Set,
Grid math shrinks each repeated get,
Tiny changes, smoother mind.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The PR title 'Perf alloc' is vague and does not clearly convey the main change; it lacks specificity about performance optimizations in core hot paths. Use a more descriptive title like 'Reduce hot-path allocations across core execution and grid systems' to clearly communicate the optimization focus.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The description clearly details performance optimizations across multiple files with specific scope, changes, and implementation details.

✏️ 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.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/core/game/UnitGrid.ts`:
- Around line 180-183: TypeScript isn't narrowing the union for `types` before
it's used in the nested loops (causing TS2345 on `Map.get`), so add an explicit
assertion to treat it as a single UnitType; change the local binding used in the
loops (the `type` constant derived from `types`) to be asserted as `UnitType`
(or assert at the `get` call) so `this.grid[cy][cx].get(...)` receives a
`UnitType` rather than `UnitType | readonly UnitType[]`; update the binding
where `const type = types` is declared to use the assertion (or replace the
argument to `get` with `types as UnitType`) to satisfy the type checker.

@github-project-automation github-project-automation bot moved this from Triage to Development in OpenFront Release Management Feb 19, 2026
Switch nearbyUnits’ type-guard from Array.isArray to typeof types !== "string" so types: UnitType | readonly UnitType[] narrows reliably and the single-type path only ever passes a UnitType into Map.get.
@evanpelle evanpelle added this to the v30 milestone Feb 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Development

Development

Successfully merging this pull request may close these issues.

2 participants

Comments