Unverified Commit fda40947 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[framework] dont allocate forgotten children set in profile/release mode (#94911)

parent 02574b7e
......@@ -98,6 +98,9 @@ Future<void> run(List<String> arguments) async {
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 @@ Future<void> verifyIssueLinks(String workingDirectory) async {
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 @@ Future<void> _checkConsumerDependencies() async {
}
}
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 {
......
// 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>{};
}
......@@ -193,4 +193,14 @@ void main() {
},
));
});
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')));
});
}
......@@ -37,6 +37,22 @@ export 'package:flutter/rendering.dart' show RenderObject, RenderBox, debugDumpR
// 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 @@ class BuildOwner {
}
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 @@ class BuildOwner {
assert(() {
assert(parent != null);
assert(child != null);
_debugGlobalKeyReservations[parent]?.remove(child);
_debugGlobalKeyReservations?[parent]?.remove(child);
return true;
}());
}
......@@ -2732,7 +2757,7 @@ class BuildOwner {
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 @@ class BuildOwner {
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 @@ class BuildOwner {
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 @@ class BuildOwner {
}
});
});
_debugGlobalKeyReservations.clear();
_debugGlobalKeyReservations?.clear();
return true;
}());
}
......@@ -2837,7 +2862,7 @@ class BuildOwner {
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 @@ class BuildOwner {
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 @@ class BuildOwner {
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 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
// 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 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
// 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 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
// key duplication that we need to catch.
assert(() {
if (child.widget.key is GlobalKey)
_debugForgottenChildrenWithGlobalKey.add(child);
_debugForgottenChildrenWithGlobalKey?.add(child);
return true;
}());
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment