Revert "Improve performance of collectAllElements (#68065)" (#68207)

This reverts commit 8ba5732c.

dev/benchmarks/microbenchmarks/lib/foundation/all_elements_bench.dart

-// Copyright 2014 The Flutter Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-import 'package:flutter/material.dart';
-import 'package:flutter/scheduler.dart';
-// ignore: implementation_imports
-import 'package:flutter_test/src/all_elements.dart';
-
-import '../common.dart';
-
-const int _kNumIters = 10000;
-
-Future<void> main() async {
-  assert(false, "Don't run benchmarks in debug mode! Use 'flutter run --release'.");
-  runApp(MaterialApp(
-    home: Scaffold(
-      body: GridView.count(
-        crossAxisCount: 5,
-        children: List<Widget>.generate(100, (int index) {
-          return Center(
-            child: Scaffold(
-              appBar: AppBar(
-                title: Text('App $index'),
-                actions: const <Widget>[
-                  Icon(Icons.help),
-                  Icon(Icons.add),
-                  Icon(Icons.ac_unit),
-                ],
-              ),
-              body: Column(
-                children: const <Widget>[
-                  Text('Item 1'),
-                  Text('Item 2'),
-                  Text('Item 3'),
-                  Text('Item 4'),
-                ],
-              ),
-            ),
-          );
-        }),
-      ),
-    ),
-  ));
-
-  await SchedulerBinding.instance.endOfFrame;
-
-  final Stopwatch watch = Stopwatch();
-
-  print('flutter_test allElements benchmark... (${WidgetsBinding.instance.renderViewElement})');
-  // Make sure we get enough elements to process for consistent benchmark runs
-  int elementCount = collectAllElementsFrom(WidgetsBinding.instance.renderViewElement, skipOffstage: false).length;
-  while (elementCount < 6242) {
-    await Future<void>.delayed(Duration.zero);
-    elementCount = collectAllElementsFrom(WidgetsBinding.instance.renderViewElement, skipOffstage: false).length;
-  }
-  print('element count: $elementCount');
-
-  watch.start();
-  for (int i = 0; i < _kNumIters; i += 1) {
-    final List<Element> allElements = collectAllElementsFrom(
-      WidgetsBinding.instance.renderViewElement,
-      skipOffstage: false,
-    ).toList();
-    allElements.clear();
-  }
-  watch.stop();
-
-  final BenchmarkResultPrinter printer = BenchmarkResultPrinter();
-  printer.addResult(
-    description: 'All elements iterate',
-    value: watch.elapsedMicroseconds / _kNumIters,
-    unit: 'µs per iteration',
-    name: 'all_elements_iteration',
-  );
-  printer.printToStdout();
-}

dev/devicelab/lib/tasks/microbenchmarks.dart

@@ -55,7 +55,6 @@ TaskFunction createMicrobenchmarkTask() {
       ...await _runMicrobench('lib/stocks/animation_bench.dart'),
       ...await _runMicrobench('lib/language/sync_star_bench.dart'),
       ...await _runMicrobench('lib/language/sync_star_semantics_bench.dart'),
-      ...await _runMicrobench('lib/foundation/all_elements_bench.dart'),
       ...await _runMicrobench('lib/foundation/change_notifier_bench.dart'),
  };
 

packages/flutter_test/lib/src/all_elements.dart

@@ -23,39 +23,15 @@ Iterable<Element> collectAllElementsFrom(
   return CachingIterable<Element>(_DepthFirstChildIterator(rootElement, skipOffstage));
 }
 
-/// Provides a recursive, efficient, depth first search of an element tree.
-///
-/// [Element.visitChildren] does not guarnatee order, but does guarnatee stable
-/// order. This iterator also guarantees stable order, and iterates in a left
-/// to right order:
-///
-///       1
-///     /   \
-///    2     3
-///   / \   / \
-///  4   5 6   7
-///
-/// Will iterate in order 2, 4, 5, 3, 6, 7.
-///
-/// Performance is important here because this method is on the critical path
-/// for flutter_driver and package:integration_test performance tests.
-/// Performance is measured in the all_elements_bench microbenchmark.
-/// Any changes to this implementation should check the before and after numbers
-/// on that benchmark to avoid regressions in general performance test overhead.
-///
-/// If we could use RTL order, we could save on performance, but numerous tests
-/// have been written (and developers clearly expect) that LTR order will be
-/// respected.
 class _DepthFirstChildIterator implements Iterator<Element> {
-  _DepthFirstChildIterator(Element rootElement, this.skipOffstage) {
-    _fillChildren(rootElement);
-  }
+  _DepthFirstChildIterator(Element rootElement, this.skipOffstage)
+    : _stack = _reverseChildrenOf(rootElement, skipOffstage).toList();
 
   final bool skipOffstage;
 
   late Element _current;
 
-  final List<Element> _stack = <Element>[];
+  final List<Element> _stack;
 
   @override
   Element get current => _current;
@@ -66,26 +42,20 @@ class _DepthFirstChildIterator implements Iterator<Element> {
       return false;
 
     _current = _stack.removeLast();
-    _fillChildren(_current);
+    // Stack children in reverse order to traverse first branch first
+    _stack.addAll(_reverseChildrenOf(_current, skipOffstage));
 
     return true;
   }
 
-  void _fillChildren(Element element) {
+  static Iterable<Element> _reverseChildrenOf(Element element, bool skipOffstage) {
     assert(element != null);
-    // If we did not have to follow LTR order and could instead use RTL,
-    // we could avoid reversing this and the operation would be measurably
-    // faster. Unfortunately, a lot of tests depend on LTR order.
-    final List<Element> reversed = <Element>[];
+    final List<Element> children = <Element>[];
     if (skipOffstage) {
-      element.debugVisitOnstageChildren(reversed.add);
+      element.debugVisitOnstageChildren(children.add);
     } else {
-      element.visitChildren(reversed.add);
-    }
-    // This is faster than _stack.addAll(reversed.reversed), presumably since
-    // we don't actually care about maintaining an iteration pointer.
-    while (reversed.isNotEmpty) {
-      _stack.add(reversed.removeLast());
+      element.visitChildren(children.add);
     }
+    return children.reversed;
   }
 }

packages/flutter_test/test/all_elements_test.dart

-// Copyright 2014 The Flutter Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-import 'package:flutter/material.dart';
-import 'package:flutter_test/flutter_test.dart';
-
-void main() {
-  testWidgets('collectAllElements goes in LTR DFS', (WidgetTester tester) async {
-    final GlobalKey key = GlobalKey();
-    await tester.pumpWidget(Directionality(
-      key: key,
-      textDirection: TextDirection.ltr,
-      child: Row(
-        children: <Widget>[
-          RichText(text: const TextSpan(text: 'a')),
-          RichText(text: const TextSpan(text: 'b')),
-        ],
-      ),
-    ));
-
-    final List<Element> elements = collectAllElementsFrom(
-      key.currentContext! as Element,
-      skipOffstage: false,
-    ).toList();
-
-    expect(elements.length, 3);
-    expect(elements[0].widget, isA<Row>());
-    expect(elements[1].widget, isA<RichText>());
-    expect(((elements[1].widget as RichText).text as TextSpan).text, 'a');
-    expect(elements[2].widget, isA<RichText>());
-    expect(((elements[2].widget as RichText).text as TextSpan).text, 'b');
-  });
-}