Unverified Commit 35b18ba2 authored by gaaclarke's avatar gaaclarke Committed by GitHub

Made flag for debugging build time of user created widgets (#100926)

* Added a bool that allows us to limit debugProfileBuildsEnabled to user
created widgets.

* made it turned on by default

* switched to hashmap

* Cleaned everything up and added tests

* fixed an odd test where it wants to be able to add asserts and run in profile mode

* hixie feedback

* hixie2

* made it default to false

* updated docstring as per dans request
parent 839a183e
...@@ -110,6 +110,17 @@ void main() { ...@@ -110,6 +110,17 @@ void main() {
expect(args['color'], 'Color(0xffffffff)'); expect(args['color'], 'Color(0xffffffff)');
debugProfileBuildsEnabled = false; debugProfileBuildsEnabled = false;
debugProfileBuildsEnabledUserWidgets = true;
await runFrame(() { TestRoot.state.updateWidget(Placeholder(key: UniqueKey(), color: const Color(0xFFFFFFFF))); });
events = await fetchInterestingEvents(interestingLabels);
expect(
events.map<String>(eventToName),
<String>['BUILD', 'Placeholder', 'LAYOUT', 'UPDATING COMPOSITING BITS', 'PAINT', 'COMPOSITING', 'FINALIZE TREE'],
);
args = (events.where((TimelineEvent event) => event.json!['name'] == '$Placeholder').single.json!['args'] as Map<String, Object?>).cast<String, String>();
expect(args['color'], 'Color(0xffffffff)');
debugProfileBuildsEnabledUserWidgets = false;
debugProfileLayoutsEnabled = true; debugProfileLayoutsEnabled = true;
await runFrame(() { TestRoot.state.updateWidget(Placeholder(key: UniqueKey())); }); await runFrame(() { TestRoot.state.updateWidget(Placeholder(key: UniqueKey())); });
events = await fetchInterestingEvents(interestingLabels); events = await fetchInterestingEvents(interestingLabels);
......
...@@ -114,8 +114,24 @@ bool debugPrintGlobalKeyedWidgetLifecycle = false; ...@@ -114,8 +114,24 @@ bool debugPrintGlobalKeyedWidgetLifecycle = false;
/// * [debugProfileLayoutsEnabled], which does something similar for layout, /// * [debugProfileLayoutsEnabled], which does something similar for layout,
/// and [debugPrintLayouts], its console equivalent. /// and [debugPrintLayouts], its console equivalent.
/// * [debugProfilePaintsEnabled], which does something similar for painting. /// * [debugProfilePaintsEnabled], which does something similar for painting.
/// * [debugProfileBuildsEnabledUserWidgets], which adds events for user-created
/// [Widget] build times and incurs less overhead.
bool debugProfileBuildsEnabled = false; bool debugProfileBuildsEnabled = false;
/// Adds [Timeline] events for every user-created [Widget] built.
///
/// A user-created [Widget] is any [Widget] that is constructed in the root
/// library. Often [Widget]s contain child [Widget]s that are constructed in
/// libraries (for example, a [TextButton] having a [RichText] child). Timeline
/// events for those children will be omitted with this flag. This works for any
/// [Widget] not just ones declared in the root library.
///
/// See also:
///
/// * [debugProfileBuildsEnabled], which functions similarly but shows events
/// for every widget and has a higher overhead cost.
bool debugProfileBuildsEnabledUserWidgets = false;
/// Show banners for deprecated widgets. /// Show banners for deprecated widgets.
bool debugHighlightDeprecatedWidgets = false; bool debugHighlightDeprecatedWidgets = false;
...@@ -423,7 +439,8 @@ bool debugAssertAllWidgetVarsUnset(String reason) { ...@@ -423,7 +439,8 @@ bool debugAssertAllWidgetVarsUnset(String reason) {
debugPrintScheduleBuildForStacks || debugPrintScheduleBuildForStacks ||
debugPrintGlobalKeyedWidgetLifecycle || debugPrintGlobalKeyedWidgetLifecycle ||
debugProfileBuildsEnabled || debugProfileBuildsEnabled ||
debugHighlightDeprecatedWidgets) { debugHighlightDeprecatedWidgets ||
debugProfileBuildsEnabledUserWidgets) {
throw FlutterError(reason); throw FlutterError(reason);
} }
return true; return true;
......
...@@ -14,6 +14,7 @@ import 'debug.dart'; ...@@ -14,6 +14,7 @@ import 'debug.dart';
import 'focus_manager.dart'; import 'focus_manager.dart';
import 'inherited_model.dart'; import 'inherited_model.dart';
import 'notification_listener.dart'; import 'notification_listener.dart';
import 'widget_inspector.dart';
export 'package:flutter/foundation.dart' show export 'package:flutter/foundation.dart' show
factory, factory,
...@@ -2640,10 +2641,13 @@ class BuildOwner { ...@@ -2640,10 +2641,13 @@ class BuildOwner {
} }
return true; return true;
}()); }());
if (!kReleaseMode && debugProfileBuildsEnabled) { final bool isTimelineTracked = !kReleaseMode && _isProfileBuildsEnabledFor(element.widget);
if (isTimelineTracked) {
Map<String, String> debugTimelineArguments = timelineArgumentsIndicatingLandmarkEvent; Map<String, String> debugTimelineArguments = timelineArgumentsIndicatingLandmarkEvent;
assert(() { assert(() {
debugTimelineArguments = element.widget.toDiagnosticsNode().toTimelineArguments(); if (kDebugMode) {
debugTimelineArguments = element.widget.toDiagnosticsNode().toTimelineArguments();
}
return true; return true;
}()); }());
Timeline.startSync( Timeline.startSync(
...@@ -2668,7 +2672,7 @@ class BuildOwner { ...@@ -2668,7 +2672,7 @@ class BuildOwner {
], ],
); );
} }
if (!kReleaseMode && debugProfileBuildsEnabled) if (isTimelineTracked)
Timeline.finishSync(); Timeline.finishSync();
index += 1; index += 1;
if (dirtyCount < _dirtyElements.length || _dirtyElementsNeedsResorting!) { if (dirtyCount < _dirtyElements.length || _dirtyElementsNeedsResorting!) {
...@@ -3078,6 +3082,12 @@ class _NotificationNode { ...@@ -3078,6 +3082,12 @@ class _NotificationNode {
} }
} }
bool _isProfileBuildsEnabledFor(Widget widget) {
return debugProfileBuildsEnabled ||
(debugProfileBuildsEnabledUserWidgets &&
debugIsWidgetLocalCreation(widget));
}
/// An instantiation of a [Widget] at a particular location in the tree. /// An instantiation of a [Widget] at a particular location in the tree.
/// ///
/// Widgets describe how to configure a subtree but the same widget can be used /// Widgets describe how to configure a subtree but the same widget can be used
...@@ -3503,10 +3513,13 @@ abstract class Element extends DiagnosticableTree implements BuildContext { ...@@ -3503,10 +3513,13 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
} else if (hasSameSuperclass && Widget.canUpdate(child.widget, newWidget)) { } else if (hasSameSuperclass && Widget.canUpdate(child.widget, newWidget)) {
if (child.slot != newSlot) if (child.slot != newSlot)
updateSlotForChild(child, newSlot); updateSlotForChild(child, newSlot);
if (!kReleaseMode && debugProfileBuildsEnabled) { final bool isTimelineTracked = !kReleaseMode && _isProfileBuildsEnabledFor(newWidget);
if (isTimelineTracked) {
Map<String, String> debugTimelineArguments = timelineArgumentsIndicatingLandmarkEvent; Map<String, String> debugTimelineArguments = timelineArgumentsIndicatingLandmarkEvent;
assert(() { assert(() {
debugTimelineArguments = newWidget.toDiagnosticsNode().toTimelineArguments(); if (kDebugMode) {
debugTimelineArguments = newWidget.toDiagnosticsNode().toTimelineArguments();
}
return true; return true;
}()); }());
Timeline.startSync( Timeline.startSync(
...@@ -3515,7 +3528,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext { ...@@ -3515,7 +3528,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
); );
} }
child.update(newWidget); child.update(newWidget);
if (!kReleaseMode && debugProfileBuildsEnabled) if (isTimelineTracked)
Timeline.finishSync(); Timeline.finishSync();
assert(child.widget == newWidget); assert(child.widget == newWidget);
assert(() { assert(() {
...@@ -3765,10 +3778,13 @@ abstract class Element extends DiagnosticableTree implements BuildContext { ...@@ -3765,10 +3778,13 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
Element inflateWidget(Widget newWidget, Object? newSlot) { Element inflateWidget(Widget newWidget, Object? newSlot) {
assert(newWidget != null); assert(newWidget != null);
if (!kReleaseMode && debugProfileBuildsEnabled) { final bool isTimelineTracked = !kReleaseMode && _isProfileBuildsEnabledFor(newWidget);
if (isTimelineTracked) {
Map<String, String> debugTimelineArguments = timelineArgumentsIndicatingLandmarkEvent; Map<String, String> debugTimelineArguments = timelineArgumentsIndicatingLandmarkEvent;
assert(() { assert(() {
debugTimelineArguments = newWidget.toDiagnosticsNode().toTimelineArguments(); if (kDebugMode) {
debugTimelineArguments = newWidget.toDiagnosticsNode().toTimelineArguments();
}
return true; return true;
}()); }());
Timeline.startSync( Timeline.startSync(
...@@ -3800,7 +3816,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext { ...@@ -3800,7 +3816,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
newChild.mount(this, newSlot); newChild.mount(this, newSlot);
assert(newChild._lifecycleState == _ElementLifecycle.active); assert(newChild._lifecycleState == _ElementLifecycle.active);
if (!kReleaseMode && debugProfileBuildsEnabled) if (isTimelineTracked)
Timeline.finishSync(); Timeline.finishSync();
return newChild; return newChild;
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:async'; import 'dart:async';
import 'dart:collection' show HashMap;
import 'dart:convert'; import 'dart:convert';
import 'dart:developer' as developer; import 'dart:developer' as developer;
import 'dart:math' as math; import 'dart:math' as math;
...@@ -742,6 +743,8 @@ mixin WidgetInspectorService { ...@@ -742,6 +743,8 @@ mixin WidgetInspectorService {
int _nextId = 0; int _nextId = 0;
List<String>? _pubRootDirectories; List<String>? _pubRootDirectories;
/// Memoization for [_isLocalCreationLocation].
final HashMap<String, bool> _isLocalCreationCache = HashMap<String, bool>();
bool _trackRebuildDirtyWidgets = false; bool _trackRebuildDirtyWidgets = false;
bool _trackRepaintWidgets = false; bool _trackRepaintWidgets = false;
...@@ -1345,6 +1348,7 @@ mixin WidgetInspectorService { ...@@ -1345,6 +1348,7 @@ mixin WidgetInspectorService {
_pubRootDirectories = pubRootDirectories _pubRootDirectories = pubRootDirectories
.map<String>((String directory) => Uri.parse(directory).path) .map<String>((String directory) => Uri.parse(directory).path)
.toList(); .toList();
_isLocalCreationCache.clear();
} }
/// Set the [WidgetInspector] selection to the object matching the specified /// Set the [WidgetInspector] selection to the object matching the specified
...@@ -1518,14 +1522,11 @@ mixin WidgetInspectorService { ...@@ -1518,14 +1522,11 @@ mixin WidgetInspectorService {
if (creationLocation == null) { if (creationLocation == null) {
return false; return false;
} }
return _isLocalCreationLocation(creationLocation); return _isLocalCreationLocation(creationLocation.file);
} }
bool _isLocalCreationLocation(_Location? location) { bool _isLocalCreationLocationImpl(String locationUri) {
if (location == null) { final String file = Uri.parse(locationUri).path;
return false;
}
final String file = Uri.parse(location.file).path;
// By default check whether the creation location was within package:flutter. // By default check whether the creation location was within package:flutter.
if (_pubRootDirectories == null) { if (_pubRootDirectories == null) {
...@@ -1541,6 +1542,17 @@ mixin WidgetInspectorService { ...@@ -1541,6 +1542,17 @@ mixin WidgetInspectorService {
return false; return false;
} }
/// Memoized version of [_isLocalCreationLocationImpl].
bool _isLocalCreationLocation(String locationUri) {
final bool? cachedValue = _isLocalCreationCache[locationUri];
if (cachedValue != null) {
return cachedValue;
}
final bool result = _isLocalCreationLocationImpl(locationUri);
_isLocalCreationCache[locationUri] = result;
return result;
}
/// Wrapper around `json.encode` that uses a ring of cached values to prevent /// Wrapper around `json.encode` that uses a ring of cached values to prevent
/// the Dart garbage collector from collecting objects between when /// the Dart garbage collector from collecting objects between when
/// the value is returned over the Observatory protocol and when the /// the value is returned over the Observatory protocol and when the
...@@ -2066,7 +2078,7 @@ class _ElementLocationStatsTracker { ...@@ -2066,7 +2078,7 @@ class _ElementLocationStatsTracker {
entry = _LocationCount( entry = _LocationCount(
location: location, location: location,
id: id, id: id,
local: WidgetInspectorService.instance._isLocalCreationLocation(location), local: WidgetInspectorService.instance._isLocalCreationLocation(location.file),
); );
if (entry.local) { if (entry.local) {
newLocations.add(entry); newLocations.add(entry);
...@@ -3072,14 +3084,24 @@ bool debugIsLocalCreationLocation(Object object) { ...@@ -3072,14 +3084,24 @@ bool debugIsLocalCreationLocation(Object object) {
bool isLocal = false; bool isLocal = false;
assert(() { assert(() {
final _Location? location = _getCreationLocation(object); final _Location? location = _getCreationLocation(object);
if (location == null) if (location != null) {
isLocal = false; isLocal = WidgetInspectorService.instance._isLocalCreationLocation(location.file);
isLocal = WidgetInspectorService.instance._isLocalCreationLocation(location); }
return true; return true;
}()); }());
return isLocal; return isLocal;
} }
/// Returns true if a [Widget] is user created.
///
/// This is a faster variant of `debugIsLocalCreationLocation` that is available
/// in debug and profile builds but only works for [Widget].
bool debugIsWidgetLocalCreation(Widget widget) {
final _Location? location = _getObjectCreationLocation(widget);
return location != null &&
WidgetInspectorService.instance._isLocalCreationLocation(location.file);
}
/// Returns the creation location of an object in String format if one is available. /// Returns the creation location of an object in String format if one is available.
/// ///
/// ex: "file:///path/to/main.dart:4:3" /// ex: "file:///path/to/main.dart:4:3"
...@@ -3092,14 +3114,18 @@ String? _describeCreationLocation(Object object) { ...@@ -3092,14 +3114,18 @@ String? _describeCreationLocation(Object object) {
return location?.toString(); return location?.toString();
} }
_Location? _getObjectCreationLocation(Object object) {
return object is _HasCreationLocation ? object._location : null;
}
/// Returns the creation location of an object if one is available. /// Returns the creation location of an object if one is available.
/// ///
/// {@macro flutter.widgets.WidgetInspectorService.getChildrenSummaryTree} /// {@macro flutter.widgets.WidgetInspectorService.getChildrenSummaryTree}
/// ///
/// Currently creation locations are only available for [Widget] and [Element]. /// Currently creation locations are only available for [Widget] and [Element].
_Location? _getCreationLocation(Object? object) { _Location? _getCreationLocation(Object? object) {
final Object? candidate = object is Element && !object.debugIsDefunct ? object.widget : object; final Object? candidate = object is Element && !object.debugIsDefunct ? object.widget : object;
return candidate is _HasCreationLocation ? candidate._location : null; return candidate == null ? null : _getObjectCreationLocation(candidate);
} }
// _Location objects are always const so we don't need to worry about the GC // _Location objects are always const so we don't need to worry about the GC
...@@ -3226,7 +3252,7 @@ class InspectorSerializationDelegate implements DiagnosticsSerializationDelegate ...@@ -3226,7 +3252,7 @@ class InspectorSerializationDelegate implements DiagnosticsSerializationDelegate
if (creationLocation != null) { if (creationLocation != null) {
result['locationId'] = _toLocationId(creationLocation); result['locationId'] = _toLocationId(creationLocation);
result['creationLocation'] = creationLocation.toJsonMap(); result['creationLocation'] = creationLocation.toJsonMap();
if (service._isLocalCreationLocation(creationLocation)) { if (service._isLocalCreationLocation(creationLocation.file)) {
_nodesCreatedByLocalProject.add(node); _nodesCreatedByLocalProject.add(node);
result['createdByLocalProject'] = true; result['createdByLocalProject'] = true;
} }
......
...@@ -2998,6 +2998,46 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { ...@@ -2998,6 +2998,46 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService {
expect(debugIsLocalCreationLocation(paddingElement.widget), isFalse); expect(debugIsLocalCreationLocation(paddingElement.widget), isFalse);
}, skip: !WidgetInspectorService.instance.isWidgetCreationTracked()); // [intended] Test requires --track-widget-creation flag. }, skip: !WidgetInspectorService.instance.isWidgetCreationTracked()); // [intended] Test requires --track-widget-creation flag.
testWidgets('debugIsWidgetLocalCreation test', (WidgetTester tester) async {
setupDefaultPubRootDirectory(service);
final GlobalKey key = GlobalKey();
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Container(
padding: const EdgeInsets.all(8),
child: Text('target', key: key, textDirection: TextDirection.ltr),
),
),
);
final Element element = key.currentContext! as Element;
expect(debugIsWidgetLocalCreation(element.widget), isTrue);
final Finder paddingFinder = find.byType(Padding);
final Element paddingElement = paddingFinder.evaluate().first;
expect(debugIsWidgetLocalCreation(paddingElement.widget), isFalse);
}, skip: !WidgetInspectorService.instance.isWidgetCreationTracked()); // [intended] Test requires --track-widget-creation flag.
testWidgets('debugIsWidgetLocalCreation false test', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Container(
padding: const EdgeInsets.all(8),
child: Text('target', key: key, textDirection: TextDirection.ltr),
),
),
);
final Element element = key.currentContext! as Element;
expect(debugIsWidgetLocalCreation(element.widget), isFalse);
}, skip: WidgetInspectorService.instance.isWidgetCreationTracked()); // [intended] Test requires --no-track-widget-creation flag.
test('devToolsInspectorUri test', () { test('devToolsInspectorUri test', () {
activeDevToolsServerAddress = 'http://127.0.0.1:9100'; activeDevToolsServerAddress = 'http://127.0.0.1:9100';
connectedVmServiceUri = 'http://127.0.0.1:55269/798ay5al_FM=/'; connectedVmServiceUri = 'http://127.0.0.1:55269/798ay5al_FM=/';
......
...@@ -222,7 +222,7 @@ class KernelSnapshot extends Target { ...@@ -222,7 +222,7 @@ class KernelSnapshot extends Target {
), ),
aot: buildMode.isPrecompiled, aot: buildMode.isPrecompiled,
buildMode: buildMode, buildMode: buildMode,
trackWidgetCreation: trackWidgetCreation && buildMode == BuildMode.debug, trackWidgetCreation: trackWidgetCreation && buildMode != BuildMode.release,
targetModel: targetModel, targetModel: targetModel,
outputFilePath: environment.buildDir.childFile('app.dill').path, outputFilePath: environment.buildDir.childFile('app.dill').path,
packagesPath: packagesFile.path, packagesPath: packagesFile.path,
......
...@@ -93,6 +93,7 @@ void main() { ...@@ -93,6 +93,7 @@ void main() {
'--target=flutter', '--target=flutter',
'--no-print-incremental-dependencies', '--no-print-incremental-dependencies',
...buildModeOptions(BuildMode.profile, <String>[]), ...buildModeOptions(BuildMode.profile, <String>[]),
'--track-widget-creation',
'--aot', '--aot',
'--tfa', '--tfa',
'--packages', '--packages',
...@@ -109,7 +110,7 @@ void main() { ...@@ -109,7 +110,7 @@ void main() {
expect(processManager, hasNoRemainingExpectations); expect(processManager, hasNoRemainingExpectations);
}); });
testWithoutContext('KernelSnapshot does not use track widget creation on profile builds', () async { testWithoutContext('KernelSnapshot does use track widget creation on profile builds', () async {
fileSystem.file('.dart_tool/package_config.json') fileSystem.file('.dart_tool/package_config.json')
..createSync(recursive: true) ..createSync(recursive: true)
..writeAsStringSync('{"configVersion": 2, "packages":[]}'); ..writeAsStringSync('{"configVersion": 2, "packages":[]}');
...@@ -129,6 +130,7 @@ void main() { ...@@ -129,6 +130,7 @@ void main() {
'--target=flutter', '--target=flutter',
'--no-print-incremental-dependencies', '--no-print-incremental-dependencies',
...buildModeOptions(BuildMode.profile, <String>[]), ...buildModeOptions(BuildMode.profile, <String>[]),
'--track-widget-creation',
'--aot', '--aot',
'--tfa', '--tfa',
'--packages', '--packages',
...@@ -166,6 +168,7 @@ void main() { ...@@ -166,6 +168,7 @@ void main() {
'--target=flutter', '--target=flutter',
'--no-print-incremental-dependencies', '--no-print-incremental-dependencies',
...buildModeOptions(BuildMode.profile, <String>[]), ...buildModeOptions(BuildMode.profile, <String>[]),
'--track-widget-creation',
'--aot', '--aot',
'--tfa', '--tfa',
'--packages', '--packages',
...@@ -204,6 +207,7 @@ void main() { ...@@ -204,6 +207,7 @@ void main() {
'--target=flutter', '--target=flutter',
'--no-print-incremental-dependencies', '--no-print-incremental-dependencies',
...buildModeOptions(BuildMode.profile, <String>[]), ...buildModeOptions(BuildMode.profile, <String>[]),
'--track-widget-creation',
'--aot', '--aot',
'--tfa', '--tfa',
'--packages', '--packages',
......
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