From b7669f44ad44b38238cd89460a116dc81e43ae12 Mon Sep 17 00:00:00 2001 From: Frotty Date: Thu, 10 Sep 2026 10:26:01 +0200 Subject: [PATCH 1/5] Optimize spatial query result reuse --- wurst/StdlibIngameTests.wurst | 67 ++++---- .../SpatialIndexForDestructables.wurst | 78 +++++++-- wurst/closures/SpatialIndexForUnits.wurst | 158 +++++++++++++----- wurst/data/ArrayList.wurst | 23 ++- wurst/data/ArrayListTests.wurst | 12 ++ wurst/data/SparseSet.wurst | 46 ++--- wurst/data/SparseSetTests.wurst | 14 ++ 7 files changed, 290 insertions(+), 108 deletions(-) diff --git a/wurst/StdlibIngameTests.wurst b/wurst/StdlibIngameTests.wurst index 61bb0a2a..17f17082 100644 --- a/wurst/StdlibIngameTests.wurst +++ b/wurst/StdlibIngameTests.wurst @@ -8,7 +8,7 @@ import Execute import ErrorHandling import UnitSpatialIndex import SpatialIndexForUnits -import SparseSet +import ArrayList import UnitIndexer import OnUnitEnterLeave @@ -467,22 +467,26 @@ function testGroupNesting() cases (hidden, locust, corpses, boundary distances) need no engine behaviour to be hardcoded. These checks are Lua-index checks only. On Jass or with the index disabled, the native-less API - returns an empty SparseSet by contract, so there is no meaningful parity assertion to run. */ + leaves the provided result empty by contract, so there is no meaningful parity assertion to run. */ constant SPATIAL_TEST_POS = vec2(-2000, 2000) unit spatialInitProbe = null var creatingSpatialInitProbe = false var spatialInitProbeEnterEvents = 0 +let spatialResultScratch = new ArrayList(64) +let spatialOuterScratch = new ArrayList(64) +let spatialMiddleScratch = new ArrayList(64) +let spatialInnerScratch = new ArrayList(64) +constant UnitSpatialFilter spatialPlayerZeroFilter = u -> u.getOwner() == players[0] function countSpatialInitProbeEnter() if creatingSpatialInitProbe or getEnterLeaveUnit() == spatialInitProbe spatialInitProbeEnterEvents++ -function collectViaSparseSet(vec2 pos, real radius, bool collisionFiltering, group into) - let matches = unitsInRange(pos, radius, collisionFiltering) - for i = 0 to matches.size() - 1 - into.add(matches.get(i)) - destroy matches +function collectViaSpatialIndex(vec2 pos, real radius, bool collisionFiltering, group into) + unitsInRange(spatialResultScratch, pos, radius, collisionFiltering) + for i = 0 to spatialResultScratch.size() - 1 + into.add(spatialResultScratch.get(i)) function collectNative(vec2 pos, real radius, bool collisionFiltering, group into) if collisionFiltering @@ -510,7 +514,7 @@ function describeDifference(group actual, group expected) returns string function checkRangeParity(vec2 pos, real radius, bool collisionFiltering, string label) let actual = CreateGroup() let expected = CreateGroup() - collectViaSparseSet(pos, radius, collisionFiltering, actual) + collectViaSpatialIndex(pos, radius, collisionFiltering, actual) collectNative(pos, radius, collisionFiltering, expected) var equal = actual.size() == expected.size() if equal @@ -560,6 +564,11 @@ function testSpatialIndexParity() checkRangeParity(SPATIAL_TEST_POS, 121., false, "tight radius on a probe boundary") checkRangeParity(SPATIAL_TEST_POS, 2000., false, "wide radius") + let foreign = createUnit(players[1], UnitIds.footman, SPATIAL_TEST_POS, angle(0)) + unitsInRange(spatialResultScratch, SPATIAL_TEST_POS, 300., false, spatialPlayerZeroFilter) + check(not spatialResultScratch.has(foreign), "query filter excludes a spatial match in one pass") + foreign.remove() + // Manual UnitIndexer deindexing does not remove a live unit from native enumeration, so the // independent spatial registry must retain it until OnUnitEnterLeave observes a real leave. probes[0].deindex() @@ -585,11 +594,10 @@ function testSpatialIndexParity() SPATIAL_TEST_POS.x + 200., SPATIAL_TEST_POS.y + 200.) let actualRect = CreateGroup() let expectedRect = CreateGroup() - let rectMatches = unitsInBox(vec2(r.getMinX() + 32., r.getMinY() + 32.), + unitsInBox(spatialResultScratch, vec2(r.getMinX() + 32., r.getMinY() + 32.), vec2(r.getMaxX(), r.getMaxY())) - for i = 0 to rectMatches.size() - 1 - actualRect.add(rectMatches.get(i)) - destroy rectMatches + for i = 0 to spatialResultScratch.size() - 1 + actualRect.add(spatialResultScratch.get(i)) GroupEnumUnitsInRect(expectedRect, r, null) var rectEqual = actualRect.size() == expectedRect.size() if rectEqual @@ -669,9 +677,9 @@ function testSpatialIndexReentrancy() // readout which caches the group size and indexes into it: groups silently drop removed units, // so every later index shifts down and a unit is skipped. A snapshot is immune. reentrancyVisited = 0 - let removalSnapshot = unitsInRange(SPATIAL_TEST_POS.add(96., 0.), 400.) - for i = 0 to removalSnapshot.size() - 1 - let u = removalSnapshot.get(i) + unitsInRange(spatialOuterScratch, SPATIAL_TEST_POS.add(96., 0.), 400.) + for i = 0 to spatialOuterScratch.size() - 1 + let u = spatialOuterScratch.get(i) reentrancyVisited++ if reentrancyVisited == 1 for j = 0 to 3 @@ -681,7 +689,6 @@ function testSpatialIndexReentrancy() break check(reentrancyVisited >= 4, "removing a unit mid-iteration skips nobody (" + reentrancyVisited + " visited)") - destroy removalSnapshot for i = 0 to 3 if reentrancyProbes[i] != null @@ -695,22 +702,19 @@ function testSpatialIndexReentrancy() // pushes above the outer one and must not disturb it. reentrancyVisited = 0 reentrancyNested = 0 - let outerSnapshot = unitsInRange(SPATIAL_TEST_POS.add(96., 0.), 400.) - for outerIndex = 0 to outerSnapshot.size() - 1 - let outerUnit = outerSnapshot.get(outerIndex) + unitsInRange(spatialOuterScratch, SPATIAL_TEST_POS.add(96., 0.), 400.) + for outerIndex = 0 to spatialOuterScratch.size() - 1 + let outerUnit = spatialOuterScratch.get(outerIndex) reentrancyVisited++ - let middleSnapshot = unitsInRange(outerUnit.getPos(), 300.) - for middleIndex = 0 to middleSnapshot.size() - 1 - let middleUnit = middleSnapshot.get(middleIndex) + unitsInRange(spatialMiddleScratch, outerUnit.getPos(), 300.) + for middleIndex = 0 to spatialMiddleScratch.size() - 1 + let middleUnit = spatialMiddleScratch.get(middleIndex) reentrancyNested++ - let innerSnapshot = unitsInRange(middleUnit.getPos(), 100.) - for innerIndex = 0 to innerSnapshot.size() - 1 - let innerUnit = innerSnapshot.get(innerIndex) + unitsInRange(spatialInnerScratch, middleUnit.getPos(), 100.) + for innerIndex = 0 to spatialInnerScratch.size() - 1 + let innerUnit = spatialInnerScratch.get(innerIndex) if innerUnit == null reentrancyNested-- - destroy innerSnapshot - destroy middleSnapshot - destroy outerSnapshot check(reentrancyVisited == 4, "outer enumeration unaffected by nesting (" + reentrancyVisited + ")") check(reentrancyNested > 0, "nested enumerations ran (" + reentrancyNested + ")") @@ -719,13 +723,12 @@ function testSpatialIndexReentrancy() // that is already running, and must not corrupt the iteration. reentrancyVisited = 0 spawnedDuringQuery = null - let creationSnapshot = unitsInRange(SPATIAL_TEST_POS.add(96., 0.), 400.) - for i = 0 to creationSnapshot.size() - 1 - let _u = creationSnapshot.get(i) + unitsInRange(spatialOuterScratch, SPATIAL_TEST_POS.add(96., 0.), 400.) + for i = 0 to spatialOuterScratch.size() - 1 + let _u = spatialOuterScratch.get(i) reentrancyVisited++ if spawnedDuringQuery == null spawnedDuringQuery = createUnit(players[0], UnitIds.footman, SPATIAL_TEST_POS, angle(0)) - destroy creationSnapshot check(reentrancyVisited == 4, "creating a unit mid-iteration does not extend it (" + reentrancyVisited + ")") spawnedDuringQuery?.remove() diff --git a/wurst/closures/SpatialIndexForDestructables.wurst b/wurst/closures/SpatialIndexForDestructables.wurst index bcfdb960..aafa2ca7 100644 --- a/wurst/closures/SpatialIndexForDestructables.wurst +++ b/wurst/closures/SpatialIndexForDestructables.wurst @@ -1,4 +1,5 @@ package SpatialIndexForDestructables +import ArrayList import SparseSet import DestructableSpatialIndex import Rect @@ -6,29 +7,74 @@ import Rect /** * Native-less Lua spatial queries for destructables. * - * Each result is owned by the caller and must be destroyed. The range query intentionally matches + * Pass a reusable ArrayList for dense iteration, or a reusable + * SparseSet when membership operations are needed. The target is reset before use. + * Filters are caller-owned, and nested queries must use separate result collections. + * The range query intentionally matches * ClosureForGroups.forDestructablesInRange: it returns the square that encloses the circle rather * than applying a second distance test. Runtime-created destructables must be registered explicitly. */ -function newDestructableResult() returns SparseSet - return new SparseSet(DESTRUCTABLE_SPARSE_SET_KEY) +// Caller-owned predicate evaluated once for each spatial match before it enters the result. +public interface DestructableSpatialFilter + function matches(destructable whichDestructable) returns boolean -public function destructablesInRect(rect area) returns SparseSet - let result = newDestructableResult() - if isLua and USE_DESTRUCTABLE_SPATIAL_INDEX - let matched = destructableSpatialIndexBeginBoxQuery( - vec2(area.getMinX(), area.getMinY()), vec2(area.getMaxX(), area.getMaxY())) +function appendMatches(ArrayList result, int matched, DestructableSpatialFilter filter) + if filter == null for i = 0 to matched - 1 result.add(destructableSpatialIndexQuery(i)) - destructableSpatialIndexEndQuery() - return result + else + for i = 0 to matched - 1 + let d = destructableSpatialIndexQuery(i) + if filter.matches(d) + result.add(d) + destructableSpatialIndexEndQuery() -public function destructablesInRange(vec2 center, real range) returns SparseSet - let result = newDestructableResult() - if isLua and USE_DESTRUCTABLE_SPATIAL_INDEX - let matched = destructableSpatialIndexBeginRangeQuery(center, range) +function appendMatches(SparseSet result, int matched, DestructableSpatialFilter filter) + if filter == null for i = 0 to matched - 1 result.add(destructableSpatialIndexQuery(i)) - destructableSpatialIndexEndQuery() - return result + else + for i = 0 to matched - 1 + let d = destructableSpatialIndexQuery(i) + if filter.matches(d) + result.add(d) + destructableSpatialIndexEndQuery() + +public function destructablesInRect(ArrayList result, rect area, + DestructableSpatialFilter filter) + result.reset() + if isLua and USE_DESTRUCTABLE_SPATIAL_INDEX + appendMatches(result, destructableSpatialIndexBeginBoxQuery( + vec2(area.getMinX(), area.getMinY()), vec2(area.getMaxX(), area.getMaxY())), filter) + +public function destructablesInRect(ArrayList result, rect area) + destructablesInRect(result, area, null) + +public function destructablesInRect(SparseSet result, rect area, + DestructableSpatialFilter filter) + result.reset() + if isLua and USE_DESTRUCTABLE_SPATIAL_INDEX + appendMatches(result, destructableSpatialIndexBeginBoxQuery( + vec2(area.getMinX(), area.getMinY()), vec2(area.getMaxX(), area.getMaxY())), filter) + +public function destructablesInRect(SparseSet result, rect area) + destructablesInRect(result, area, null) + +public function destructablesInRange(ArrayList result, vec2 center, real range, + DestructableSpatialFilter filter) + result.reset() + if isLua and USE_DESTRUCTABLE_SPATIAL_INDEX + appendMatches(result, destructableSpatialIndexBeginRangeQuery(center, range), filter) + +public function destructablesInRange(ArrayList result, vec2 center, real range) + destructablesInRange(result, center, range, null) + +public function destructablesInRange(SparseSet result, vec2 center, real range, + DestructableSpatialFilter filter) + result.reset() + if isLua and USE_DESTRUCTABLE_SPATIAL_INDEX + appendMatches(result, destructableSpatialIndexBeginRangeQuery(center, range), filter) + +public function destructablesInRange(SparseSet result, vec2 center, real range) + destructablesInRange(result, center, range, null) diff --git a/wurst/closures/SpatialIndexForUnits.wurst b/wurst/closures/SpatialIndexForUnits.wurst index 3b8d475f..edc6753f 100644 --- a/wurst/closures/SpatialIndexForUnits.wurst +++ b/wurst/closures/SpatialIndexForUnits.wurst @@ -1,4 +1,5 @@ package SpatialIndexForUnits +import ArrayList import SparseSet import UnitSpatialIndex import Rect @@ -6,54 +7,135 @@ import Rect /** * Native-less Lua spatial queries. * - * Each call returns an owned SparseSet. The result is independent of Warcraft groups and must - * be destroyed by the caller. These APIs are intentionally Lua-oriented: on Jass or while disabled, - * they return an empty set because no native fallback is hidden behind the generic result type. + * Pass a reusable ArrayList for the cheapest dense result, or a reusable SparseSet when + * membership operations are needed. The target is reset before it is populated. No Warcraft group, + * result collection, or filter is allocated or destroyed by these overloads. + * + * Filters are caller-owned so one instance can be reused across queries. A capturing lambda creates + * a closure at its declaration site; keep it outside hot loops when allocation matters. + * Nested queries must use separate result collections because each call resets its target. + * + * These APIs are intentionally Lua-oriented: on Jass or while disabled, the target remains empty + * because no native fallback is hidden behind the generic result type. */ -function newUnitResult() returns SparseSet - return new SparseSet(UNIT_SPARSE_SET_KEY) +// Caller-owned predicate evaluated once for each spatial match before it enters the result. +public interface UnitSpatialFilter + function matches(unit whichUnit) returns boolean + +function appendMatches(ArrayList result, int matched, UnitSpatialFilter filter) + if filter == null + for i = 0 to matched - 1 + result.add(spatialIndexQueryUnit(i)) + else + for i = 0 to matched - 1 + let u = spatialIndexQueryUnit(i) + if filter.matches(u) + result.add(u) + spatialIndexEndQuery() -function addRangeMatches(SparseSet result, vec2 center, real radius, boolean collisionFiltering) - let matched = spatialIndexBeginQuery(center, radius, collisionFiltering) - for i = 0 to matched - 1 - result.add(spatialIndexQueryUnit(i)) +function appendMatches(SparseSet result, int matched, UnitSpatialFilter filter) + if filter == null + for i = 0 to matched - 1 + result.add(spatialIndexQueryUnit(i)) + else + for i = 0 to matched - 1 + let u = spatialIndexQueryUnit(i) + if filter.matches(u) + result.add(u) spatialIndexEndQuery() -/** Returns units whose origins are within radius of center. */ -public function unitsInRange(vec2 center, real radius) returns SparseSet - return unitsInRange(center, radius, false) +/** Replaces result with units whose origins are within radius of center. */ +public function unitsInRange(ArrayList result, vec2 center, real radius) + unitsInRange(result, center, radius, false, null) + +/** Replaces result with filtered units whose origins are within radius of center. */ +public function unitsInRange(ArrayList result, vec2 center, real radius, + UnitSpatialFilter filter) + unitsInRange(result, center, radius, false, filter) + +/** Replaces result with units in range, optionally applying collision-size filtering. */ +public function unitsInRange(ArrayList result, vec2 center, real radius, + boolean collisionFiltering) + unitsInRange(result, center, radius, collisionFiltering, null) -/** Returns units in range, optionally applying collision-size filtering. */ -public function unitsInRange(vec2 center, real radius, boolean collisionFiltering) returns SparseSet - let result = newUnitResult() +/** Replaces result with filtered units in range, optionally applying collision-size filtering. */ +public function unitsInRange(ArrayList result, vec2 center, real radius, + boolean collisionFiltering, UnitSpatialFilter filter) + result.reset() if isLua and USE_UNIT_SPATIAL_INDEX - addRangeMatches(result, center, radius, collisionFiltering) - return result + appendMatches(result, spatialIndexBeginQuery(center, radius, collisionFiltering), filter) -/** Returns units whose origins are inside the axis-aligned box. */ -public function unitsInBox(vec2 boxMin, vec2 boxMax) returns SparseSet - let result = newUnitResult() +/** Replaces result with units whose origins are within radius of center. */ +public function unitsInRange(SparseSet result, vec2 center, real radius) + unitsInRange(result, center, radius, false, null) + +/** Replaces result with filtered units whose origins are within radius of center. */ +public function unitsInRange(SparseSet result, vec2 center, real radius, + UnitSpatialFilter filter) + unitsInRange(result, center, radius, false, filter) + +/** Replaces result with units in range, optionally applying collision-size filtering. */ +public function unitsInRange(SparseSet result, vec2 center, real radius, + boolean collisionFiltering) + unitsInRange(result, center, radius, collisionFiltering, null) + +/** Replaces result with filtered units in range, optionally applying collision-size filtering. */ +public function unitsInRange(SparseSet result, vec2 center, real radius, + boolean collisionFiltering, UnitSpatialFilter filter) + result.reset() if isLua and USE_UNIT_SPATIAL_INDEX - let matched = spatialIndexBeginBoxQuery(boxMin, boxMax) - for i = 0 to matched - 1 - result.add(spatialIndexQueryUnit(i)) - spatialIndexEndQuery() - return result + appendMatches(result, spatialIndexBeginQuery(center, radius, collisionFiltering), filter) + +/** Replaces result with filtered units whose origins are inside the axis-aligned box. */ +public function unitsInBox(ArrayList result, vec2 boxMin, vec2 boxMax, + UnitSpatialFilter filter) + result.reset() + if isLua and USE_UNIT_SPATIAL_INDEX + appendMatches(result, spatialIndexBeginBoxQuery(boxMin, boxMax), filter) + +public function unitsInBox(ArrayList result, vec2 boxMin, vec2 boxMax) + unitsInBox(result, boxMin, boxMax, null) -/** Returns units matching GroupEnumUnitsInRect semantics for the given rect. */ -public function unitsInRect(rect area) returns SparseSet +/** Replaces result with filtered units whose origins are inside the axis-aligned box. */ +public function unitsInBox(SparseSet result, vec2 boxMin, vec2 boxMax, + UnitSpatialFilter filter) + result.reset() + if isLua and USE_UNIT_SPATIAL_INDEX + appendMatches(result, spatialIndexBeginBoxQuery(boxMin, boxMax), filter) + +public function unitsInBox(SparseSet result, vec2 boxMin, vec2 boxMax) + unitsInBox(result, boxMin, boxMax, null) + +public function unitsInRect(ArrayList result, rect area, UnitSpatialFilter filter) // Warcraft's native unit rect enum starts 32 units above the requested minimum edge. - return unitsInBox(vec2(area.getMinX() + 32., area.getMinY() + 32.), - vec2(area.getMaxX(), area.getMaxY())) + unitsInBox(result, vec2(area.getMinX() + 32., area.getMinY() + 32.), + vec2(area.getMaxX(), area.getMaxY()), filter) + +public function unitsInRect(ArrayList result, rect area) + unitsInRect(result, area, null) -/** Returns currently indexed units owned by owner. This is a linear registry scan; per-player - secondary sets are intentionally not maintained in this first iteration. */ -public function unitsOfPlayer(player owner) returns SparseSet - let result = newUnitResult() +public function unitsInRect(SparseSet result, rect area, UnitSpatialFilter filter) + unitsInBox(result, vec2(area.getMinX() + 32., area.getMinY() + 32.), + vec2(area.getMaxX(), area.getMaxY()), filter) + +public function unitsInRect(SparseSet result, rect area) + unitsInRect(result, area, null) + +/** Replaces result with currently indexed units owned by owner. This is a linear registry scan; + per-player secondary sets are intentionally not maintained. */ +public function unitsOfPlayer(ArrayList result, player owner, UnitSpatialFilter filter) + result.reset() if isLua and USE_UNIT_SPATIAL_INDEX - let matched = spatialIndexBeginPlayerQuery(owner) - for i = 0 to matched - 1 - result.add(spatialIndexQueryUnit(i)) - spatialIndexEndQuery() - return result + appendMatches(result, spatialIndexBeginPlayerQuery(owner), filter) + +public function unitsOfPlayer(ArrayList result, player owner) + unitsOfPlayer(result, owner, null) + +public function unitsOfPlayer(SparseSet result, player owner, UnitSpatialFilter filter) + result.reset() + if isLua and USE_UNIT_SPATIAL_INDEX + appendMatches(result, spatialIndexBeginPlayerQuery(owner), filter) + +public function unitsOfPlayer(SparseSet result, player owner) + unitsOfPlayer(result, owner, null) diff --git a/wurst/data/ArrayList.wurst b/wurst/data/ArrayList.wurst index 09dabab1..5348c6ab 100644 --- a/wurst/data/ArrayList.wurst +++ b/wurst/data/ArrayList.wurst @@ -115,6 +115,8 @@ public class ArrayList private int startIndex private int capacity private int size = 0 + // Number of slots intentionally retained by reset(). They are released by clear()/destroy. + private int retainedSize = 0 /** Creates a new empty list with default capacity (16) */ construct() @@ -272,7 +274,10 @@ public class ArrayList ondestroy // Clear references - for i = 0 to size - 1 + var slotsToClear = size + if retainedSize > slotsToClear + slotsToClear = retainedSize + for i = 0 to slotsToClear - 1 store[startIndex + i] = null // Return storage to free pool @@ -429,9 +434,23 @@ public class ArrayList O(1) on Jass; O(n) on Lua, where the slots are nulled so the GC can reclaim them. */ function clear() if isLua - for i = 0 to size - 1 + var slotsToClear = size + if retainedSize > slotsToClear + slotsToClear = retainedSize + for i = 0 to slotsToClear - 1 store[startIndex + i] = null size = 0 + retainedSize = 0 + + /** Resets the logical size while retaining both capacity and old slot references (O(1)). + + This is intended for hot scratch-list reuse, where subsequent writes replace the stale + slots and the caller accepts that values remain GC-reachable up to the list's historical + high-water mark. Use #clear when releasing those references matters. */ + function reset() + if size > retainedSize + retainedSize = size + size = 0 /** Returns a shallow copy of this list */ function copy() returns ArrayList diff --git a/wurst/data/ArrayListTests.wurst b/wurst/data/ArrayListTests.wurst index 6306bbbd..2ac9d0a6 100644 --- a/wurst/data/ArrayListTests.wurst +++ b/wurst/data/ArrayListTests.wurst @@ -134,6 +134,18 @@ function testFilter() destroy list destroy filtered +@Test +function testResetRetainsCapacityAndReplacesLogicalContents() + let list = new ArrayList(2) + list.add(1, 2) + list.reset() + list.size().assertEquals(0) + list.add(3, 4) + list.size().assertEquals(2) + list.get(0).assertEquals(3) + list.get(1).assertEquals(4) + destroy list + @Test function testFoldl() let list = new ArrayList() diff --git a/wurst/data/SparseSet.wurst b/wurst/data/SparseSet.wurst index 21842ef3..1fa9fe8c 100644 --- a/wurst/data/SparseSet.wurst +++ b/wurst/data/SparseSet.wurst @@ -20,18 +20,22 @@ public interface SparseSetKey * * Removal swaps the last element into the removed element's slot, so dense * iteration order is not preserved. + * + * Membership currently uses Warcraft's hashtable-backed Table API. Wurst cannot yet express a + * per-instance native Lua table, so Lua callers that only need dense iteration should prefer an + * ArrayList. The initial-capacity constructor avoids dense-list growth when a set is required. */ -public class SparseSet +public class SparseSet extends Table private readonly ArrayList dense - private readonly ArrayList denseKeys - private readonly Table sparse private readonly SparseSetKey keyProvider construct(SparseSetKey keyProvider) this.keyProvider = keyProvider dense = new ArrayList() - denseKeys = new ArrayList() - sparse = new Table() + + construct(SparseSetKey keyProvider, int initialCapacity) + this.keyProvider = keyProvider + dense = new ArrayList(initialCapacity) /** Adds an element and returns whether it was newly inserted. */ function add(T value) returns boolean @@ -45,8 +49,7 @@ public class SparseSet removeAt(existingIndex) dense.add(value) - denseKeys.add(key) - sparse.saveInt(key, dense.size()) + saveInt(key, dense.size()) return true /** Adds every element from another set. */ @@ -60,7 +63,7 @@ public class SparseSet /** Returns whether the set contains an element under the given key. */ function hasKey(int key) returns boolean - return sparse.hasInt(key) + return hasInt(key) /** Returns the dense index of an element, or -1 when it is absent. */ function indexOf(T value) returns int @@ -85,25 +88,30 @@ public class SparseSet let lastIndex = dense.size() - 1 let removed = dense.get(index) - let removedKey = denseKeys.get(index) + let removedKey = keyProvider.getKey(removed) if index != lastIndex let moved = dense.get(lastIndex) - let movedKey = denseKeys.get(lastIndex) + let movedKey = keyProvider.getKey(moved) dense.set(index, moved) - denseKeys.set(index, movedKey) - sparse.saveInt(movedKey, index + 1) + saveInt(movedKey, index + 1) dense.removeAtUnordered(lastIndex) - denseKeys.removeAtUnordered(lastIndex) - sparse.removeInt(removedKey) + removeInt(removedKey) return removed /** Removes all elements while retaining the set object. */ function clear() dense.clear() - denseKeys.clear() - sparse.flush() + flush() + + /** Resets membership in O(1) collection work while retaining dense slot references. + + Intended for hot scratch-set reuse. Use #clear when releasing the values for Lua GC + matters more than avoiding a dense traversal. */ + function reset() + dense.reset() + flush() /** Returns the number of elements in the set. */ function size() returns int @@ -130,14 +138,12 @@ public class SparseSet return result private function indexForKey(int key) returns int - if not sparse.hasInt(key) + if not hasInt(key) return -1 - return sparse.loadInt(key) - 1 + return loadInt(key) - 1 ondestroy destroy dense - destroy denseKeys - destroy sparse /** * Key provider for unit sets. diff --git a/wurst/data/SparseSetTests.wurst b/wurst/data/SparseSetTests.wurst index 0a22eb7d..8d030a89 100644 --- a/wurst/data/SparseSetTests.wurst +++ b/wurst/data/SparseSetTests.wurst @@ -114,3 +114,17 @@ function testSignedKeys() set.hasKey(-17).assertFalse() destroy set + +@Test +function testResetDropsMembershipAndAllowsReuse() + let set = new SparseSet(new IntSparseSetKey(), 2) + set.add(4) + set.add(9) + set.reset() + + set.size().assertEquals(0) + set.has(4).assertFalse() + set.has(9).assertFalse() + set.add(12).assertTrue() + set.has(12).assertTrue() + destroy set From 2939413515b126514cc6f707705f8d9f850f0fe2 Mon Sep 17 00:00:00 2001 From: Frotty Date: Thu, 10 Sep 2026 11:00:15 +0200 Subject: [PATCH 2/5] Clear retained ArrayList slots on regrow --- wurst/data/ArrayList.wurst | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/wurst/data/ArrayList.wurst b/wurst/data/ArrayList.wurst index 5348c6ab..d7c2eb21 100644 --- a/wurst/data/ArrayList.wurst +++ b/wurst/data/ArrayList.wurst @@ -257,12 +257,19 @@ public class ArrayList for i = 0 to size - 1 store[startIndex + i] = store[oldStart + i] - // The old section now holds duplicate references and no longer belongs to any - // live list once freed. Clear it so the Lua GC can reclaim them (no-op on Jass). + // The old section now holds duplicate references plus any stale references retained by + // reset(). It no longer belongs to a live list once freed, so clear the full high-water + // range before returning it to the pool (no-op on Jass). if isLua - for i = 0 to size - 1 + var slotsToClear = size + if retainedSize > slotsToClear + slotsToClear = retainedSize + for i = 0 to slotsToClear - 1 store[oldStart + i] = null + // The new section contains only the current logical elements. + retainedSize = 0 + // Free old section let tempStart = startIndex let tempCap = capacity @@ -792,4 +799,3 @@ public function ArrayList.joinBy(string separator) returns string /** Joins elements from a string list into one string */ public function ArrayList.join() returns string return this.joinBy("") - From ba73af13982fabf220dfa1b3590445ead5833513 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 11 Sep 2026 21:06:58 +0200 Subject: [PATCH 3/5] Take a scratch list from the caller and drop SparseSet. A spatial query returns something you walk, so a list is the shape it wants. The set overloads existed so a caller could test membership afterwards, but that is the wrong place for it: pass a filter and it runs once per match, before the result is built, instead of collecting everything and asking questions about it later. With those gone SparseSet has no caller. It existed to answer membership without a Warcraft group, and it paid for that with an ArrayList, a Table and a caller-supplied key provider per set - overhead that never bought anything here, because the query results were being iterated either way. Removes the type and its tests and benchmark. The remaining overloads take a reusable ArrayList, reset before use, so one scratch list serves a hot path without allocating. --- .../SpatialIndexForDestructables.wurst | 36 +-- wurst/closures/SpatialIndexForUnits.wurst | 68 +---- wurst/data/SparseSet.wurst | 170 ------------- wurst/data/SparseSetBenchmark.wurst | 234 ------------------ wurst/data/SparseSetTests.wurst | 130 ---------- 5 files changed, 10 insertions(+), 628 deletions(-) delete mode 100644 wurst/data/SparseSet.wurst delete mode 100644 wurst/data/SparseSetBenchmark.wurst delete mode 100644 wurst/data/SparseSetTests.wurst diff --git a/wurst/closures/SpatialIndexForDestructables.wurst b/wurst/closures/SpatialIndexForDestructables.wurst index aafa2ca7..1258e353 100644 --- a/wurst/closures/SpatialIndexForDestructables.wurst +++ b/wurst/closures/SpatialIndexForDestructables.wurst @@ -1,14 +1,14 @@ package SpatialIndexForDestructables import ArrayList -import SparseSet import DestructableSpatialIndex import Rect /** * Native-less Lua spatial queries for destructables. * - * Pass a reusable ArrayList for dense iteration, or a reusable - * SparseSet when membership operations are needed. The target is reset before use. + * Pass a reusable ArrayList and the query fills it. The target is reset before it is + * populated, so one scratch list serves every call. To narrow what comes back, pass a filter rather + * than collecting everything and testing membership afterwards. * Filters are caller-owned, and nested queries must use separate result collections. * The range query intentionally matches * ClosureForGroups.forDestructablesInRange: it returns the square that encloses the circle rather @@ -30,17 +30,6 @@ function appendMatches(ArrayList result, int matched, Destructable result.add(d) destructableSpatialIndexEndQuery() -function appendMatches(SparseSet result, int matched, DestructableSpatialFilter filter) - if filter == null - for i = 0 to matched - 1 - result.add(destructableSpatialIndexQuery(i)) - else - for i = 0 to matched - 1 - let d = destructableSpatialIndexQuery(i) - if filter.matches(d) - result.add(d) - destructableSpatialIndexEndQuery() - public function destructablesInRect(ArrayList result, rect area, DestructableSpatialFilter filter) result.reset() @@ -51,16 +40,6 @@ public function destructablesInRect(ArrayList result, rect area, public function destructablesInRect(ArrayList result, rect area) destructablesInRect(result, area, null) -public function destructablesInRect(SparseSet result, rect area, - DestructableSpatialFilter filter) - result.reset() - if isLua and USE_DESTRUCTABLE_SPATIAL_INDEX - appendMatches(result, destructableSpatialIndexBeginBoxQuery( - vec2(area.getMinX(), area.getMinY()), vec2(area.getMaxX(), area.getMaxY())), filter) - -public function destructablesInRect(SparseSet result, rect area) - destructablesInRect(result, area, null) - public function destructablesInRange(ArrayList result, vec2 center, real range, DestructableSpatialFilter filter) result.reset() @@ -69,12 +48,3 @@ public function destructablesInRange(ArrayList result, vec2 center public function destructablesInRange(ArrayList result, vec2 center, real range) destructablesInRange(result, center, range, null) - -public function destructablesInRange(SparseSet result, vec2 center, real range, - DestructableSpatialFilter filter) - result.reset() - if isLua and USE_DESTRUCTABLE_SPATIAL_INDEX - appendMatches(result, destructableSpatialIndexBeginRangeQuery(center, range), filter) - -public function destructablesInRange(SparseSet result, vec2 center, real range) - destructablesInRange(result, center, range, null) diff --git a/wurst/closures/SpatialIndexForUnits.wurst b/wurst/closures/SpatialIndexForUnits.wurst index edc6753f..fb63bd85 100644 --- a/wurst/closures/SpatialIndexForUnits.wurst +++ b/wurst/closures/SpatialIndexForUnits.wurst @@ -1,15 +1,18 @@ package SpatialIndexForUnits import ArrayList -import SparseSet import UnitSpatialIndex import Rect /** * Native-less Lua spatial queries. * - * Pass a reusable ArrayList for the cheapest dense result, or a reusable SparseSet when - * membership operations are needed. The target is reset before it is populated. No Warcraft group, - * result collection, or filter is allocated or destroyed by these overloads. + * Pass a reusable ArrayList and the query fills it. The target is reset before it is + * populated, so one scratch list serves every call on a hot path. No Warcraft group, result + * collection, or filter is allocated or destroyed by these overloads. + * + * A query result is something you walk, so a list is the shape it wants. To narrow what comes back, + * pass a UnitSpatialFilter rather than collecting everything and testing membership afterwards - + * the filter runs once per match, before the result is built. * * Filters are caller-owned so one instance can be reused across queries. A capturing lambda creates * a closure at its declaration site; keep it outside hot loops when allocation matters. @@ -34,17 +37,6 @@ function appendMatches(ArrayList result, int matched, UnitSpatialFilter fi result.add(u) spatialIndexEndQuery() -function appendMatches(SparseSet result, int matched, UnitSpatialFilter filter) - if filter == null - for i = 0 to matched - 1 - result.add(spatialIndexQueryUnit(i)) - else - for i = 0 to matched - 1 - let u = spatialIndexQueryUnit(i) - if filter.matches(u) - result.add(u) - spatialIndexEndQuery() - /** Replaces result with units whose origins are within radius of center. */ public function unitsInRange(ArrayList result, vec2 center, real radius) unitsInRange(result, center, radius, false, null) @@ -66,27 +58,6 @@ public function unitsInRange(ArrayList result, vec2 center, real radius, if isLua and USE_UNIT_SPATIAL_INDEX appendMatches(result, spatialIndexBeginQuery(center, radius, collisionFiltering), filter) -/** Replaces result with units whose origins are within radius of center. */ -public function unitsInRange(SparseSet result, vec2 center, real radius) - unitsInRange(result, center, radius, false, null) - -/** Replaces result with filtered units whose origins are within radius of center. */ -public function unitsInRange(SparseSet result, vec2 center, real radius, - UnitSpatialFilter filter) - unitsInRange(result, center, radius, false, filter) - -/** Replaces result with units in range, optionally applying collision-size filtering. */ -public function unitsInRange(SparseSet result, vec2 center, real radius, - boolean collisionFiltering) - unitsInRange(result, center, radius, collisionFiltering, null) - -/** Replaces result with filtered units in range, optionally applying collision-size filtering. */ -public function unitsInRange(SparseSet result, vec2 center, real radius, - boolean collisionFiltering, UnitSpatialFilter filter) - result.reset() - if isLua and USE_UNIT_SPATIAL_INDEX - appendMatches(result, spatialIndexBeginQuery(center, radius, collisionFiltering), filter) - /** Replaces result with filtered units whose origins are inside the axis-aligned box. */ public function unitsInBox(ArrayList result, vec2 boxMin, vec2 boxMax, UnitSpatialFilter filter) @@ -97,16 +68,6 @@ public function unitsInBox(ArrayList result, vec2 boxMin, vec2 boxMax, public function unitsInBox(ArrayList result, vec2 boxMin, vec2 boxMax) unitsInBox(result, boxMin, boxMax, null) -/** Replaces result with filtered units whose origins are inside the axis-aligned box. */ -public function unitsInBox(SparseSet result, vec2 boxMin, vec2 boxMax, - UnitSpatialFilter filter) - result.reset() - if isLua and USE_UNIT_SPATIAL_INDEX - appendMatches(result, spatialIndexBeginBoxQuery(boxMin, boxMax), filter) - -public function unitsInBox(SparseSet result, vec2 boxMin, vec2 boxMax) - unitsInBox(result, boxMin, boxMax, null) - public function unitsInRect(ArrayList result, rect area, UnitSpatialFilter filter) // Warcraft's native unit rect enum starts 32 units above the requested minimum edge. unitsInBox(result, vec2(area.getMinX() + 32., area.getMinY() + 32.), @@ -115,13 +76,6 @@ public function unitsInRect(ArrayList result, rect area, UnitSpatialFilter public function unitsInRect(ArrayList result, rect area) unitsInRect(result, area, null) -public function unitsInRect(SparseSet result, rect area, UnitSpatialFilter filter) - unitsInBox(result, vec2(area.getMinX() + 32., area.getMinY() + 32.), - vec2(area.getMaxX(), area.getMaxY()), filter) - -public function unitsInRect(SparseSet result, rect area) - unitsInRect(result, area, null) - /** Replaces result with currently indexed units owned by owner. This is a linear registry scan; per-player secondary sets are intentionally not maintained. */ public function unitsOfPlayer(ArrayList result, player owner, UnitSpatialFilter filter) @@ -131,11 +85,3 @@ public function unitsOfPlayer(ArrayList result, player owner, UnitSpatialF public function unitsOfPlayer(ArrayList result, player owner) unitsOfPlayer(result, owner, null) - -public function unitsOfPlayer(SparseSet result, player owner, UnitSpatialFilter filter) - result.reset() - if isLua and USE_UNIT_SPATIAL_INDEX - appendMatches(result, spatialIndexBeginPlayerQuery(owner), filter) - -public function unitsOfPlayer(SparseSet result, player owner) - unitsOfPlayer(result, owner, null) diff --git a/wurst/data/SparseSet.wurst b/wurst/data/SparseSet.wurst deleted file mode 100644 index 1fa9fe8c..00000000 --- a/wurst/data/SparseSet.wurst +++ /dev/null @@ -1,170 +0,0 @@ -package SparseSet - -import ArrayList -import ErrorHandling -import Table -import public TypeCasting - -/** Supplies the stable, unique integer key used by a SparseSet. */ -public interface SparseSetKey - function getKey(T value) returns int - -/** - * A set with O(1) membership checks, insertion, and unordered removal. - * - * Elements are stored in a dense typed array. The sparse index maps the key - * supplied by SparseSetKey to the element's dense index. Keys must be unique - * and stable for the lifetime of an element in the set. A key may be reused - * after its old value is gone; add() validates the stored value before - * accepting the new one. - * - * Removal swaps the last element into the removed element's slot, so dense - * iteration order is not preserved. - * - * Membership currently uses Warcraft's hashtable-backed Table API. Wurst cannot yet express a - * per-instance native Lua table, so Lua callers that only need dense iteration should prefer an - * ArrayList. The initial-capacity constructor avoids dense-list growth when a set is required. - */ -public class SparseSet extends Table - private readonly ArrayList dense - private readonly SparseSetKey keyProvider - - construct(SparseSetKey keyProvider) - this.keyProvider = keyProvider - dense = new ArrayList() - - construct(SparseSetKey keyProvider, int initialCapacity) - this.keyProvider = keyProvider - dense = new ArrayList(initialCapacity) - - /** Adds an element and returns whether it was newly inserted. */ - function add(T value) returns boolean - let key = keyProvider.getKey(value) - let existingIndex = indexForKey(key) - if existingIndex >= 0 - if dense.get(existingIndex) == value - return false - // The key was reused by a different value. This is valid for - // handles whose old value has been destroyed or deindexed. - removeAt(existingIndex) - - dense.add(value) - saveInt(key, dense.size()) - return true - - /** Adds every element from another set. */ - function addAll(SparseSet other) - for i = 0 to other.size() - 1 - add(other.get(i)) - - /** Returns whether the set contains the given element. */ - function has(T value) returns boolean - return indexOf(value) >= 0 - - /** Returns whether the set contains an element under the given key. */ - function hasKey(int key) returns boolean - return hasInt(key) - - /** Returns the dense index of an element, or -1 when it is absent. */ - function indexOf(T value) returns int - let key = keyProvider.getKey(value) - let index = indexForKey(key) - if index < 0 or dense.get(index) != value - return -1 - return index - - /** Removes an element and returns whether it was present. */ - function remove(T value) returns boolean - let index = indexOf(value) - if index < 0 - return false - removeAt(index) - return true - - /** Removes the element at a dense index without preserving order. */ - function removeAt(int index) returns T - if index < 0 or index >= dense.size() - error("SparseSet: Index out of bounds: " + index) - - let lastIndex = dense.size() - 1 - let removed = dense.get(index) - let removedKey = keyProvider.getKey(removed) - - if index != lastIndex - let moved = dense.get(lastIndex) - let movedKey = keyProvider.getKey(moved) - dense.set(index, moved) - saveInt(movedKey, index + 1) - - dense.removeAtUnordered(lastIndex) - removeInt(removedKey) - return removed - - /** Removes all elements while retaining the set object. */ - function clear() - dense.clear() - flush() - - /** Resets membership in O(1) collection work while retaining dense slot references. - - Intended for hot scratch-set reuse. Use #clear when releasing the values for Lua GC - matters more than avoiding a dense traversal. */ - function reset() - dense.reset() - flush() - - /** Returns the number of elements in the set. */ - function size() returns int - return dense.size() - - /** Returns whether the set contains no elements. */ - function isEmpty() returns boolean - return dense.isEmpty() - - /** Returns an element by its dense index. */ - function get(int index) returns T - return dense.get(index) - - /** Returns an element by sparse key, or null when the key is absent. */ - function getByKey(int key) returns T - if not hasKey(key) - return null - return dense.get(indexForKey(key)) - - /** Returns a shallow copy of this set. */ - function copy() returns SparseSet - let result = new SparseSet(keyProvider) - result.addAll(this) - return result - - private function indexForKey(int key) returns int - if not hasInt(key) - return -1 - return loadInt(key) - 1 - - ondestroy - destroy dense - -/** - * Key provider for unit sets. - * - * This deliberately uses the native handle identity rather than UnitIndexer - * IDs. That matches native groups: membership is not automatically removed - * when a unit is deindexed. SparseSet validates the stored unit when a key is - * reused, so a new unit cannot silently inherit stale membership. - */ -public class UnitSparseSetKey implements SparseSetKey - override function getKey(unit value) returns int - return value.getTCHandleId() - -/** Reusable key provider for SparseSet. */ -public constant SparseSetKey UNIT_SPARSE_SET_KEY = new UnitSparseSetKey() - -/** Key provider for destructable sets. */ -public class DestructableSparseSetKey implements SparseSetKey - override function getKey(destructable value) returns int - return value.getTCHandleId() - -/** Reusable key provider for SparseSet. */ -public constant SparseSetKey DESTRUCTABLE_SPARSE_SET_KEY = new DestructableSparseSetKey() - diff --git a/wurst/data/SparseSetBenchmark.wurst b/wurst/data/SparseSetBenchmark.wurst deleted file mode 100644 index 110b36a5..00000000 --- a/wurst/data/SparseSetBenchmark.wurst +++ /dev/null @@ -1,234 +0,0 @@ -package SparseSetBenchmark - -import ClosureTimers -import HashSet -import SparseSet - -/* - Manual in-game benchmark for the Lua target. - - This uses the classic Warcraft III method: a fixed workload is repeated - from a short-period timer while the player watches the game's FPS counter. - Compare the lowest FPS reached by the HashSet and SparseSet phases. The - benchmark is deliberately not a unit test and is not run automatically. - - HOW TO USE - =========== - Import SparseSetBenchmark into a test map and type -sparsebench in game. - Keep the map, camera, graphics settings, and other running systems the - same for every build. After each phase the temporary set or group is - destroyed and the benchmark waits 2.5 seconds for FPS to recover before - starting the next phase. The workload is identical within each comparison - pair. - - A timer callback can be delayed by a heavy workload. That is expected: the - resulting FPS drop is the measurement. Increase the per-tick round - constants if the phases do not visibly affect FPS on the target machine. -*/ - -constant MEMBERSHIP_ELEMENTS = 256 -constant MEMBERSHIP_ROUNDS_PER_TICK = 25 -constant REMOVE_ELEMENTS = 64 -constant REMOVE_ROUNDS_PER_TICK = 5 -constant ITERATION_ELEMENTS = 512 -constant ITERATION_ROUNDS_PER_TICK = 10 -constant PHASE_COUNT = 7 - -constant BENCHMARK_INTERVAL = 0.05 -constant PHASE_TICKS = 60 -constant START_DELAY = 2. -constant RECOVERY_DELAY = 2.5 - -class BenchmarkIntKey implements SparseSetKey - override function getKey(int value) returns int - return value - -constant benchmarkKey = new BenchmarkIntKey() -constant benchmarkUnitKey = new UnitSparseSetKey() - -var phase = 0 -var phaseTick = 0 -var sink = 0 -var phaseStartSink = 0 -var phaseRunning = false -unit array benchmarkUnits - -function runHashSetMembership() - let set = new HashSet - for i = 0 to MEMBERSHIP_ELEMENTS - 1 - set.add(i) - - for round = 0 to MEMBERSHIP_ROUNDS_PER_TICK - 1 - for i = 0 to MEMBERSHIP_ELEMENTS - 1 - if set.has(i) - sink++ - if not set.has(i + MEMBERSHIP_ELEMENTS) - sink++ - - destroy set - -function runSparseSetMembership() - let set = new SparseSet(benchmarkKey) - for i = 0 to MEMBERSHIP_ELEMENTS - 1 - set.add(i) - - for round = 0 to MEMBERSHIP_ROUNDS_PER_TICK - 1 - for i = 0 to MEMBERSHIP_ELEMENTS - 1 - if set.has(i) - sink++ - if not set.has(i + MEMBERSHIP_ELEMENTS) - sink++ - - destroy set - -function runHashSetRemove() - ensureBenchmarkUnits() - let set = new HashSet - for round = 0 to REMOVE_ROUNDS_PER_TICK - 1 - for i = 0 to REMOVE_ELEMENTS - 1 - set.add(benchmarkUnits[i]) - for i = 0 to REMOVE_ELEMENTS - 1 - if set.remove(benchmarkUnits[i]) - sink++ - - destroy set - -function runSparseSetRemove() - ensureBenchmarkUnits() - let set = new SparseSet(benchmarkUnitKey) - for round = 0 to REMOVE_ROUNDS_PER_TICK - 1 - for i = 0 to REMOVE_ELEMENTS - 1 - set.add(benchmarkUnits[i]) - for i = 0 to REMOVE_ELEMENTS - 1 - if set.remove(benchmarkUnits[i]) - sink++ - - destroy set - -function runGroupRemove() - ensureBenchmarkUnits() - let set = CreateGroup() - for round = 0 to REMOVE_ROUNDS_PER_TICK - 1 - for i = 0 to REMOVE_ELEMENTS - 1 - set.add(benchmarkUnits[i]) - for i = 0 to REMOVE_ELEMENTS - 1 - if set.remove(benchmarkUnits[i]) > 0 - sink++ - - set.destr() - -function ensureBenchmarkUnits() - if benchmarkUnits[0] == null - for i = 0 to REMOVE_ELEMENTS - 1 - benchmarkUnits[i] = createUnit(DUMMY_PLAYER, 'hfoo', vec2(0., 0.), 0 .fromDeg()) - benchmarkUnits[i].hide() - benchmarkUnits[i].pause() - -function destroyBenchmarkUnits() - if benchmarkUnits[0] != null - for i = 0 to REMOVE_ELEMENTS - 1 - benchmarkUnits[i].remove() - benchmarkUnits[i] = null - -function runHashSetIteration() - let set = new HashSet - for i = 0 to ITERATION_ELEMENTS - 1 - set.add(i) - - for round = 0 to ITERATION_ROUNDS_PER_TICK - 1 - for i = 0 to set.size() - 1 - sink += set.get(i) - - destroy set - -function runSparseSetIteration() - let set = new SparseSet(benchmarkKey) - for i = 0 to ITERATION_ELEMENTS - 1 - set.add(i) - - for round = 0 to ITERATION_ROUNDS_PER_TICK - 1 - for i = 0 to set.size() - 1 - sink += set.get(i) - - destroy set - -function phaseName() returns string - if phase == 1 - return "HashSet membership" - else if phase == 2 - return "SparseSet membership" - else if phase == 3 - return "HashSet remove/re-add" - else if phase == 4 - return "SparseSet remove/re-add" - else if phase == 5 - return "Native group remove/re-add" - else if phase == 6 - return "HashSet iteration" - else if phase == 7 - return "SparseSet iteration" - return "unknown phase" - -function runCurrentWorkload() - if phase == 1 - runHashSetMembership() - else if phase == 2 - runSparseSetMembership() - else if phase == 3 - runHashSetRemove() - else if phase == 4 - runSparseSetRemove() - else if phase == 5 - runGroupRemove() - else if phase == 6 - runHashSetIteration() - else if phase == 7 - runSparseSetIteration() - - phaseTick++ - if phaseTick == PHASE_TICKS - if phase == 5 - destroyBenchmarkUnits() - phaseRunning = false - print("Finished " + phaseName() + ". Sink delta: " + (sink - phaseStartSink)) - print("Workload cleaned up. Waiting " + RECOVERY_DELAY + " seconds for FPS recovery.") - doAfter(RECOVERY_DELAY) -> - runNextPhase() - -function startPhaseWorkload() - phaseTick = 0 - phaseStartSink = sink - phaseRunning = true - print("Running " + phaseName() + ": " + PHASE_TICKS - + " ticks at " + BENCHMARK_INTERVAL + " seconds") - doPeriodicallyCounted(BENCHMARK_INTERVAL, PHASE_TICKS) (CallbackCounted _cb) -> - runCurrentWorkload() - -function startPhase() - if phase == 3 - ensureBenchmarkUnits() - print("Starting " + phaseName() + " in " + START_DELAY - + " seconds; watch the lowest FPS.") - doAfter(START_DELAY) -> - startPhaseWorkload() - -function runNextPhase() - phase++ - if phase <= PHASE_COUNT - startPhase() - else - print("SparseSet benchmark complete. Final sink: " + sink) - -public function startSparseSetBenchmark() - if phase != 0 - print("SparseSet benchmark is already running or complete.") - return - print("SparseSet benchmark: compare equal repeated Lua workloads.") - runNextPhase() - -init - let benchmarkTrigger = CreateTrigger() - for i = 0 to bj_MAX_PLAYER_SLOTS - 1 - benchmarkTrigger.registerPlayerChatEvent(players[i], "-sparsebench", true) - benchmarkTrigger.addAction(function startSparseSetBenchmark) - diff --git a/wurst/data/SparseSetTests.wurst b/wurst/data/SparseSetTests.wurst deleted file mode 100644 index 8d030a89..00000000 --- a/wurst/data/SparseSetTests.wurst +++ /dev/null @@ -1,130 +0,0 @@ -package SparseSetTests - -import SparseSet - -class IntSparseSetKey implements SparseSetKey - override function getKey(int value) returns int - return value - -class ReusedKeyValue - int id - int payload - - construct(int id, int payload) - this.id = id - this.payload = payload - -class ReusedKeyProvider implements SparseSetKey - override function getKey(ReusedKeyValue value) returns int - return value.id - -@Test -function testAddAndMembership() - let set = new SparseSet(new IntSparseSetKey()) - set.add(4).assertTrue() - set.add(9).assertTrue() - set.add(4).assertFalse() - - set.size().assertEquals(2) - set.has(4).assertTrue() - set.has(9).assertTrue() - set.has(7).assertFalse() - set.getByKey(9).assertEquals(9) - - destroy set - -@Test -function testUnorderedRemovalKeepsMembership() - let set = new SparseSet(new IntSparseSetKey()) - set.add(1) - set.add(2) - set.add(3) - - set.remove(2).assertTrue() - set.remove(2).assertFalse() - set.size().assertEquals(2) - set.has(1).assertTrue() - set.has(3).assertTrue() - set.has(2).assertFalse() - - set.removeAt(0).assertEquals(1) - set.has(3).assertTrue() - set.size().assertEquals(1) - - destroy set - -@Test -function testClearAndCopy() - let original = new SparseSet(new IntSparseSetKey()) - original.add(2) - original.add(5) - - let copy = original.copy() - copy.size().assertEquals(2) - copy.has(2).assertTrue() - copy.remove(2).assertTrue() - original.has(2).assertTrue() - - original.clear() - original.isEmpty().assertTrue() - original.add(8).assertTrue() - original.has(8).assertTrue() - - destroy copy - destroy original - -@Test -function testReusedKeyReplacesStaleValue() - let set = new SparseSet(new ReusedKeyProvider()) - let oldValue = new ReusedKeyValue(17, 1) - let newValue = new ReusedKeyValue(17, 2) - - set.add(oldValue).assertTrue() - set.has(newValue).assertFalse() - set.add(newValue).assertTrue() - - set.size().assertEquals(1) - set.has(oldValue).assertFalse() - set.has(newValue).assertTrue() - set.get(0).payload.assertEquals(2) - - destroy set - destroy oldValue - destroy newValue - -@Test -function testLargeKeyDoesNotMaterializeGaps() - let set = new SparseSet(new IntSparseSetKey()) - set.add(1000000).assertTrue() - set.has(1000000).assertTrue() - set.size().assertEquals(1) - destroy set - -@Test -function testSignedKeys() - let set = new SparseSet(new IntSparseSetKey()) - set.add(-17).assertTrue() - set.add(23).assertTrue() - - set.has(-17).assertTrue() - set.hasKey(-17).assertTrue() - set.getByKey(-17).assertEquals(-17) - set.remove(-17).assertTrue() - set.has(-17).assertFalse() - set.hasKey(-17).assertFalse() - - destroy set - -@Test -function testResetDropsMembershipAndAllowsReuse() - let set = new SparseSet(new IntSparseSetKey(), 2) - set.add(4) - set.add(9) - set.reset() - - set.size().assertEquals(0) - set.has(4).assertFalse() - set.has(9).assertFalse() - set.add(12).assertTrue() - set.has(12).assertTrue() - destroy set From f7aedcab534d268cfa41203c192578fcbfcc7d68 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 12 Sep 2026 09:07:55 +0200 Subject: [PATCH 4/5] Fill a query result with one capacity check instead of one per element. add cannot be inlined: its body calls grow() when the list is full, so an appending loop pays a call, a stack-trace push, and a global table lookup for every field it touches instead of a hoisted local. Measured on Lua, an append which cannot grow costs about a quarter of one which can - 36ns against 134ns per element - and the difference is the inlining, not the capacity check. A spatial query is told how many matches there are before it copies any of them out, so it can reserve once and then append without checking. reserve becomes public for that, and unsafeAdd is the append which assumes it. unsafeAdd is documented as needing the reserve, and why: the backing store is one array shared by every list, so writing past this list's capacity does not overflow into nothing, it overwrites another list's section. --- .../SpatialIndexForDestructables.wurst | 7 ++- wurst/closures/SpatialIndexForUnits.wurst | 7 ++- wurst/data/ArrayList.wurst | 25 +++++++++- wurst/data/ArrayListTests.wurst | 47 +++++++++++++++++++ 4 files changed, 80 insertions(+), 6 deletions(-) diff --git a/wurst/closures/SpatialIndexForDestructables.wurst b/wurst/closures/SpatialIndexForDestructables.wurst index 1258e353..91cb5090 100644 --- a/wurst/closures/SpatialIndexForDestructables.wurst +++ b/wurst/closures/SpatialIndexForDestructables.wurst @@ -20,14 +20,17 @@ public interface DestructableSpatialFilter function matches(destructable whichDestructable) returns boolean function appendMatches(ArrayList result, int matched, DestructableSpatialFilter filter) + // One capacity check for the whole batch. The query already knows its upper bound, and + // an append which cannot grow contains no call, so it inlines into this loop. + result.reserve(result.size() + matched) if filter == null for i = 0 to matched - 1 - result.add(destructableSpatialIndexQuery(i)) + result.unsafeAdd(destructableSpatialIndexQuery(i)) else for i = 0 to matched - 1 let d = destructableSpatialIndexQuery(i) if filter.matches(d) - result.add(d) + result.unsafeAdd(d) destructableSpatialIndexEndQuery() public function destructablesInRect(ArrayList result, rect area, diff --git a/wurst/closures/SpatialIndexForUnits.wurst b/wurst/closures/SpatialIndexForUnits.wurst index fb63bd85..2c2dfdfb 100644 --- a/wurst/closures/SpatialIndexForUnits.wurst +++ b/wurst/closures/SpatialIndexForUnits.wurst @@ -27,14 +27,17 @@ public interface UnitSpatialFilter function matches(unit whichUnit) returns boolean function appendMatches(ArrayList result, int matched, UnitSpatialFilter filter) + // One capacity check for the whole batch. The query already knows its upper bound, and + // an append which cannot grow contains no call, so it inlines into this loop. + result.reserve(result.size() + matched) if filter == null for i = 0 to matched - 1 - result.add(spatialIndexQueryUnit(i)) + result.unsafeAdd(spatialIndexQueryUnit(i)) else for i = 0 to matched - 1 let u = spatialIndexQueryUnit(i) if filter.matches(u) - result.add(u) + result.unsafeAdd(u) spatialIndexEndQuery() /** Replaces result with units whose origins are within radius of center. */ diff --git a/wurst/data/ArrayList.wurst b/wurst/data/ArrayList.wurst index d7c2eb21..e3872874 100644 --- a/wurst/data/ArrayList.wurst +++ b/wurst/data/ArrayList.wurst @@ -302,6 +302,24 @@ public class ArrayList store[startIndex + size] = elem size++ + /** Appends without checking capacity. **Reserve first.** + + `add` cannot be inlined, because its body calls `grow()` when the list is full, and an + appending loop therefore pays a call, a stack-trace push and a global lookup for every + field it touches. This one contains no call, so it inlines into the caller and reads its + fields from hoisted locals - measured at roughly a quarter of the cost of `add` on Lua. + + The precondition is not a formality. The backing store is one array shared by every list, + each holding a section of it, so writing past this list's capacity does not overflow into + nothing - it silently overwrites whatever another list is keeping there. Call + `reserve(size + count)` first, and only append that many. + + Intended for a caller which already knows how many elements it is about to add, such as a + query copying out a result whose size it was told up front. Prefer `add` everywhere else. */ + function unsafeAdd(T elem) + store[startIndex + size] = elem + size++ + /** Adds all elements from another list */ function addAll(ArrayList other) let otherSize = other.size @@ -315,8 +333,11 @@ public class ArrayList size++ /** Ensures capacity for at least the given number of elements, moving the - section at most once instead of once per doubling. */ - private function reserve(int needed) + section at most once instead of once per doubling. + + Public so a caller which knows its final size can pay one capacity check for a whole batch + rather than one per element, which is also the precondition `unsafeAdd` needs. */ + function reserve(int needed) if needed <= capacity return var newCapacity = capacity diff --git a/wurst/data/ArrayListTests.wurst b/wurst/data/ArrayListTests.wurst index 2ac9d0a6..d064ebb8 100644 --- a/wurst/data/ArrayListTests.wurst +++ b/wurst/data/ArrayListTests.wurst @@ -738,3 +738,50 @@ function testDetectRepeatedSlotsBug() destroy seen destroy l + +@Test +function testReserveKeepsContentsAndGrowsCapacity() + let list = new ArrayList() + list.add(1) + list.add(2) + list.reserve(64) + // Reserving must not disturb what is already there, only where it can fit. + list.size().assertEquals(2) + list.get(0).assertEquals(1) + list.get(1).assertEquals(2) + // And it must be enough to append that many without growing again. + for i = 0 to 61 + list.unsafeAdd(i) + list.size().assertEquals(64) + list.get(63).assertEquals(61) + destroy list + +@Test +function testUnsafeAddAppendsLikeAdd() + let reserved = new ArrayList() + let plain = new ArrayList() + reserved.reserve(16) + for i = 0 to 15 + reserved.unsafeAdd(i) + plain.add(i) + reserved.size().assertEquals(plain.size()) + for i = 0 to 15 + reserved.get(i).assertEquals(plain.get(i)) + destroy reserved + destroy plain + +@Test +function testUnsafeAddAfterResetRefillsInPlace() + let list = new ArrayList() + list.reserve(8) + for i = 0 to 7 + list.unsafeAdd(i) + // reset keeps the capacity, which is what makes a scratch list reusable without reserving again + list.reset() + list.size().assertEquals(0) + for i = 0 to 7 + list.unsafeAdd(100 + i) + list.size().assertEquals(8) + list.get(0).assertEquals(100) + list.get(7).assertEquals(107) + destroy list From 6c4f4b16b3457db92a4f6432bfe63a757673816f Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 12 Sep 2026 09:18:29 +0200 Subject: [PATCH 5/5] Close the query snapshot before running a caller's filter. A filter is caller code. It can fail, and it can yield. Running it while the global query snapshot is still open means a failure leaves that snapshot open for good - along with the unit handles it holds - and a yield lets another query overlap this one, after which the two read and close each other's snapshot. The matches are now copied out and the snapshot closed before the filter sees anything. Filtering then compacts what was appended, in place, so it still needs no second collection; nested queries already have to use separate result lists, which is what makes writing back into this one safe. truncate is the counterpart to reserve that the compaction needs: it drops everything past a size and keeps the capacity, like reset generalised. --- .../SpatialIndexForDestructables.wurst | 30 ++++++++++++------- wurst/closures/SpatialIndexForUnits.wurst | 30 ++++++++++++------- wurst/data/ArrayList.wurst | 11 +++++++ wurst/data/ArrayListTests.wurst | 27 +++++++++++++++++ 4 files changed, 76 insertions(+), 22 deletions(-) diff --git a/wurst/closures/SpatialIndexForDestructables.wurst b/wurst/closures/SpatialIndexForDestructables.wurst index 91cb5090..f565221b 100644 --- a/wurst/closures/SpatialIndexForDestructables.wurst +++ b/wurst/closures/SpatialIndexForDestructables.wurst @@ -20,19 +20,27 @@ public interface DestructableSpatialFilter function matches(destructable whichDestructable) returns boolean function appendMatches(ArrayList result, int matched, DestructableSpatialFilter filter) - // One capacity check for the whole batch. The query already knows its upper bound, and - // an append which cannot grow contains no call, so it inlines into this loop. - result.reserve(result.size() + matched) - if filter == null - for i = 0 to matched - 1 - result.unsafeAdd(destructableSpatialIndexQuery(i)) - else - for i = 0 to matched - 1 - let d = destructableSpatialIndexQuery(i) - if filter.matches(d) - result.unsafeAdd(d) + // Copy the snapshot out and close it before any caller code runs. A filter is caller code: it + // can fail, and it can yield. Failing leaves the snapshot open for good, along with the unit + // handles it holds; yielding lets another query overlap this one, and the two then read and + // close each other's snapshot. + let firstNew = result.size() + result.reserve(firstNew + matched) + for i = 0 to matched - 1 + result.unsafeAdd(destructableSpatialIndexQuery(i)) destructableSpatialIndexEndQuery() + if filter != null + // Compact in place, so filtering needs no second collection. Nested queries already have + // to use separate result lists, which is what makes writing back into this one safe. + var kept = firstNew + for i = firstNew to result.size() - 1 + let match = result.get(i) + if filter.matches(match) + result.set(kept, match) + kept++ + result.truncate(kept) + public function destructablesInRect(ArrayList result, rect area, DestructableSpatialFilter filter) result.reset() diff --git a/wurst/closures/SpatialIndexForUnits.wurst b/wurst/closures/SpatialIndexForUnits.wurst index 2c2dfdfb..725c38bc 100644 --- a/wurst/closures/SpatialIndexForUnits.wurst +++ b/wurst/closures/SpatialIndexForUnits.wurst @@ -27,19 +27,27 @@ public interface UnitSpatialFilter function matches(unit whichUnit) returns boolean function appendMatches(ArrayList result, int matched, UnitSpatialFilter filter) - // One capacity check for the whole batch. The query already knows its upper bound, and - // an append which cannot grow contains no call, so it inlines into this loop. - result.reserve(result.size() + matched) - if filter == null - for i = 0 to matched - 1 - result.unsafeAdd(spatialIndexQueryUnit(i)) - else - for i = 0 to matched - 1 - let u = spatialIndexQueryUnit(i) - if filter.matches(u) - result.unsafeAdd(u) + // Copy the snapshot out and close it before any caller code runs. A filter is caller code: it + // can fail, and it can yield. Failing leaves the snapshot open for good, along with the unit + // handles it holds; yielding lets another query overlap this one, and the two then read and + // close each other's snapshot. + let firstNew = result.size() + result.reserve(firstNew + matched) + for i = 0 to matched - 1 + result.unsafeAdd(spatialIndexQueryUnit(i)) spatialIndexEndQuery() + if filter != null + // Compact in place, so filtering needs no second collection. Nested queries already have + // to use separate result lists, which is what makes writing back into this one safe. + var kept = firstNew + for i = firstNew to result.size() - 1 + let match = result.get(i) + if filter.matches(match) + result.set(kept, match) + kept++ + result.truncate(kept) + /** Replaces result with units whose origins are within radius of center. */ public function unitsInRange(ArrayList result, vec2 center, real radius) unitsInRange(result, center, radius, false, null) diff --git a/wurst/data/ArrayList.wurst b/wurst/data/ArrayList.wurst index e3872874..531c6468 100644 --- a/wurst/data/ArrayList.wurst +++ b/wurst/data/ArrayList.wurst @@ -480,6 +480,17 @@ public class ArrayList retainedSize = size size = 0 + /** Drops everything past the given size, keeping capacity (O(1)). + + Like `reset`, this leaves the dropped slots holding their references until they are + overwritten or `clear` is called - which is what makes it O(1). */ + function truncate(int newSize) + if newSize < 0 or newSize > size + error("ArrayList: truncate out of bounds: " + newSize.toString()) + if size > retainedSize + retainedSize = size + size = newSize + /** Returns a shallow copy of this list */ function copy() returns ArrayList let list = new ArrayList(size) diff --git a/wurst/data/ArrayListTests.wurst b/wurst/data/ArrayListTests.wurst index d064ebb8..a41c160e 100644 --- a/wurst/data/ArrayListTests.wurst +++ b/wurst/data/ArrayListTests.wurst @@ -785,3 +785,30 @@ function testUnsafeAddAfterResetRefillsInPlace() list.get(0).assertEquals(100) list.get(7).assertEquals(107) destroy list + +@Test +function testTruncateKeepsPrefixAndCapacity() + let list = new ArrayList() + list.reserve(32) + for i = 0 to 15 + list.unsafeAdd(i) + list.truncate(4) + list.size().assertEquals(4) + list.get(0).assertEquals(0) + list.get(3).assertEquals(3) + // Capacity survives, so the list refills without growing again. + for i = 0 to 27 + list.unsafeAdd(100 + i) + list.size().assertEquals(32) + list.get(4).assertEquals(100) + destroy list + +@Test +function testTruncateToOwnSizeIsANoOp() + let list = new ArrayList() + list.add(7) + list.add(8) + list.truncate(2) + list.size().assertEquals(2) + list.get(1).assertEquals(8) + destroy list