diff --git a/packages/flame/benchmark/README.md b/packages/flame/benchmark/README.md index 16c4521fa2b..2bb2c966f3e 100644 --- a/packages/flame/benchmark/README.md +++ b/packages/flame/benchmark/README.md @@ -45,7 +45,12 @@ the benchmark results are printed above it. changes across many parents, and the y-sort pattern where a whole container reorders every tick. - `type_query_benchmark.dart`: maintenance and read cost of the - `register()`/`query()` type-query caches under mixed-type churn. + `register()`/`query()` type-query caches. The churn suite varies how + many types are registered and how much of the container each cache matches; + the read suite compares a cached `query()` against the `whereType()` + scan that an unregistered type falls back to. Together they say what a cache + is worth, and what an accidental registration (the thing that + `Component.strictQueryMode` turns into an error) costs. - `update_components_benchmark.dart`: end-to-end update pass with game-like logic and inputs on a two-level tree. - `render_components_benchmark.dart`: render pass over a randomized tree onto diff --git a/packages/flame/benchmark/type_query_benchmark.dart b/packages/flame/benchmark/type_query_benchmark.dart index b235dc18bc3..47851bde922 100644 --- a/packages/flame/benchmark/type_query_benchmark.dart +++ b/packages/flame/benchmark/type_query_benchmark.dart @@ -8,54 +8,100 @@ import 'common.dart'; const _dt = 1.0 / 60; +/// Builds a population of [amount] components in which one in five is a +/// `_MarkedComponent`, one in fifty is a `_RareComponent`, and the remaining +/// four in five are `_PlainComponent`s. +List _mixedComponents(int amount) { + return List.generate(amount, (i) { + if (i % 50 == 0) { + return _RareComponent(); + } + return i % 5 == 0 ? _MarkedComponent() : _PlainComponent(); + }); +} + +/// Which query caches are registered on the container that is churned by +/// [TypeQueryChurnBenchmark]. +enum QueryRegistrations { + none('no registered queries, whereType scan'), + marked('1 registered query (matches 1 in 5)'), + markedAndRare('2 registered queries (second matches 1 in 50)'), + markedAndPlain('2 registered queries (second matches 4 in 5)'); + + const QueryRegistrations(this.label); + + final String label; + + /// Whether `_MarkedComponent` has a cache, and reads can go through + /// `query()` instead of a `whereType()` scan. + bool get isMarkedRegistered => this != none; + + void applyTo(ComponentList children) { + if (this != none) { + children.register<_MarkedComponent>(); + } + if (this == markedAndRare) { + children.register<_RareComponent>(); + } + if (this == markedAndPlain) { + children.register<_PlainComponent>(); + } + } +} + /// Measures the per-type query caches of the children container /// (`children.register()` / `children.query()`), which Flame uses /// internally for hitboxes (`GestureHitboxes`), post-processing /// (`CameraComponent`), and layout (`LinearLayoutComponent`). /// -/// Every add and remove has to update all registered caches, so this -/// benchmark churns a mixed-type population in a container with two -/// registered queries while reading one query per tick. A children-container -/// replacement must keep both the cache-maintenance and the query-read cost -/// at least this fast. +/// Every add and remove has to update all registered caches, so this benchmark +/// churns a mixed-type population while reading one query per tick. A +/// children-container replacement must keep both the cache-maintenance and the +/// query-read cost at least this fast. +/// +/// The [registrations] variants separate the two things that could drive the +/// maintenance cost: +/// - [QueryRegistrations.marked] against [QueryRegistrations.markedAndRare]: +/// one more cache, but one that almost never matches, so only the per-add +/// and per-remove type check is added; +/// - [QueryRegistrations.markedAndRare] against +/// [QueryRegistrations.markedAndPlain]: the same number of caches, but the +/// second one now holds most of the container, so the cost of maintaining a +/// cache entry itself dominates; +/// - [QueryRegistrations.none] against [QueryRegistrations.marked]: the whole +/// trade, cache maintenance on every structural change against a +/// `whereType` scan on every read. The no-cache variant reads through +/// `whereType()`, which falls back to a linear scan of the backing array +/// when no cache exists for the type. class TypeQueryChurnBenchmark extends AsyncBenchmarkBase { static const _amountStatic = 1000; static const _batchSize = 50; static const _liveBatches = 5; static const _amountTicks = 60; - static const _markedInterval = 5; + + final QueryRegistrations registrations; late final FlameGame _game; final Queue> _batches = Queue(); - TypeQueryChurnBenchmark() : super('Type-query churn (2 registered queries)'); + TypeQueryChurnBenchmark({ + this.registrations = QueryRegistrations.markedAndPlain, + }) : super('Type-query churn (${registrations.label})'); static Future main() async { - await TypeQueryChurnBenchmark().report(); - } - - List _newBatch() { - return List.generate( - _batchSize, - (i) => i % _markedInterval == 0 ? _MarkedComponent() : _PlainComponent(), - ); + for (final registrations in QueryRegistrations.values) { + await TypeQueryChurnBenchmark(registrations: registrations).report(); + } } @override Future setup() async { _game = FlameGame(); await mountGame(_game); - _game.world.children.register<_MarkedComponent>(); - _game.world.children.register<_PlainComponent>(); - await _game.world.addAll( - List.generate( - _amountStatic, - (i) => - i % _markedInterval == 0 ? _MarkedComponent() : _PlainComponent(), - ), - ); + registrations.applyTo(_game.world.children); + await _game.world.addAll(_mixedComponents(_amountStatic)); for (var i = 0; i < _liveBatches; i++) { - final batch = _newBatch(); + final batch = _mixedComponents(_batchSize); _batches.addLast(batch); await _game.world.addAll(batch); } @@ -64,14 +110,19 @@ class TypeQueryChurnBenchmark extends AsyncBenchmarkBase { @override Future run() async { + final children = _game.world.children; + final isMarkedRegistered = registrations.isMarkedRegistered; var visited = 0; for (var i = 0; i < _amountTicks; i++) { _game.world.removeAll(_batches.removeFirst()); - final batch = _newBatch(); + final batch = _mixedComponents(_batchSize); _batches.addLast(batch); await _game.world.addAll(batch); - for (final marked in _game.world.children.query<_MarkedComponent>()) { - visited += marked.marker; + final marked = isMarkedRegistered + ? children.query<_MarkedComponent>() + : children.whereType<_MarkedComponent>(); + for (final component in marked) { + visited += component.marker; } _game.update(_dt); } @@ -79,12 +130,86 @@ class TypeQueryChurnBenchmark extends AsyncBenchmarkBase { } } +/// Measures the read side of the query caches in isolation: [_amountReads] +/// repeated reads of every `_MarkedComponent` in a static container of +/// [amountChildren] children, one fifth of which match. +/// +/// The [cached] variant registers the type and reads through `query()`, +/// which returns a maintained list of exactly the matching children. The +/// uncached variant reads through `whereType()`, which, without a cache for +/// the type, scans the whole backing array. +/// +/// The gap between the two is what a cache buys on the read side, and it is +/// the number to weigh against the maintenance cost measured by +/// [TypeQueryChurnBenchmark]. It is measured at both a large container size +/// (where the scan has to skip many non-matching children) and at a typical +/// per-component size (where a query such as `GestureHitboxes.hitboxes` runs +/// over a handful of children). +class TypeQueryReadBenchmark extends AsyncBenchmarkBase { + static const _amountReads = 500; + + final int amountChildren; + final bool cached; + + late final FlameGame _game; + late final Component _parent; + + TypeQueryReadBenchmark({required this.amountChildren, required this.cached}) + : super( + 'Type-query read ($amountChildren children, ' + '${cached ? 'cached query' : 'whereType scan'})', + ); + + static Future main() async { + for (final amountChildren in [1000, 16]) { + for (final cached in [false, true]) { + await TypeQueryReadBenchmark( + amountChildren: amountChildren, + cached: cached, + ).report(); + } + } + } + + @override + Future setup() async { + _game = FlameGame(); + await mountGame(_game); + _parent = Component(); + await _game.world.add(_parent); + await _game.ready(); + if (cached) { + _parent.children.register<_MarkedComponent>(); + } + await _parent.addAll(_mixedComponents(amountChildren)); + await _game.ready(); + } + + @override + Future run() async { + final children = _parent.children; + var visited = 0; + for (var i = 0; i < _amountReads; i++) { + final marked = cached + ? children.query<_MarkedComponent>() + : children.whereType<_MarkedComponent>(); + for (final component in marked) { + visited += component.marker; + } + } + assert(visited > 0); + } +} + class _MarkedComponent extends Component { final int marker = 1; } class _PlainComponent extends Component {} +class _RareComponent extends Component {} + Future main() async { await TypeQueryChurnBenchmark.main(); + await TypeQueryReadBenchmark.main(); } diff --git a/packages/flame/lib/src/components/core/component_list.dart b/packages/flame/lib/src/components/core/component_list.dart index 033a92127b8..9a8bbc8c5f2 100644 --- a/packages/flame/lib/src/components/core/component_list.dart +++ b/packages/flame/lib/src/components/core/component_list.dart @@ -23,7 +23,11 @@ part of 'component.dart'; /// A removal does not shift the array, it leaves a `null` "tombstone" in /// place. Tombstones are invisible to iteration and are compacted away at the /// start of the next update tick, or earlier if they grow to dominate the -/// array. +/// array. The per-type [query] caches work the same way: a removal only marks +/// them, and the entries of the removed components are dropped in a single +/// pass before anything reads, reorders or adds to them again, so that a +/// removal stays O(1) no matter how many types are registered or how much of +/// the container each of them matches. /// /// Mutating the container while iterating it is allowed in the ways that the /// component lifecycle needs: removals take effect immediately (the removed @@ -96,6 +100,15 @@ class ComponentList extends Iterable { /// paths can iterate them without allocating a map-values iterator. List<_QueryCache>? _queryCaches; + /// Whether any of [_queryCaches] may still hold entries for components that + /// have since been removed from this list. + /// + /// Removals only raise this flag instead of searching every matching cache + /// for the removed component, which would be O(n) per removal. The stale + /// entries are dropped by [_compactQueryCaches], in a single pass that + /// covers any number of removals at once, before anything can observe them. + bool _hasStaleQueryEntries = false; + /// A monotonically increasing counter, bumped on every membership or order /// change of any [ComponentList] (adds, removes, clears, reorders). The /// root's flattened update list compares against this to know when it must @@ -198,6 +211,10 @@ class ComponentList extends Iterable { component._containerList == null, 'A component cannot be contained by two children containers at once', ); + // Must happen before [component] is linked to this list: a component that + // is removed and added back before the caches are compacted would + // otherwise be seen as a live entry and end up in a cache twice. + _compactQueryCaches(); final elements = _elements; if (_length == 0 && elements.isNotEmpty) { // The array contains only tombstones; reset it. @@ -274,16 +291,22 @@ class ComponentList extends Iterable { for (var i = 0; i < caches.length; i++) { final cache = caches[i]; if (cache.check(component)) { - cache.data.remove(component); + cache.hasStaleEntries = true; + _hasStaleQueryEntries = true; } } } if (_length == 0) { _elements.clear(); _tombstones = 0; + // Nothing is left to hold on to, so drop the stale cache entries right + // away instead of keeping the removed components alive until the next + // add or query. + _compactQueryCaches(); } else if (_tombstones >= _tombstoneCompactionThreshold && _tombstones * 2 >= _elements.length) { _compact(); + _compactQueryCaches(); } return true; } @@ -311,9 +334,12 @@ class ComponentList extends Iterable { final caches = _queryCaches; if (caches != null) { for (var i = 0; i < caches.length; i++) { - caches[i].data.clear(); + caches[i] + ..data.clear() + ..hasStaleEntries = false; } } + _hasStaleQueryEntries = false; } /// Restores the priority ordering after one or more elements have changed @@ -324,6 +350,10 @@ class ComponentList extends Iterable { /// components with equal priority keep their relative order. void rebalance() { _compact(); + // Not needed for correctness, since every read compacts as well, but it + // keeps [_QueryCache.resort] from ordering entries that are about to be + // dropped, by an element index that they no longer have. + _compactQueryCaches(); final elements = _elements; var isSorted = true; for (var i = 1; i < elements.length; i++) { @@ -377,6 +407,21 @@ class ComponentList extends Iterable { _shiftCount++; } + /// Drops the entries of removed components from the query caches, if any + /// removal has left some behind. + @pragma('vm:prefer-inline') + @pragma('wasm:prefer-inline') + void _compactQueryCaches() { + if (!_hasStaleQueryEntries) { + return; + } + _hasStaleQueryEntries = false; + final caches = _queryCaches!; + for (var i = 0; i < caches.length; i++) { + caches[i].compact(this); + } + } + /// Whether type [C] has been registered as a queryable type. bool isRegistered() { return _queries?.containsKey(C) ?? false; @@ -418,6 +463,7 @@ class ComponentList extends Iterable { register(); return query(); } + _compactQueryCaches(); // The cached list itself is returned, but typed as an Iterable to prevent // accidental modification of the cache from the outside. return cache.data as Iterable; @@ -427,6 +473,7 @@ class ComponentList extends Iterable { Iterable whereType() { final cache = _queries?[C]; if (cache != null) { + _compactQueryCaches(); return cache.data as Iterable; } return super.whereType(); @@ -529,8 +576,33 @@ class _QueryCache { final List data; + /// Whether [data] may hold entries for components that have since been + /// removed from the list; see [ComponentList._hasStaleQueryEntries]. + bool hasStaleEntries = false; + bool check(Component component) => component is C; + /// Drops the entries that are no longer in [list], in a single pass that + /// preserves the order of the remaining ones. + void compact(ComponentList list) { + if (!hasStaleEntries) { + return; + } + hasStaleEntries = false; + final data = this.data; + var write = 0; + for (var read = 0; read < data.length; read++) { + final element = data[read]; + if (identical(element._containerList, list)) { + if (write != read) { + data[write] = element; + } + write++; + } + } + data.length = write; + } + /// Inserts [component] into [data], keeping it ordered consistently with /// the main backing array (which orders by priority). void insertSorted(Component component) { diff --git a/packages/flame/test/components/core/component_list_query_test.dart b/packages/flame/test/components/core/component_list_query_test.dart new file mode 100644 index 00000000000..68469152db8 --- /dev/null +++ b/packages/flame/test/components/core/component_list_query_test.dart @@ -0,0 +1,215 @@ +import 'package:flame/collisions.dart'; +import 'package:flame/components.dart'; +import 'package:flame_test/flame_test.dart'; +import 'package:test/test.dart'; + +void main() { + group('ComponentList queries', () { + test('a removed component leaves the cache', () { + final list = ComponentList()..register<_Marked>(); + final marked = _Marked(1); + list + ..add(marked) + ..add(_Plain()); + + expect(list.query<_Marked>(), [marked]); + + list.remove(marked); + expect(list.query<_Marked>(), isEmpty); + }); + + test('removing and adding back does not duplicate the cache entry', () { + final list = ComponentList()..register<_Marked>(); + final marked = _Marked(1); + // A sibling keeps the list non-empty, so that the removal does not + // compact the cache on its own. + list + ..add(marked) + ..add(_Marked(2)); + + list + ..remove(marked) + ..add(marked); + + expect(list.query<_Marked>().map((c) => c.id), [2, 1]); + }); + + test('a component moved to another list leaves the first cache', () { + final source = ComponentList()..register<_Marked>(); + final target = ComponentList()..register<_Marked>(); + final marked = _Marked(1); + source + ..add(marked) + ..add(_Marked(2)); + target.add(_Marked(3)); + + source.remove(marked); + target.add(marked); + + expect(source.query<_Marked>().map((c) => c.id), [2]); + expect(target.query<_Marked>().map((c) => c.id), [3, 1]); + }); + + test('the cache keeps the list order across removals and additions', () { + final list = ComponentList()..register<_Marked>(); + final marked = List.generate(10, (i) => _Marked(i, priority: i)); + for (var i = 0; i < 10; i++) { + list + ..add(marked[i]) + ..add(_Plain(priority: i)); + } + + // Removals from the front, the middle and the end, all before anything + // reads the cache again. + list + ..remove(marked[0]) + ..remove(marked[4]) + ..remove(marked[5]) + ..remove(marked[9]); + expect(list.query<_Marked>().map((c) => c.id), [1, 2, 3, 6, 7, 8]); + + // A component that sorts into the middle lands in the right place. + final inserted = _Marked(99, priority: 4); + list.add(inserted); + expect(list.query<_Marked>().map((c) => c.id), [1, 2, 3, 99, 6, 7, 8]); + }); + + test('the cache is reordered after a rebalance that follows a removal', () { + final list = ComponentList()..register<_Marked>(); + final marked = List.generate(5, (i) => _Marked(i, priority: i)); + for (final component in marked) { + list.add(component); + } + + list.remove(marked[2]); + marked[0].priority = 10; + list.rebalance(); + + expect(list.query<_Marked>().map((c) => c.id), [1, 3, 4, 0]); + }); + + test('emptying the list empties the caches', () { + final list = ComponentList()..register<_Marked>(); + final marked = List.generate(3, _Marked.new); + for (final component in marked) { + list.add(component); + } + + for (final component in marked) { + list.remove(component); + } + + expect(list.query<_Marked>(), isEmpty); + expect(list, isEmpty); + }); + + test('clear empties the caches', () { + final list = ComponentList()..register<_Marked>(); + list + ..add(_Marked(1)) + ..add(_Plain()) + ..clear(); + + expect(list.query<_Marked>(), isEmpty); + }); + + test('removals past the tombstone compaction threshold', () { + final list = ComponentList()..register<_Marked>(); + // More than the threshold at which the backing array compacts itself. + final marked = List.generate(100, (i) => _Marked(i, priority: i)); + for (final component in marked) { + list.add(component); + } + + for (var i = 0; i < 100; i += 2) { + list.remove(marked[i]); + } + + expect( + list.query<_Marked>().map((c) => c.id), + [for (var i = 1; i < 100; i += 2) i], + ); + }); + + test('whereType sees removals as query does', () { + final list = ComponentList()..register<_Marked>(); + final marked = _Marked(1); + list.add(marked); + + list.remove(marked); + + expect(list.whereType<_Marked>(), isEmpty); + // Unregistered types scan the backing array instead of a cache. + expect(list.whereType<_Plain>(), isEmpty); + }); + + test('a cache of an unrelated type is unaffected by removals', () { + final list = ComponentList() + ..register<_Marked>() + ..register<_Plain>(); + final marked = _Marked(1); + final plain = _Plain(); + list + ..add(marked) + ..add(plain); + + list.remove(marked); + + expect(list.query<_Plain>(), [plain]); + expect(list.query<_Marked>(), isEmpty); + }); + + testWithFlameGame('queries follow the component lifecycle', (game) async { + game.world.children.register<_Marked>(); + final marked = List.generate(20, (i) => _Marked(i, priority: i)); + await game.world.ensureAddAll([ + ...marked, + ...List.generate(20, (i) => _Plain(priority: i)), + ]); + + expect(game.world.children.query<_Marked>(), marked); + + game.world.removeAll(marked.sublist(0, 10)); + game.update(0); + expect(game.world.children.query<_Marked>(), marked.sublist(10)); + + // Removals and additions within the same tick. + final added = _Marked(100, priority: 100); + game.world + ..removeAll(marked.sublist(10, 15)) + ..add(added); + game.update(0); + expect(game.world.children.query<_Marked>(), [ + ...marked.sublist(15), + added, + ]); + }); + + testWithFlameGame('hitbox queries follow removals', (game) async { + final component = _Hitboxes(); + final hitboxes = List.generate(3, (_) => RectangleHitbox()); + await game.world.ensureAdd(component); + await component.ensureAddAll(hitboxes); + + expect(component.hitboxes, hitboxes); + + hitboxes.first.removeFromParent(); + game.update(0); + expect(component.hitboxes, hitboxes.sublist(1)); + }); + }); +} + +class _Marked extends Component { + _Marked(this.id, {super.priority}); + + final int id; +} + +class _Plain extends Component { + _Plain({super.priority}); +} + +class _Hitboxes extends PositionComponent with GestureHitboxes { + _Hitboxes() : super(size: Vector2.all(10)); +}