mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-06-21 09:40:44 +00:00
Perf spawn train (#3130)
## Description: Train spawning hot-path optimization (trade destination selection) ## Summary This PR reduces per-tick overhead in train spawning by removing temporary allocations and reducing work in the destination-selection path. The change focuses on `Cluster` trade destination lookup and how `TrainStationExecution` picks a destination. ## What changed ### 1) Maintain a “trade-capable” station subset per cluster `src/core/game/TrainStation.ts` - `Cluster` now maintains: - `stations`: all stations in the cluster (unchanged) - `tradeStations`: maintained subset of stations that can act as trade endpoints (`City` or `Port`) - `tradeStations` is kept in sync in: - `addStation()` - `removeStation()` - `clear()` Impact: - Trade queries no longer scan every station in the cluster; they only scan `tradeStations`. ### 2) Add cheap eligibility helpers `src/core/game/TrainStation.ts` - `hasAnyTradeDestination(player)`: - Fast early-exit check: returns as soon as it finds any eligible trade destination. - `randomTradeDestination(player, random)`: - Picks a random eligible trade destination directly without materializing an intermediate `Set`. ### 3) Use reservoir sampling for single-pass random choice `src/core/game/TrainStation.ts` `Cluster.randomTradeDestination()` uses reservoir sampling: - Iterates `tradeStations` once. - Maintains a running count of eligible stations (`eligibleSeen`). - Replaces the selected station with probability `1/eligibleSeen`. Properties: - Uniform selection among eligible stations. - One pass instead of “count then pick by index” (two pass). - Allocation-free. - Returns `null` when no eligible destination exists. ### 4) Update train spawning to avoid temporary sets `src/core/execution/TrainStationExecution.ts` - Previously: `spawnTrain()` called `cluster.availableForTrade()` and then `random.randFromSet(...)`. - This built a new `Set` on the hot path. - Now: - Early-exit via `cluster.hasAnyTradeDestination(owner)`. - Destination via `cluster.randomTradeDestination(owner, random)`. Net effect: - Less per-tick work and no per-spawn temporary `Set` allocations. ## Why this helps Train spawning happens frequently and can become a hot path in large games / large rail clusters. Avoiding repeated allocations and reducing work inside `tick()` helps keep frame/update time predictable. ## notes - Trade rules are unchanged (`tradeAvailable(player)` still gates eligibility). - Destination selection remains random-uniform over eligible `City`/`Port` stations that satisfy `tradeAvailable(player)`. - `TrainStationExecution` now avoids calling `spawnTrain()` entirely when `spawnTrains` is falsy (it was already guarded inside). ## 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
This commit is contained in:
@@ -45,7 +45,9 @@ export class TrainStationExecution implements Execution {
|
||||
this.active = false;
|
||||
return;
|
||||
}
|
||||
this.spawnTrain(this.station, ticks);
|
||||
if (this.spawnTrains) {
|
||||
this.spawnTrain(this.station, ticks);
|
||||
}
|
||||
}
|
||||
|
||||
private shouldSpawnTrain(): boolean {
|
||||
@@ -69,8 +71,8 @@ export class TrainStationExecution implements Execution {
|
||||
if (cluster === null) {
|
||||
return;
|
||||
}
|
||||
const availableForTrade = cluster.availableForTrade(this.unit.owner());
|
||||
if (availableForTrade.size === 0) {
|
||||
const owner = this.unit.owner();
|
||||
if (!cluster.hasAnyTradeDestination(owner)) {
|
||||
return;
|
||||
}
|
||||
if (!this.shouldSpawnTrain()) {
|
||||
@@ -79,20 +81,20 @@ export class TrainStationExecution implements Execution {
|
||||
|
||||
// Pick a destination randomly.
|
||||
// Could be improved to pick a lucrative trip
|
||||
const destination: TrainStation =
|
||||
this.random.randFromSet(availableForTrade);
|
||||
if (destination !== station) {
|
||||
this.mg.addExecution(
|
||||
new TrainExecution(
|
||||
this.mg.railNetwork(),
|
||||
this.unit.owner(),
|
||||
station,
|
||||
destination,
|
||||
this.numCars,
|
||||
),
|
||||
);
|
||||
this.lastSpawnTick = currentTick;
|
||||
}
|
||||
const destination = cluster.randomTradeDestination(owner, this.random);
|
||||
if (destination === null) return;
|
||||
if (destination === station) return;
|
||||
|
||||
this.mg.addExecution(
|
||||
new TrainExecution(
|
||||
this.mg.railNetwork(),
|
||||
owner,
|
||||
station,
|
||||
destination,
|
||||
this.numCars,
|
||||
),
|
||||
);
|
||||
this.lastSpawnTick = currentTick;
|
||||
}
|
||||
|
||||
activeDuringSpawnPhase(): boolean {
|
||||
|
||||
@@ -155,6 +155,12 @@ export class TrainStation {
|
||||
*/
|
||||
export class Cluster {
|
||||
public stations: Set<TrainStation> = new Set();
|
||||
private tradeStations: Set<TrainStation> = new Set();
|
||||
|
||||
private isTradeStation(station: TrainStation): boolean {
|
||||
const type = station.unit.type();
|
||||
return type === UnitType.City || type === UnitType.Port;
|
||||
}
|
||||
|
||||
has(station: TrainStation) {
|
||||
return this.stations.has(station);
|
||||
@@ -162,11 +168,15 @@ export class Cluster {
|
||||
|
||||
addStation(station: TrainStation) {
|
||||
this.stations.add(station);
|
||||
if (this.isTradeStation(station)) {
|
||||
this.tradeStations.add(station);
|
||||
}
|
||||
station.setCluster(this);
|
||||
}
|
||||
|
||||
removeStation(station: TrainStation) {
|
||||
this.stations.delete(station);
|
||||
this.tradeStations.delete(station);
|
||||
}
|
||||
|
||||
addStations(stations: Set<TrainStation>) {
|
||||
@@ -181,14 +191,39 @@ export class Cluster {
|
||||
}
|
||||
}
|
||||
|
||||
hasAnyTradeDestination(player: Player): boolean {
|
||||
for (const station of this.tradeStations) {
|
||||
if (station.tradeAvailable(player)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
randomTradeDestination(
|
||||
player: Player,
|
||||
random: PseudoRandom,
|
||||
): TrainStation | null {
|
||||
let selected: TrainStation | null = null;
|
||||
let eligibleSeen = 0;
|
||||
|
||||
for (const station of this.tradeStations) {
|
||||
if (!station.tradeAvailable(player)) continue;
|
||||
eligibleSeen++;
|
||||
|
||||
// Reservoir sampling: keep each eligible station with probability 1/eligibleSeen.
|
||||
if (random.nextInt(0, eligibleSeen) === 0) {
|
||||
selected = station;
|
||||
}
|
||||
}
|
||||
|
||||
return selected;
|
||||
}
|
||||
|
||||
availableForTrade(player: Player): Set<TrainStation> {
|
||||
const tradingStations = new Set<TrainStation>();
|
||||
for (const station of this.stations) {
|
||||
if (
|
||||
(station.unit.type() === UnitType.City ||
|
||||
station.unit.type() === UnitType.Port) &&
|
||||
station.tradeAvailable(player)
|
||||
) {
|
||||
for (const station of this.tradeStations) {
|
||||
if (station.tradeAvailable(player)) {
|
||||
tradingStations.add(station);
|
||||
}
|
||||
}
|
||||
@@ -201,6 +236,7 @@ export class Cluster {
|
||||
|
||||
clear() {
|
||||
this.stations.clear();
|
||||
this.tradeStations.clear();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,9 +1,13 @@
|
||||
import { vi, type Mocked } from "vitest";
|
||||
import { UnitType } from "../../../src/core/game/Game";
|
||||
import { Cluster, TrainStation } from "../../../src/core/game/TrainStation";
|
||||
|
||||
const createMockStation = (id: string): Mocked<TrainStation> => {
|
||||
return {
|
||||
id,
|
||||
unit: {
|
||||
type: vi.fn(() => UnitType.City),
|
||||
} as any,
|
||||
setCluster: vi.fn(),
|
||||
getCluster: vi.fn(() => null),
|
||||
} as any;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Unit } from "../../../src/core/game/Game";
|
||||
import { Unit, UnitType } from "../../../src/core/game/Game";
|
||||
import {
|
||||
RailNetworkImpl,
|
||||
StationManagerImpl,
|
||||
@@ -14,6 +14,7 @@ const createMockStation = (unitId: number): any => {
|
||||
unit: {
|
||||
id: unitId,
|
||||
setTrainStation: vi.fn(),
|
||||
type: vi.fn(() => UnitType.City),
|
||||
},
|
||||
tile: vi.fn(),
|
||||
neighbors: vi.fn(() => []),
|
||||
|
||||
Reference in New Issue
Block a user