-
Notifications
You must be signed in to change notification settings - Fork 238
perf: optimize hot loops, Set lookups, and O(n) path reconstruction #444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,32 +39,38 @@ const PARK_TYPES = new Set<BuildingType>([ | |
| 'community_garden', 'pond_park', 'park_gate', 'mountain_lodge', 'mountain_trailhead' | ||
| ]); | ||
|
|
||
| // PERF: Use Sets for O(1) lookup instead of O(n) array.includes() | ||
| // Sports facilities where pedestrians play sports | ||
| export const SPORTS_TYPES: BuildingType[] = [ | ||
| export const SPORTS_TYPES_SET = new Set<BuildingType>([ | ||
| 'basketball_courts', 'tennis', 'soccer_field_small', 'baseball_field_small', | ||
| 'football_field', 'baseball_stadium', 'stadium', 'swimming_pool', 'skate_park' | ||
| ]; | ||
| ]); | ||
| // Keep array export for backward compatibility where iteration is needed | ||
| export const SPORTS_TYPES: BuildingType[] = [...SPORTS_TYPES_SET]; | ||
|
|
||
| // Recreation areas where pedestrians relax | ||
| export const RELAXATION_TYPES: BuildingType[] = [ | ||
| export const RELAXATION_TYPES_SET = new Set<BuildingType>([ | ||
| 'park', 'park_large', 'community_garden', 'pond_park', 'greenhouse_garden', | ||
| 'amphitheater', 'campground', 'marina_docks_small', 'pier_large' | ||
| ]; | ||
| ]); | ||
| export const RELAXATION_TYPES: BuildingType[] = [...RELAXATION_TYPES_SET]; | ||
|
|
||
| // Active recreation (not sitting) | ||
| export const ACTIVE_RECREATION_TYPES: BuildingType[] = [ | ||
| export const ACTIVE_RECREATION_TYPES_SET = new Set<BuildingType>([ | ||
| 'playground_small', 'playground_large', 'mini_golf_course', 'go_kart_track', | ||
| 'roller_coaster_small', 'amusement_park', 'mountain_trailhead' | ||
| ]; | ||
| ]); | ||
| export const ACTIVE_RECREATION_TYPES: BuildingType[] = [...ACTIVE_RECREATION_TYPES_SET]; | ||
|
|
||
| // Enterable buildings (pedestrians go inside) | ||
| const ENTERABLE_BUILDING_TYPES: BuildingType[] = [ | ||
| // PERF: Use Set for O(1) lookup instead of O(n) array.includes() | ||
| const ENTERABLE_BUILDING_TYPES_SET = new Set<BuildingType>([ | ||
| 'shop_small', 'shop_medium', 'office_low', 'office_high', 'mall', | ||
| 'school', 'university', 'hospital', 'museum', 'community_center', | ||
| 'factory_small', 'factory_medium', 'factory_large', 'warehouse', | ||
| 'police_station', 'fire_station', 'city_hall', 'rail_station', | ||
| 'subway_station', 'mountain_lodge' | ||
| ]; | ||
| ]); | ||
|
|
||
| // Recreation area types for more specific destination finding | ||
| export type RecreationType = 'sports' | 'relaxation' | 'active' | 'general'; | ||
|
|
@@ -156,11 +162,11 @@ export function findRecreationAreas( | |
| for (let x = 0; x < gridSize; x++) { | ||
| const buildingType = grid[y][x].building.type; | ||
|
|
||
| if (SPORTS_TYPES.includes(buildingType)) { | ||
| if (SPORTS_TYPES_SET.has(buildingType)) { | ||
| destinations.push({ x, y, type: 'sports', buildingType }); | ||
| } else if (RELAXATION_TYPES.includes(buildingType)) { | ||
| } else if (RELAXATION_TYPES_SET.has(buildingType)) { | ||
| destinations.push({ x, y, type: 'relaxation', buildingType }); | ||
| } else if (ACTIVE_RECREATION_TYPES.includes(buildingType)) { | ||
| } else if (ACTIVE_RECREATION_TYPES_SET.has(buildingType)) { | ||
| destinations.push({ x, y, type: 'active', buildingType }); | ||
| } else if (PARK_TYPES.has(buildingType)) { | ||
| destinations.push({ x, y, type: 'general', buildingType }); | ||
|
|
@@ -187,7 +193,7 @@ export function findEnterableBuildings( | |
|
|
||
| // Only include active buildings (powered, not abandoned, construction complete) | ||
| if ( | ||
| ENTERABLE_BUILDING_TYPES.includes(buildingType) && | ||
| ENTERABLE_BUILDING_TYPES_SET.has(buildingType) && | ||
| tile.building.constructionProgress >= 100 && | ||
| !tile.building.abandoned | ||
| ) { | ||
|
|
@@ -202,21 +208,21 @@ export function findEnterableBuildings( | |
| * Check if a building type is a sports facility | ||
| */ | ||
| export function isSportsFacility(buildingType: BuildingType): boolean { | ||
| return SPORTS_TYPES.includes(buildingType); | ||
| return SPORTS_TYPES_SET.has(buildingType); | ||
| } | ||
|
|
||
| /** | ||
| * Check if a building type is a relaxation area | ||
| */ | ||
| export function isRelaxationArea(buildingType: BuildingType): boolean { | ||
| return RELAXATION_TYPES.includes(buildingType); | ||
| return RELAXATION_TYPES_SET.has(buildingType); | ||
| } | ||
|
Comment on lines
217
to
219
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| /** | ||
| * Check if a building type is enterable | ||
| */ | ||
| export function isEnterableBuilding(buildingType: BuildingType): boolean { | ||
| return ENTERABLE_BUILDING_TYPES.includes(buildingType); | ||
| return ENTERABLE_BUILDING_TYPES_SET.has(buildingType); | ||
| } | ||
|
Comment on lines
224
to
226
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,10 @@ import { | |
| spawnPedestrianAtBeach, | ||
| } from './pedestrianSystem'; | ||
|
|
||
| // PERF: Module-level Set for building types to skip in vehicle occlusion checks | ||
| // Avoids allocating a new array on every isVehicleBehindBuilding() call | ||
| const VEHICLE_SKIP_BUILDING_TYPES = new Set<BuildingType>(['road', 'grass', 'empty', 'water', 'tree']); | ||
|
|
||
| /** Train type for crossing detection (minimal interface) */ | ||
| export interface TrainForCrossing { | ||
| tileX: number; | ||
|
|
@@ -971,14 +975,19 @@ export function useVehicleSystems( | |
| // Build spatial index of cars by tile for efficient collision detection | ||
| // PERF: Use numeric keys (y * gridSize + x) instead of string keys | ||
| const carsByTile = new Map<number, Car[]>(); | ||
| for (const car of carsRef.current) { | ||
| const key = car.tileY * currentGridSize + car.tileX; | ||
| if (!carsByTile.has(key)) carsByTile.set(key, []); | ||
| carsByTile.get(key)!.push(car); | ||
| for (let ci = 0; ci < carsRef.current.length; ci++) { | ||
| const c = carsRef.current[ci]; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| const key = c.tileY * currentGridSize + c.tileX; | ||
| const bucket = carsByTile.get(key); | ||
| if (bucket) bucket.push(c); | ||
| else carsByTile.set(key, [c]); | ||
| } | ||
|
Comment on lines
+978
to
984
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| // PERF: Iterate original array directly instead of spreading to a copy | ||
| const sourceCars = carsRef.current; | ||
| const updatedCars: Car[] = []; | ||
| for (const car of [...carsRef.current]) { | ||
| for (let ci = 0; ci < sourceCars.length; ci++) { | ||
| const car = sourceCars[ci]; | ||
| // Update car age and remove if too old | ||
| car.age += delta * speedMultiplier; | ||
| if (car.age > car.maxAge) { | ||
|
|
@@ -1215,7 +1224,10 @@ export function useVehicleSystems( | |
|
|
||
| const updatedBuses: Bus[] = []; | ||
|
|
||
| for (const bus of [...busesRef.current]) { | ||
| // PERF: Iterate original array directly instead of spreading to a copy | ||
| const sourceBuses = busesRef.current; | ||
| for (let bi = 0; bi < sourceBuses.length; bi++) { | ||
| const bus = sourceBuses[bi]; | ||
| bus.age += delta * speedMultiplier; | ||
| if (bus.age > bus.maxAge) { | ||
| continue; | ||
|
|
@@ -1440,14 +1452,30 @@ export function useVehicleSystems( | |
| ctx.scale(dpr * currentZoom, dpr * currentZoom); | ||
| ctx.translate(currentOffset.x / currentZoom, currentOffset.y / currentZoom); | ||
|
|
||
| carsRef.current.forEach(car => { | ||
| // PERF: Compute viewport bounds for culling off-screen cars | ||
| const viewWidth = canvas.width / (dpr * currentZoom); | ||
| const viewHeight = canvas.height / (dpr * currentZoom); | ||
| const viewLeft = -currentOffset.x / currentZoom - TILE_WIDTH; | ||
| const viewTop = -currentOffset.y / currentZoom - TILE_HEIGHT * 2; | ||
| const viewRight = viewWidth - currentOffset.x / currentZoom + TILE_WIDTH; | ||
| const viewBottom = viewHeight - currentOffset.y / currentZoom + TILE_HEIGHT * 2; | ||
|
|
||
| // PERF: Use for loop instead of forEach to avoid closure allocation per car | ||
| const cars = carsRef.current; | ||
| for (let i = 0; i < cars.length; i++) { | ||
| const car = cars[i]; | ||
| const { screenX, screenY } = gridToScreen(car.tileX, car.tileY, 0, 0); | ||
| const centerX = screenX + TILE_WIDTH / 2; | ||
| const centerY = screenY + TILE_HEIGHT / 2; | ||
| const meta = DIRECTION_META[car.direction]; | ||
| const carX = centerX + meta.vec.dx * car.progress + meta.normal.nx * car.laneOffset; | ||
| const carY = centerY + meta.vec.dy * car.progress + meta.normal.ny * car.laneOffset; | ||
|
|
||
| // PERF: Skip cars outside viewport | ||
| if (carX < viewLeft - 20 || carX > viewRight + 20 || carY < viewTop - 20 || carY > viewBottom + 20) { | ||
| continue; | ||
| } | ||
|
|
||
| ctx.save(); | ||
| ctx.translate(carX, carY); | ||
| ctx.rotate(meta.angle); | ||
|
|
@@ -1471,7 +1499,7 @@ export function useVehicleSystems( | |
| ctx.fillRect(-10 * scale, -4 * scale, 2.4 * scale, 8 * scale); | ||
|
|
||
| ctx.restore(); | ||
| }); | ||
| } | ||
|
|
||
| ctx.restore(); | ||
| }, [worldStateRef, carsRef, isMobile]); | ||
|
|
@@ -1500,7 +1528,10 @@ export function useVehicleSystems( | |
| const viewRight = viewWidth - currentOffset.x / currentZoom + TILE_WIDTH; | ||
| const viewBottom = viewHeight - currentOffset.y / currentZoom + TILE_HEIGHT * 2; | ||
|
|
||
| busesRef.current.forEach(bus => { | ||
| // PERF: Use for loop instead of forEach to avoid closure allocation per bus | ||
| const buses = busesRef.current; | ||
| for (let i = 0; i < buses.length; i++) { | ||
| const bus = buses[i]; | ||
| const { screenX, screenY } = gridToScreen(bus.tileX, bus.tileY, 0, 0); | ||
| const centerX = screenX + TILE_WIDTH / 2; | ||
| const centerY = screenY + TILE_HEIGHT / 2; | ||
|
|
@@ -1509,7 +1540,7 @@ export function useVehicleSystems( | |
| const busY = centerY + meta.vec.dy * bus.progress + meta.normal.ny * bus.laneOffset; | ||
|
|
||
| if (busX < viewLeft - 60 || busX > viewRight + 60 || busY < viewTop - 80 || busY > viewBottom + 80) { | ||
| return; | ||
| continue; | ||
| } | ||
|
|
||
| ctx.save(); | ||
|
|
@@ -1527,8 +1558,8 @@ export function useVehicleSystems( | |
| ctx.fillRect(-length * 0.8, -width * 0.7, length * 1.4, width * 0.9); | ||
|
|
||
| ctx.fillStyle = 'rgba(191, 219, 254, 0.8)'; | ||
| for (let i = 0; i < 4; i++) { | ||
| const wx = -length * 0.7 + i * length * 0.45; | ||
| for (let j = 0; j < 4; j++) { | ||
| const wx = -length * 0.7 + j * length * 0.45; | ||
| ctx.fillRect(wx, -width * 0.55, length * 0.25, width * 1.1); | ||
| } | ||
|
|
||
|
|
@@ -1539,7 +1570,7 @@ export function useVehicleSystems( | |
| ctx.fillRect(length * 0.85, -width * 0.35, length * 0.1, width * 0.7); | ||
|
|
||
| ctx.restore(); | ||
| }); | ||
| } | ||
|
|
||
| ctx.restore(); | ||
| }, [worldStateRef, busesRef]); | ||
|
|
@@ -1652,8 +1683,8 @@ export function useVehicleSystems( | |
| if (!tile) continue; | ||
|
|
||
| const buildingType = tile.building.type; | ||
| const skipTypes: BuildingType[] = ['road', 'grass', 'empty', 'water', 'tree']; | ||
| if (skipTypes.includes(buildingType)) { | ||
| // PERF: Use module-level Set instead of allocating array every call | ||
| if (VEHICLE_SKIP_BUILDING_TYPES.has(buildingType)) { | ||
| continue; | ||
| } | ||
|
|
||
|
|
@@ -1667,7 +1698,10 @@ export function useVehicleSystems( | |
| return false; | ||
| }; | ||
|
|
||
| emergencyVehiclesRef.current.forEach(vehicle => { | ||
| // PERF: Use for loop instead of forEach to avoid closure allocation per vehicle | ||
| const vehicles = emergencyVehiclesRef.current; | ||
| for (let i = 0; i < vehicles.length; i++) { | ||
| const vehicle = vehicles[i]; | ||
| const { screenX, screenY } = gridToScreen(vehicle.tileX, vehicle.tileY, 0, 0); | ||
| const centerX = screenX + TILE_WIDTH / 2; | ||
| const centerY = screenY + TILE_HEIGHT / 2; | ||
|
|
@@ -1676,7 +1710,7 @@ export function useVehicleSystems( | |
| const vehicleY = centerY + meta.vec.dy * vehicle.progress + meta.normal.ny * vehicle.laneOffset; | ||
|
|
||
| if (vehicleX < viewLeft - 40 || vehicleX > viewRight + 40 || vehicleY < viewTop - 60 || vehicleY > viewBottom + 60) { | ||
| return; | ||
| continue; | ||
| } | ||
|
|
||
| ctx.save(); | ||
|
|
@@ -1739,7 +1773,7 @@ export function useVehicleSystems( | |
| ctx.fillRect(-length * scale, -4 * scale, 2 * scale, 8 * scale); | ||
|
|
||
| ctx.restore(); | ||
| }); | ||
| } | ||
|
|
||
| ctx.restore(); | ||
| }, [worldStateRef, emergencyVehiclesRef]); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.