From 8a50fdb3927c54e526ecedb72ff96804be305455 Mon Sep 17 00:00:00 2001 From: Phlosioneer Date: Tue, 16 Jun 2020 19:43:29 -0400 Subject: [PATCH 1/3] Remove belt cache array; use BeltComponent instead Removed the belt cache array. Follow-up belts are cached in the belt's BeltComponent instead. This change also removes the recursive follow-up search, which could cause a stack overflow for an extremely long belt chain. Saves one object allocation per belt per change, two very large array allocations per change, many function calls, and belts are only visited exactly once per change. --- src/js/game/components/belt.js | 5 ++++ src/js/game/systems/belt.js | 49 ++++++---------------------------- 2 files changed, 13 insertions(+), 41 deletions(-) diff --git a/src/js/game/components/belt.js b/src/js/game/components/belt.js index d98db49a..a9be5c99 100644 --- a/src/js/game/components/belt.js +++ b/src/js/game/components/belt.js @@ -5,6 +5,7 @@ import { BaseItem } from "../base_item"; import { Vector, enumDirection } from "../../core/vector"; import { Math_PI, Math_sin, Math_cos } from "../../core/builtins"; import { globalConfig } from "../../core/config"; +import { Entity } from "../entity"; export class BeltComponent extends Component { static getId() { @@ -12,6 +13,7 @@ export class BeltComponent extends Component { } static getSchema() { + // The followUpCache field is not serialized. return { direction: types.string, sortedItems: types.array(types.pair(types.float, types.obj(gItemRegistry))), @@ -34,6 +36,9 @@ export class BeltComponent extends Component { /** @type {Array<[number, BaseItem]>} */ this.sortedItems = []; + + /** @type {Entity} */ + this.followUpCache = null; } /** diff --git a/src/js/game/systems/belt.js b/src/js/game/systems/belt.js index 5fa5a265..494c9fa0 100644 --- a/src/js/game/systems/belt.js +++ b/src/js/game/systems/belt.js @@ -19,8 +19,6 @@ const SQRT_2 = Math_sqrt(2); const logger = createLogger("belt"); -/** @typedef {Array<{ entity: Entity, followUp: Entity }>} BeltCache */ - export class BeltSystem extends GameSystemWithFilter { constructor(root) { super(root, [BeltComponent]); @@ -66,9 +64,6 @@ export class BeltSystem extends GameSystemWithFilter { this.root.signals.entityDestroyed.add(this.updateSurroundingBeltPlacement, this); this.cacheNeedsUpdate = true; - - /** @type {BeltCache} */ - this.beltCache = []; } /** @@ -163,42 +158,14 @@ export class BeltSystem extends GameSystemWithFilter { return null; } - /** - * Adds a single entity to the cache - * @param {Entity} entity - * @param {BeltCache} cache - * @param {Set} visited - */ - computeSingleBeltCache(entity, cache, visited) { - // Check for double visit - if (visited.has(entity.uid)) { - return; - } - visited.add(entity.uid); - - const followUp = this.findFollowUpEntity(entity); - if (followUp) { - // Process followup first - this.computeSingleBeltCache(followUp, cache, visited); - } - - cache.push({ entity, followUp }); - } - computeBeltCache() { logger.log("Updating belt cache"); - let cache = []; let visited = new Set(); for (let i = 0; i < this.allEntities.length; ++i) { - this.computeSingleBeltCache(this.allEntities[i], cache, visited); + const entity = this.allEntities[i]; + entity.components.Belt.followUpCache = this.findFollowUpEntity(entity); } - assert( - cache.length === this.allEntities.length, - "Belt cache mismatch: Has " + cache.length + " entries but should have " + this.allEntities.length - ); - - this.beltCache = cache; } update() { @@ -217,8 +184,8 @@ export class BeltSystem extends GameSystemWithFilter { beltSpeed *= 100; } - for (let i = 0; i < this.beltCache.length; ++i) { - const { entity, followUp } = this.beltCache[i]; + for (let i = 0; i < this.allEntities.length; ++i) { + const entity = this.allEntities[i]; const beltComp = entity.components.Belt; const items = beltComp.sortedItems; @@ -244,8 +211,8 @@ export class BeltSystem extends GameSystemWithFilter { maxProgress = 1 - globalConfig.itemSpacingOnBelts; } else { // Otherwise our progress depends on the follow up - if (followUp) { - const spacingOnBelt = followUp.components.Belt.getDistanceToFirstItemCenter(); + if (beltComp.followUpCache) { + const spacingOnBelt = beltComp.followUpCache.components.Belt.getDistanceToFirstItemCenter(); maxProgress = Math.min(2, 1 - globalConfig.itemSpacingOnBelts + spacingOnBelt); // Useful check, but hurts performance @@ -270,8 +237,8 @@ export class BeltSystem extends GameSystemWithFilter { progressAndItem[0] = Math.min(maxProgress, progressAndItem[0] + speedMultiplier * beltSpeed); if (progressAndItem[0] >= 1.0) { - if (followUp) { - const followUpBelt = followUp.components.Belt; + if (beltComp.followUpCache) { + const followUpBelt = beltComp.followUpCache.components.Belt; if (followUpBelt.canAcceptItem()) { followUpBelt.takeItem(progressAndItem[1], progressAndItem[0] - 1.0); items.splice(itemIndex, 1); From 23417724257a7ee5d5e5d4e046da9722897a504a Mon Sep 17 00:00:00 2001 From: Phlosioneer Date: Tue, 16 Jun 2020 20:23:11 -0400 Subject: [PATCH 2/3] Optimize belt cache for common case Most of the time, we're adding/removing one building at a time. We don't need to recheck every belt, only the ones near the changed belt. --- src/js/game/systems/belt.js | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/js/game/systems/belt.js b/src/js/game/systems/belt.js index 494c9fa0..056692a8 100644 --- a/src/js/game/systems/belt.js +++ b/src/js/game/systems/belt.js @@ -13,6 +13,7 @@ import { MetaBeltBaseBuilding } from "../buildings/belt_base"; import { defaultBuildingVariant } from "../meta_building"; import { GameRoot } from "../root"; import { createLogger } from "../../core/logging"; +import { Rectangle } from "../../core/rectangle"; const BELT_ANIM_COUNT = 6; const SQRT_2 = Math_sqrt(2); @@ -64,6 +65,9 @@ export class BeltSystem extends GameSystemWithFilter { this.root.signals.entityDestroyed.add(this.updateSurroundingBeltPlacement, this); this.cacheNeedsUpdate = true; + /** @type {Rectangle} */ + this.singleUpdateArea = null; + this.isMultiUpdate = false; } /** @@ -114,6 +118,17 @@ export class BeltSystem extends GameSystemWithFilter { } } } + + // Optimize for the common case of adding or removing one belt. + if (this.cacheNeedsUpdate) { + if (this.isMultiUpdate || this.singleUpdateArea) { + // This isn't the common case. + // The singleUpdateArea will be cleared by the cache update. + this.isMultiUpdate = true; + } else { + this.singleUpdateArea = affectedArea.expandedInAllDirections(1); + } + } } draw(parameters) { @@ -161,11 +176,23 @@ export class BeltSystem extends GameSystemWithFilter { computeBeltCache() { logger.log("Updating belt cache"); - let visited = new Set(); - for (let i = 0; i < this.allEntities.length; ++i) { - const entity = this.allEntities[i]; - entity.components.Belt.followUpCache = this.findFollowUpEntity(entity); + if (this.singleUpdateArea && !this.isMultiUpdate) { + for (let x = this.singleUpdateArea.x; x < this.singleUpdateArea.right(); ++x) { + for (let y = this.singleUpdateArea.y; y < this.singleUpdateArea.bottom(); ++y) { + const tile = this.root.map.getTileContentXY(x, y); + if (tile && tile.components.Belt) { + tile.components.Belt.followUpCache = this.findFollowUpEntity(tile); + } + } + } + } else { + for (let i = 0; i < this.allEntities.length; ++i) { + const entity = this.allEntities[i]; + entity.components.Belt.followUpCache = this.findFollowUpEntity(entity); + } } + this.isMultiUpdate = false; + this.singleUpdateArea = null; } update() { From da24c472d7aeb3942cf145d80e6871b570cf0f2f Mon Sep 17 00:00:00 2001 From: Phlosioneer Date: Tue, 16 Jun 2020 22:08:46 -0400 Subject: [PATCH 3/3] Fix click and drag Clicking and dragging can trigger up to 4 add/destroy signals, and it's a common case. --- src/js/game/systems/belt.js | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/js/game/systems/belt.js b/src/js/game/systems/belt.js index 056692a8..a4b6d85f 100644 --- a/src/js/game/systems/belt.js +++ b/src/js/game/systems/belt.js @@ -65,9 +65,8 @@ export class BeltSystem extends GameSystemWithFilter { this.root.signals.entityDestroyed.add(this.updateSurroundingBeltPlacement, this); this.cacheNeedsUpdate = true; - /** @type {Rectangle} */ - this.singleUpdateArea = null; - this.isMultiUpdate = false; + /** @type {Rectangle[]} */ + this.smallUpdateAreas = []; } /** @@ -119,15 +118,10 @@ export class BeltSystem extends GameSystemWithFilter { } } - // Optimize for the common case of adding or removing one belt. + // Optimize for the common case of adding or removing buildings one at a time. + // Clicking and dragging can fire up to 4 create/destroy signals. if (this.cacheNeedsUpdate) { - if (this.isMultiUpdate || this.singleUpdateArea) { - // This isn't the common case. - // The singleUpdateArea will be cleared by the cache update. - this.isMultiUpdate = true; - } else { - this.singleUpdateArea = affectedArea.expandedInAllDirections(1); - } + this.smallUpdateAreas.push(affectedArea.expandedInAllDirections(1)); } } @@ -176,12 +170,15 @@ export class BeltSystem extends GameSystemWithFilter { computeBeltCache() { logger.log("Updating belt cache"); - if (this.singleUpdateArea && !this.isMultiUpdate) { - for (let x = this.singleUpdateArea.x; x < this.singleUpdateArea.right(); ++x) { - for (let y = this.singleUpdateArea.y; y < this.singleUpdateArea.bottom(); ++y) { - const tile = this.root.map.getTileContentXY(x, y); - if (tile && tile.components.Belt) { - tile.components.Belt.followUpCache = this.findFollowUpEntity(tile); + if (this.smallUpdateAreas.length <= 4) { + for (let i = 0; i < this.smallUpdateAreas.length; ++i) { + const area = this.smallUpdateAreas[i]; + for (let x = area.x; x < area.right(); ++x) { + for (let y = area.y; y < area.bottom(); ++y) { + const tile = this.root.map.getTileContentXY(x, y); + if (tile && tile.components.Belt) { + tile.components.Belt.followUpCache = this.findFollowUpEntity(tile); + } } } } @@ -191,8 +188,7 @@ export class BeltSystem extends GameSystemWithFilter { entity.components.Belt.followUpCache = this.findFollowUpEntity(entity); } } - this.isMultiUpdate = false; - this.singleUpdateArea = null; + this.smallUpdateAreas = []; } update() {