[framework] dont allocate forgotten children set in profile/release mode (#94911)
diff --git a/dev/bots/analyze.dart b/dev/bots/analyze.dart
index 5e77fe7..68dae54 100644
--- a/dev/bots/analyze.dart
+++ b/dev/bots/analyze.dart
@@ -98,6 +98,9 @@
print('$clock Integration test timeouts...');
await verifyIntegrationTestTimeouts(flutterRoot);
+ print('$clock null initialized debug fields...');
+ await verifyNullInitializedDebugExpensiveFields(flutterRoot);
+
// Ensure that all package dependencies are in sync.
print('$clock Package dependencies...');
await runCommand(flutter, <String>['update-packages', '--verify-only'],
@@ -785,7 +788,7 @@
final Set<String> suggestions = <String>{};
final List<File> files = await _gitFiles(workingDirectory);
for (final File file in files) {
- if (path.basename(file.path).endsWith('_test.dart'))
+ if (path.basename(file.path).endsWith('_test.dart') || path.basename(file.path) == 'analyze.dart')
continue; // Skip tests, they're not public-facing.
final Uint8List bytes = file.readAsBytesSync();
// We allow invalid UTF-8 here so that binaries don't trip us up.
@@ -1492,6 +1495,40 @@
}
}
+const String _kDebugOnlyAnnotation = '@_debugOnly';
+final RegExp _nullInitializedField = RegExp(r'kDebugMode \? [\w\<\> ,{}()]+ : null;');
+
+Future<void> verifyNullInitializedDebugExpensiveFields(String workingDirectory, {int minimumMatches = 400}) async {
+ final String flutterLib = path.join(workingDirectory, 'packages', 'flutter', 'lib');
+ final List<File> files = await _allFiles(flutterLib, 'dart', minimumMatches: minimumMatches)
+ .toList();
+ final List<String> errors = <String>[];
+ for (final File file in files) {
+ final List<String> lines = file.readAsLinesSync();
+ for (int i = 0; i < lines.length; i += 1) {
+ final String line = lines[i];
+ if (!line.contains(_kDebugOnlyAnnotation)) {
+ continue;
+ }
+ final String nextLine = lines[i + 1];
+ if (_nullInitializedField.firstMatch(nextLine) == null) {
+ errors.add('${file.path} L$i');
+ }
+ }
+ }
+
+ if (errors.isNotEmpty) {
+ exitWithError(<String>[
+ '${bold}ERROR: ${red}fields annotated with @_debugOnly must null initialize.$reset',
+ 'to ensure both the field and initializer are removed from profile/release mode.',
+ 'These fields should be written as:\n',
+ 'field = kDebugMode ? <DebugValue> : null;\n',
+ 'Errors were found in the following files:',
+ ...errors,
+ ]);
+ }
+}
+
Future<CommandResult> _runFlutterAnalyze(String workingDirectory, {
List<String> options = const <String>[],
}) async {
diff --git a/dev/bots/test/analyze-test-input/root/packages/flutter/lib/bar.dart b/dev/bots/test/analyze-test-input/root/packages/flutter/lib/bar.dart
new file mode 100644
index 0000000..52fe689
--- /dev/null
+++ b/dev/bots/test/analyze-test-input/root/packages/flutter/lib/bar.dart
@@ -0,0 +1,18 @@
+// 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.
+
+class _DebugOnly {
+ const _DebugOnly();
+}
+
+const _DebugOnly _debugOnly = _DebugOnly();
+const bool kDebugMode = bool.fromEnvironment('test-only');
+
+class Foo {
+ @_debugOnly
+ final Map<String, String>? foo = kDebugMode ? <String, String>{} : null;
+
+ @_debugOnly
+ final Map<String, String>? bar = kDebugMode ? null : <String, String>{};
+}
diff --git a/dev/bots/test/analyze_test.dart b/dev/bots/test/analyze_test.dart
index 5ba5ff4..d554895 100644
--- a/dev/bots/test/analyze_test.dart
+++ b/dev/bots/test/analyze_test.dart
@@ -193,4 +193,14 @@
},
));
});
+
+ test('analyze.dart - verifyNullInitializedDebugExpensiveFields', () async {
+ final String result = await capture(() => verifyNullInitializedDebugExpensiveFields(
+ testRootPath,
+ minimumMatches: 1,
+ ), exitCode: 1);
+
+ expect(result, contains('L15'));
+ expect(result, isNot(contains('L12')));
+ });
}
diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart
index 6bb84f2..1ad1e6d 100644
--- a/packages/flutter/lib/src/widgets/framework.dart
+++ b/packages/flutter/lib/src/widgets/framework.dart
@@ -37,6 +37,22 @@
// abstract class FrogJar extends RenderObjectWidget { const FrogJar({Key? key}) : super(key: key); }
// abstract class FrogJarParentData extends ParentData { late Size size; }
+// An annotation used by test_analysis package to verify patterns are followed
+// that allow for tree-shaking of both fields and their initializers. This
+// annotation has no impact on code by itself, but indicates the following pattern
+// should be followed for a given field:
+//
+// ```dart
+// class Foo {
+// final bar = kDebugMode ? Object() : null;
+// }
+// ```
+class _DebugOnly {
+ const _DebugOnly();
+}
+
+const _DebugOnly _debugOnly = _DebugOnly();
+
// KEYS
/// A key that is only equal to itself.
@@ -2705,12 +2721,21 @@
}
final Map<GlobalKey, Element> _globalKeyRegistry = <GlobalKey, Element>{};
- final Set<Element> _debugIllFatedElements = HashSet<Element>();
+
+ // In Profile/Release mode this field is initialized to `null`. The Dart compiler can
+ // eliminate unused fields, but not their initializers.
+ @_debugOnly
+ final Set<Element>? _debugIllFatedElements = kDebugMode ? HashSet<Element>() : null;
+
// This map keeps track which child reserves the global key with the parent.
// Parent, child -> global key.
// This provides us a way to remove old reservation while parent rebuilds the
// child in the same slot.
- final Map<Element, Map<Element, GlobalKey>> _debugGlobalKeyReservations = <Element, Map<Element, GlobalKey>>{};
+ //
+ // In Profile/Release mode this field is initialized to `null`. The Dart compiler can
+ // eliminate unused fields, but not their initializers.
+ @_debugOnly
+ final Map<Element, Map<Element, GlobalKey>>? _debugGlobalKeyReservations = kDebugMode ? <Element, Map<Element, GlobalKey>>{} : null;
/// The number of [GlobalKey] instances that are currently associated with
/// [Element]s that have been built by this build owner.
@@ -2720,7 +2745,7 @@
assert(() {
assert(parent != null);
assert(child != null);
- _debugGlobalKeyReservations[parent]?.remove(child);
+ _debugGlobalKeyReservations?[parent]?.remove(child);
return true;
}());
}
@@ -2732,7 +2757,7 @@
final Element oldElement = _globalKeyRegistry[key]!;
assert(oldElement.widget != null);
assert(element.widget.runtimeType != oldElement.widget.runtimeType);
- _debugIllFatedElements.add(oldElement);
+ _debugIllFatedElements?.add(oldElement);
}
return true;
}());
@@ -2757,8 +2782,8 @@
assert(() {
assert(parent != null);
assert(child != null);
- _debugGlobalKeyReservations[parent] ??= <Element, GlobalKey>{};
- _debugGlobalKeyReservations[parent]![child] = key;
+ _debugGlobalKeyReservations?[parent] ??= <Element, GlobalKey>{};
+ _debugGlobalKeyReservations?[parent]![child] = key;
return true;
}());
}
@@ -2766,7 +2791,7 @@
void _debugVerifyGlobalKeyReservation() {
assert(() {
final Map<GlobalKey, Element> keyToParent = <GlobalKey, Element>{};
- _debugGlobalKeyReservations.forEach((Element parent, Map<Element, GlobalKey> childToKey) {
+ _debugGlobalKeyReservations?.forEach((Element parent, Map<Element, GlobalKey> childToKey) {
// We ignore parent that are unmounted or detached.
if (parent._lifecycleState == _ElementLifecycle.defunct || parent.renderObject?.attached == false)
return;
@@ -2829,7 +2854,7 @@
}
});
});
- _debugGlobalKeyReservations.clear();
+ _debugGlobalKeyReservations?.clear();
return true;
}());
}
@@ -2837,7 +2862,7 @@
void _debugVerifyIllFatedPopulation() {
assert(() {
Map<GlobalKey, Set<Element>>? duplicates;
- for (final Element element in _debugIllFatedElements) {
+ for (final Element element in _debugIllFatedElements!) {
if (element._lifecycleState != _ElementLifecycle.defunct) {
assert(element != null);
assert(element.widget != null);
@@ -2851,7 +2876,7 @@
elements.add(_globalKeyRegistry[key]!);
}
}
- _debugIllFatedElements.clear();
+ _debugIllFatedElements?.clear();
if (duplicates != null) {
final List<DiagnosticsNode> information = <DiagnosticsNode>[];
information.add(ErrorSummary('Multiple widgets used the same GlobalKey.'));
@@ -2887,9 +2912,7 @@
Timeline.startSync('FINALIZE TREE', arguments: timelineArgumentsIndicatingLandmarkEvent);
}
try {
- lockState(() {
- _inactiveElements._unmountAll(); // this unregisters the GlobalKeys
- });
+ lockState(_inactiveElements._unmountAll); // this unregisters the GlobalKeys
assert(() {
try {
_debugVerifyGlobalKeyReservation();
@@ -3588,8 +3611,8 @@
// never updates (the forgotten children are not removed from the tree
// until the call to update happens)
assert(() {
- _debugForgottenChildrenWithGlobalKey.forEach(_debugRemoveGlobalKeyReservation);
- _debugForgottenChildrenWithGlobalKey.clear();
+ _debugForgottenChildrenWithGlobalKey?.forEach(_debugRemoveGlobalKeyReservation);
+ _debugForgottenChildrenWithGlobalKey?.clear();
return true;
}());
_widget = newWidget;
@@ -3795,7 +3818,11 @@
// The children that have been forgotten by forgetChild. This will be used in
// [update] to remove the global key reservations of forgotten children.
- final Set<Element> _debugForgottenChildrenWithGlobalKey = HashSet<Element>();
+ //
+ // In Profile/Release mode this field is initialized to `null`. The Dart compiler can
+ // eliminate unused fields, but not their initializers.
+ @_debugOnly
+ final Set<Element>? _debugForgottenChildrenWithGlobalKey = kDebugMode ? HashSet<Element>() : null;
/// Remove the given child from the element's child list, in preparation for
/// the child being reused elsewhere in the element tree.
@@ -3821,7 +3848,7 @@
// key duplication that we need to catch.
assert(() {
if (child.widget.key is GlobalKey)
- _debugForgottenChildrenWithGlobalKey.add(child);
+ _debugForgottenChildrenWithGlobalKey?.add(child);
return true;
}());
}
