From 9891f4eae930c2834e20bddfed1646dd45f3b71e Mon Sep 17 00:00:00 2001 From: Filip Hracek Date: Wed, 5 Feb 2025 15:01:37 +0100 Subject: [PATCH] perf: Switch from forEach to regular for-loops for about 30% improvement in raw update performance (#3472) Replaces uses of `.forEach()` with regular `for` loops. This has significant impact on performance in hot paths such as `Component.updateTree()` and `Component.renderTree()`. ![Updating Components](https://github.com/user-attachments/assets/e183ce4c-5b37-45d9-ad81-a0a35719e0dd) In the graph above, you see 50 runs of `benchmark/update_components_benchmark.dart`. The forEach results are blue, the for-loop results are green. I could see this effect after just replacing the `forEach` calls in `component.dart`. Data [here](https://docs.google.com/spreadsheets/d/e/2PACX-1vRk_yGmLN6o0oqSUWDBh7ODx7B8EIToeahZcZBS3VKHX8AbEnmrgmEqDt98cZLoBjIKQX3MlOc0XwsP/pubhtml). > Aside for posterity: `for i in {1..50}; do flutter test benchmark/main.dart --no-pub -r silent 2>/dev/null >> benchmarks_for_loop.txt; done`, then get the data from the text file. I went ahead and replaced additional `forEach` calls elsewhere in the engine codebase, but there was no additional effect on the benchmark. Still, I kept those changes in. I only replaced `forEach` in places that seemed relatively hot (e.g. `ComponentSet.reorder()`). There are more `forEach` calls in the codebase but those seem fine to me as they aren't likely to be called too often. It should be noted that I needed to update the benchmark to add children to the components. Every `_BenchmarkComponent` now has 10 children. This feels a bit more realistic use of the framework than having a flat array of components with no children. By changing the benchmark code in this way, I made it a bit slower, so I'm not sure if the effect will be seen in the CI/CD. I also tried whether the change will have effect on my game's benchmark (which is a lot more involved and uses `flutter driver` to test the whole game in AOT mode). For the game, the effect is negligible but that was kind of expected since my game spends a significant amount of its CPU time on AI, raycasting, smoke simulation and drawVertices, none of which really depend on the speed of the engine `update()` mechanism. --------- Co-authored-by: Erick --- .../update_components_benchmark.dart | 8 ++++++ packages/flame/lib/src/camera/viewport.dart | 4 ++- .../src/collisions/hitboxes/shape_hitbox.dart | 10 +++++--- .../standard_collision_detection.dart | 6 ++++- .../lib/src/components/core/component.dart | 25 ++++++++++++++----- .../src/components/core/component_set.dart | 4 ++- .../components/core/component_tree_root.dart | 4 +-- .../lib/src/components/mixins/has_paint.dart | 9 ++++--- .../src/components/position_component.dart | 5 ++-- .../src/components/text_box_component.dart | 4 +-- .../double_tap_dispatcher.dart | 8 ++++-- .../experimental/geometry/shapes/polygon.dart | 8 +++--- packages/flame/lib/src/game/flame_game.dart | 4 ++- .../flame/lib/src/geometry/line_segment.dart | 4 ++- packages/flame/lib/src/layers/layer.dart | 13 +++++++--- packages/flame/lib/src/parallax.dart | 4 +-- .../lib/src/text/elements/group_element.dart | 8 ++++-- 17 files changed, 91 insertions(+), 37 deletions(-) diff --git a/packages/flame/benchmark/update_components_benchmark.dart b/packages/flame/benchmark/update_components_benchmark.dart index baa322591a3..9914fa8f48a 100644 --- a/packages/flame/benchmark/update_components_benchmark.dart +++ b/packages/flame/benchmark/update_components_benchmark.dart @@ -7,6 +7,7 @@ import 'package:flame/game.dart'; const _amountComponents = 1000; const _amountTicks = 2000; const _amountInputs = 500; +const _amountChildren = 10; class UpdateComponentsBenchmark extends AsyncBenchmarkBase { final Random random; @@ -65,6 +66,13 @@ class _BenchmarkComponent extends PositionComponent { _BenchmarkComponent(this.id); + @override + Future onLoad() async { + for (var i = 0; i < _amountChildren; i++) { + await add(PositionComponent(position: Vector2(i * 2, 0))); + } + } + void input({ required int xDirection, required bool doJump, diff --git a/packages/flame/lib/src/camera/viewport.dart b/packages/flame/lib/src/camera/viewport.dart index 2fd3b511315..c8dbcb4a912 100644 --- a/packages/flame/lib/src/camera/viewport.dart +++ b/packages/flame/lib/src/camera/viewport.dart @@ -91,7 +91,9 @@ abstract class Viewport extends Component } onViewportResize(); if (hasChildren) { - children.forEach((child) => child.onParentResize(_size)); + for (final child in children) { + child.onParentResize(_size); + } } } diff --git a/packages/flame/lib/src/collisions/hitboxes/shape_hitbox.dart b/packages/flame/lib/src/collisions/hitboxes/shape_hitbox.dart index 5d37f29c168..8a5cded1461 100644 --- a/packages/flame/lib/src/collisions/hitboxes/shape_hitbox.dart +++ b/packages/flame/lib/src/collisions/hitboxes/shape_hitbox.dart @@ -89,10 +89,12 @@ mixin ShapeHitbox on ShapeComponent implements Hitbox { _validAabb = false; onAabbChanged?.call(); }; - ancestors(includeSelf: true).whereType().forEach((c) { - _transformAncestors.add(c.transform); - c.transform.addListener(_transformListener); - }); + final positionComponents = + ancestors(includeSelf: true).whereType(); + for (final ancestor in positionComponents) { + _transformAncestors.add(ancestor.transform); + ancestor.transform.addListener(_transformListener); + } if (shouldFillParent) { _parentSizeListener = () { diff --git a/packages/flame/lib/src/collisions/standard_collision_detection.dart b/packages/flame/lib/src/collisions/standard_collision_detection.dart index 20fadc8a325..6d8048768ca 100644 --- a/packages/flame/lib/src/collisions/standard_collision_detection.dart +++ b/packages/flame/lib/src/collisions/standard_collision_detection.dart @@ -167,7 +167,11 @@ class StandardCollisionDetection> List? ignoreHitboxes, List>? out, }) sync* { - out?.forEach((e) => e.reset()); + if (out != null) { + for (final result in out) { + result.reset(); + } + } var currentRay = ray; for (var i = 0; i < maxDepth; i++) { final hasResultObject = (out?.length ?? 0) > i; diff --git a/packages/flame/lib/src/components/core/component.dart b/packages/flame/lib/src/components/core/component.dart index 4fb0a4623b8..a0e61f76b0a 100644 --- a/packages/flame/lib/src/components/core/component.dart +++ b/packages/flame/lib/src/components/core/component.dart @@ -524,7 +524,12 @@ class Component { /// priority of the direct siblings, not the children or the ancestors. void updateTree(double dt) { update(dt); - _children?.forEach((c) => c.updateTree(dt)); + final children = _children; + if (children != null) { + for (final child in children) { + child.updateTree(dt); + } + } } /// This method will be invoked from lifecycle if [child] has been added @@ -535,7 +540,12 @@ class Component { void renderTree(Canvas canvas) { render(canvas); - _children?.forEach((c) => c.renderTree(canvas)); + final children = _children; + if (children != null) { + for (final child in children) { + child.renderTree(canvas); + } + } // Any debug rendering should be rendered on top of everything if (debugMode) { @@ -868,11 +878,14 @@ class Component { @mustCallSuper @internal void handleResize(Vector2 size) { - _children?.forEach((child) { - if (child.isLoading || child.isLoaded) { - child.onGameResize(size); + final children = _children; + if (children != null) { + for (final child in children) { + if (child.isLoading || child.isLoaded) { + child.onGameResize(size); + } } - }); + } } FutureOr _startLoading() { diff --git a/packages/flame/lib/src/components/core/component_set.dart b/packages/flame/lib/src/components/core/component_set.dart index 0f4e4fd99dc..ffecdaa7d34 100644 --- a/packages/flame/lib/src/components/core/component_set.dart +++ b/packages/flame/lib/src/components/core/component_set.dart @@ -106,6 +106,8 @@ class ComponentSet extends QueryableOrderedSet { final elements = toList(); // bypass the wrapper because the components are already added super.clear(); - elements.forEach(super.add); + for (final element in elements) { + super.add(element); + } } } diff --git a/packages/flame/lib/src/components/core/component_tree_root.dart b/packages/flame/lib/src/components/core/component_tree_root.dart index 7c680021659..cc1c24b45a8 100644 --- a/packages/flame/lib/src/components/core/component_tree_root.dart +++ b/packages/flame/lib/src/components/core/component_tree_root.dart @@ -158,12 +158,12 @@ class ComponentTreeRoot extends Component { @internal void handleResize(Vector2 size) { super.handleResize(size); - _queue.forEach((event) { + for (final event in _queue) { if ((event.kind == _LifecycleEventKind.add) && (event.child!.isLoading || event.child!.isLoaded)) { event.child!.onGameResize(size); } - }); + } } @mustCallSuper diff --git a/packages/flame/lib/src/components/mixins/has_paint.dart b/packages/flame/lib/src/components/mixins/has_paint.dart index 30ef95a3711..2d1c5545f57 100644 --- a/packages/flame/lib/src/components/mixins/has_paint.dart +++ b/packages/flame/lib/src/components/mixins/has_paint.dart @@ -208,9 +208,12 @@ class _MultiPaintOpacityProvider implements OpacityProvider { maxOpacity = max(target.getOpacity(paintId: paintId), maxOpacity); } if (includeLayers) { - target.paintLayersInternal?.forEach( - (paint) => maxOpacity = max(paint.color.a, maxOpacity), - ); + final targetLayers = target.paintLayersInternal; + if (targetLayers != null) { + for (final paint in targetLayers) { + maxOpacity = max(paint.color.a, maxOpacity); + } + } } return maxOpacity; diff --git a/packages/flame/lib/src/components/position_component.dart b/packages/flame/lib/src/components/position_component.dart index 39d49e97cde..2274d645258 100644 --- a/packages/flame/lib/src/components/position_component.dart +++ b/packages/flame/lib/src/components/position_component.dart @@ -1,5 +1,4 @@ import 'dart:math' as math; - import 'dart:ui' hide Offset; import 'package:collection/collection.dart'; @@ -195,7 +194,9 @@ class PositionComponent extends Component set size(Vector2 size) { _size.setFrom(size); if (hasChildren) { - children.forEach((child) => child.onParentResize(_size)); + for (final child in children) { + child.onParentResize(_size); + } } } diff --git a/packages/flame/lib/src/components/text_box_component.dart b/packages/flame/lib/src/components/text_box_component.dart index 7aece0d0e4d..dd4c1f23087 100644 --- a/packages/flame/lib/src/components/text_box_component.dart +++ b/packages/flame/lib/src/components/text_box_component.dart @@ -168,7 +168,7 @@ class TextBoxComponent extends TextComponent { lines.clear(); var lineHeight = 0.0; final maxBoxWidth = _fixedSize ? width : _boxConfig.maxWidth; - text.split(' ').forEach((word) { + for (final word in text.split(' ')) { final wordLines = word.split('\n'); final possibleLine = lines.isEmpty ? wordLines[0] : '${lines.last} ${wordLines[0]}'; @@ -192,7 +192,7 @@ class TextBoxComponent extends TextComponent { } else { lines.addAll(wordLines); } - }); + } _totalLines = lines.length; _lineHeight = lineHeight; size = _recomputeSize(); diff --git a/packages/flame/lib/src/events/flame_game_mixins/double_tap_dispatcher.dart b/packages/flame/lib/src/events/flame_game_mixins/double_tap_dispatcher.dart index 1a1efa5f78d..f3029bf67dc 100644 --- a/packages/flame/lib/src/events/flame_game_mixins/double_tap_dispatcher.dart +++ b/packages/flame/lib/src/events/flame_game_mixins/double_tap_dispatcher.dart @@ -36,12 +36,16 @@ class DoubleTapDispatcher extends Component with HasGameReference { } void _onDoubleTapUp(DoubleTapEvent event) { - _components.forEach((component) => component.onDoubleTapUp(event)); + for (final component in _components) { + component.onDoubleTapUp(event); + } _components.clear(); } void _onDoubleTapCancel(DoubleTapCancelEvent event) { - _components.forEach((component) => component.onDoubleTapCancel(event)); + for (final component in _components) { + component.onDoubleTapCancel(event); + } _components.clear(); } diff --git a/packages/flame/lib/src/experimental/geometry/shapes/polygon.dart b/packages/flame/lib/src/experimental/geometry/shapes/polygon.dart index 809788b9608..66b7070a485 100644 --- a/packages/flame/lib/src/experimental/geometry/shapes/polygon.dart +++ b/packages/flame/lib/src/experimental/geometry/shapes/polygon.dart @@ -65,7 +65,7 @@ class Polygon extends Shape { var nInteriorAngles = 0; var nExteriorAngles = 0; var previousEdge = _edges.last; - _edges.forEach((edge) { + for (final edge in _edges) { final crossProduct = edge.cross(previousEdge); previousEdge = edge; // A straight angle counts as both internal and external @@ -75,7 +75,7 @@ class Polygon extends Shape { if (crossProduct <= 0) { nExteriorAngles++; } - }); + } if (nInteriorAngles < nExteriorAngles) { _reverseVertices(); _initializeEdges(); @@ -116,7 +116,9 @@ class Polygon extends Shape { Aabb2? _aabb; Aabb2 _calculateAabb() { final aabb = Aabb2.minMax(_vertices.first, _vertices.first); - _vertices.forEach(aabb.hullPoint); + for (final vertex in _vertices) { + aabb.hullPoint(vertex); + } return aabb; } diff --git a/packages/flame/lib/src/game/flame_game.dart b/packages/flame/lib/src/game/flame_game.dart index 59cde184c2c..92cbfed51c7 100644 --- a/packages/flame/lib/src/game/flame_game.dart +++ b/packages/flame/lib/src/game/flame_game.dart @@ -186,7 +186,9 @@ class FlameGame extends ComponentTreeRoot // there is no way to explicitly call the [Component]'s implementation, // we propagate the event to [FlameGame]'s children manually. handleResize(size); - children.forEach((child) => child.onParentResize(size)); + for (final child in children) { + child.onParentResize(size); + } } /// Ensure that all pending tree operations finish. diff --git a/packages/flame/lib/src/geometry/line_segment.dart b/packages/flame/lib/src/geometry/line_segment.dart index d3492198e34..64e2c55dba8 100644 --- a/packages/flame/lib/src/geometry/line_segment.dart +++ b/packages/flame/lib/src/geometry/line_segment.dart @@ -48,7 +48,9 @@ class LineSegment { }; if (overlaps.isNotEmpty) { final sum = Vector2.zero(); - overlaps.forEach(sum.add); + for (final overlap in overlaps) { + sum.add(overlap); + } return [sum..scale(1 / overlaps.length)]; } } diff --git a/packages/flame/lib/src/layers/layer.dart b/packages/flame/lib/src/layers/layer.dart index 1f7dd5c724d..310eb8efde3 100644 --- a/packages/flame/lib/src/layers/layer.dart +++ b/packages/flame/lib/src/layers/layer.dart @@ -14,16 +14,21 @@ abstract class Layer { @mustCallSuper void render(Canvas canvas, {double x = 0.0, double y = 0.0}) { - if (_picture == null) { + final picture = _picture; + if (picture == null) { return; } canvas.save(); canvas.translate(x, y); - preProcessors.forEach((p) => p.process(_picture!, canvas)); - canvas.drawPicture(_picture!); - postProcessors.forEach((p) => p.process(_picture!, canvas)); + for (final p in preProcessors) { + p.process(picture, canvas); + } + canvas.drawPicture(picture); + for (final p in postProcessors) { + p.process(picture, canvas); + } canvas.restore(); } diff --git a/packages/flame/lib/src/parallax.dart b/packages/flame/lib/src/parallax.dart index c6ea4b99029..9d3234ddb61 100644 --- a/packages/flame/lib/src/parallax.dart +++ b/packages/flame/lib/src/parallax.dart @@ -468,7 +468,7 @@ class Parallax { final _delta = Vector2.zero(); void update(double dt) { - layers.forEach((layer) { + for (final layer in layers) { layer.update( _delta ..setFrom(baseVelocity) @@ -476,7 +476,7 @@ class Parallax { ..scale(dt), dt, ); - }); + } } /// Note that this method only should be used if all of your layers should diff --git a/packages/flame/lib/src/text/elements/group_element.dart b/packages/flame/lib/src/text/elements/group_element.dart index da8624dc907..4df7824e567 100644 --- a/packages/flame/lib/src/text/elements/group_element.dart +++ b/packages/flame/lib/src/text/elements/group_element.dart @@ -12,12 +12,16 @@ class GroupElement extends BlockElement { @override void translate(double dx, double dy) { - children.forEach((child) => child.translate(dx, dy)); + for (final child in children) { + child.translate(dx, dy); + } } @override void draw(Canvas canvas) { - children.forEach((child) => child.draw(canvas)); + for (final child in children) { + child.draw(canvas); + } } @override