From 61ac285608cd3c3c003507a572b34fad893a21d4 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 11 Feb 2022 13:30:18 -0800 Subject: [PATCH] [performance] Trace direct calls to inflateWidget (#98277) --- dev/tracing_tests/test/common.dart | 21 +++++ .../test/inflate_widget_tracing_test.dart | 85 +++++++++++++++++++ dev/tracing_tests/test/timeline_test.dart | 29 ++----- .../flutter/lib/src/widgets/framework.dart | 49 +++++------ 4 files changed, 136 insertions(+), 48 deletions(-) create mode 100644 dev/tracing_tests/test/inflate_widget_tracing_test.dart diff --git a/dev/tracing_tests/test/common.dart b/dev/tracing_tests/test/common.dart index 13b684ca4a..498dc18c63 100644 --- a/dev/tracing_tests/test/common.dart +++ b/dev/tracing_tests/test/common.dart @@ -4,7 +4,9 @@ import 'dart:developer' as developer; import 'dart:isolate' as isolate; +import 'dart:ui'; +import 'package:flutter/scheduler.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:vm_service/vm_service.dart'; import 'package:vm_service/vm_service_io.dart'; @@ -32,3 +34,22 @@ Future> fetchTimelineEvents() async { await _vmService.clearVMTimeline(); return timeline.traceEvents!; } + +Future> fetchInterestingEvents(Set interestingLabels) async { + return (await fetchTimelineEvents()).where((TimelineEvent event) { + return interestingLabels.contains(event.json!['name']) + && event.json!['ph'] == 'B'; // "Begin" mark of events, vs E which is for the "End" mark of events. + }).toList(); +} + +String eventToName(TimelineEvent event) => event.json!['name'] as String; + +Future> fetchInterestingEventNames(Set interestingLabels) async { + return (await fetchInterestingEvents(interestingLabels)).map(eventToName).toList(); +} + +Future runFrame(VoidCallback callback) { + final Future result = SchedulerBinding.instance.endOfFrame; // schedules a frame + callback(); + return result; +} diff --git a/dev/tracing_tests/test/inflate_widget_tracing_test.dart b/dev/tracing_tests/test/inflate_widget_tracing_test.dart new file mode 100644 index 0000000000..ffaa0012ca --- /dev/null +++ b/dev/tracing_tests/test/inflate_widget_tracing_test.dart @@ -0,0 +1,85 @@ +// 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/scheduler.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'common.dart'; + +final Set interestingLabels = { + '$Row', + '$TestRoot', + '$TestChildWidget', + '$Container', +}; + +void main() { + WidgetsFlutterBinding.ensureInitialized(); + initTimelineTests(); + test('Children of MultiChildRenderObjectElement show up in tracing', () async { + // We don't have expectations around the first frame because there's a race around + // the warm-up frame that we don't want to get involved in here. + await runFrame(() { runApp(const TestRoot()); }); + await SchedulerBinding.instance.endOfFrame; + await fetchInterestingEvents(interestingLabels); + + debugProfileBuildsEnabled = true; + + await runFrame(() { + TestRoot.state.showRow(); + }); + expect( + await fetchInterestingEventNames(interestingLabels), + ['TestRoot', 'Row', 'TestChildWidget', 'Container', 'TestChildWidget', 'Container'], + ); + + debugProfileBuildsEnabled = false; + }, skip: isBrowser); // [intended] uses dart:isolate and io. +} + +class TestRoot extends StatefulWidget { + const TestRoot({Key? key}) : super(key: key); + + static late TestRootState state; + + @override + State createState() => TestRootState(); +} + +class TestRootState extends State { + @override + void initState() { + super.initState(); + TestRoot.state = this; + } + + bool _showRow = false; + void showRow() { + setState(() { + _showRow = true; + }); + } + + @override + Widget build(BuildContext context) { + return _showRow + ? Row( + children: const [ + TestChildWidget(), + TestChildWidget(), + ], + ) + : Container(); + } +} + +class TestChildWidget extends StatelessWidget { + const TestChildWidget({Key? key}) : super(key: key); + + @override + Widget build(BuildContext context) { + return Container(); + } +} diff --git a/dev/tracing_tests/test/timeline_test.dart b/dev/tracing_tests/test/timeline_test.dart index 9972651022..92cb5bf11a 100644 --- a/dev/tracing_tests/test/timeline_test.dart +++ b/dev/tracing_tests/test/timeline_test.dart @@ -21,15 +21,6 @@ final Set interestingLabels = { '$RenderCustomPaint', }; -Future> fetchInterestingEvents() async { - return (await fetchTimelineEvents()).where((TimelineEvent event) { - return interestingLabels.contains(event.json!['name']) - && event.json!['ph'] == 'B'; // "Begin" mark of events, vs E which is for the "End" mark of events. - }).toList(); -} - -String eventToName(TimelineEvent event) => event.json!['name'] as String; - class TestRoot extends StatefulWidget { const TestRoot({ Key? key }) : super(key: key); @@ -66,12 +57,6 @@ class TestRootState extends State { } } -Future runFrame(VoidCallback callback) { - final Future result = SchedulerBinding.instance.endOfFrame; // schedules a frame - callback(); - return result; -} - void main() { WidgetsFlutterBinding.ensureInitialized(); initTimelineTests(); @@ -80,14 +65,14 @@ void main() { // the warm-up frame that we don't want to get involved in here. await runFrame(() { runApp(const TestRoot()); }); await SchedulerBinding.instance.endOfFrame; - await fetchInterestingEvents(); + await fetchInterestingEvents(interestingLabels); // The next few cases build the exact same tree so should have no effect. debugProfileBuildsEnabled = true; await runFrame(() { TestRoot.state.rebuild(); }); expect( - (await fetchInterestingEvents()).map(eventToName), + await fetchInterestingEventNames(interestingLabels), ['BUILD', 'LAYOUT', 'UPDATING COMPOSITING BITS', 'PAINT', 'COMPOSITING', 'FINALIZE TREE'], ); debugProfileBuildsEnabled = false; @@ -95,7 +80,7 @@ void main() { debugProfileLayoutsEnabled = true; await runFrame(() { TestRoot.state.rebuild(); }); expect( - (await fetchInterestingEvents()).map(eventToName), + await fetchInterestingEventNames(interestingLabels), ['BUILD', 'LAYOUT', 'UPDATING COMPOSITING BITS', 'PAINT', 'COMPOSITING', 'FINALIZE TREE'], ); debugProfileLayoutsEnabled = false; @@ -103,7 +88,7 @@ void main() { debugProfilePaintsEnabled = true; await runFrame(() { TestRoot.state.rebuild(); }); expect( - (await fetchInterestingEvents()).map(eventToName), + await fetchInterestingEventNames(interestingLabels), ['BUILD', 'LAYOUT', 'UPDATING COMPOSITING BITS', 'PAINT', 'COMPOSITING', 'FINALIZE TREE'], ); debugProfilePaintsEnabled = false; @@ -116,7 +101,7 @@ void main() { debugProfileBuildsEnabled = true; await runFrame(() { TestRoot.state.updateWidget(Placeholder(key: UniqueKey(), color: const Color(0xFFFFFFFF))); }); - events = await fetchInterestingEvents(); + events = await fetchInterestingEvents(interestingLabels); expect( events.map(eventToName), ['BUILD', 'Placeholder', 'CustomPaint', 'LAYOUT', 'UPDATING COMPOSITING BITS', 'PAINT', 'COMPOSITING', 'FINALIZE TREE'], @@ -127,7 +112,7 @@ void main() { debugProfileLayoutsEnabled = true; await runFrame(() { TestRoot.state.updateWidget(Placeholder(key: UniqueKey())); }); - events = await fetchInterestingEvents(); + events = await fetchInterestingEvents(interestingLabels); expect( events.map(eventToName), ['BUILD', 'LAYOUT', 'RenderCustomPaint', 'UPDATING COMPOSITING BITS', 'PAINT', 'COMPOSITING', 'FINALIZE TREE'], @@ -140,7 +125,7 @@ void main() { debugProfilePaintsEnabled = true; await runFrame(() { TestRoot.state.updateWidget(Placeholder(key: UniqueKey())); }); - events = await fetchInterestingEvents(); + events = await fetchInterestingEvents(interestingLabels); expect( events.map(eventToName), ['BUILD', 'LAYOUT', 'UPDATING COMPOSITING BITS', 'PAINT', 'RenderCustomPaint', 'COMPOSITING', 'FINALIZE TREE'], diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index b6921412a1..e38e2d083f 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -3550,36 +3550,16 @@ abstract class Element extends DiagnosticableTree implements BuildContext { } else { deactivateChild(child); assert(child._parent == null); - if (!kReleaseMode && debugProfileBuildsEnabled) { - Map debugTimelineArguments = timelineArgumentsIndicatingLandmarkEvent; - assert(() { - debugTimelineArguments = newWidget.toDiagnosticsNode().toTimelineArguments(); - return true; - }()); - Timeline.startSync( - '${newWidget.runtimeType}', - arguments: debugTimelineArguments, - ); - } + // The [debugProfileBuildsEnabled] code for this branch is inside + // [inflateWidget], since some [Element]s call [inflateWidget] directly + // instead of going through [updateChild]. newChild = inflateWidget(newWidget, newSlot); - if (!kReleaseMode && debugProfileBuildsEnabled) - Timeline.finishSync(); } } else { - if (!kReleaseMode && debugProfileBuildsEnabled) { - Map debugTimelineArguments = timelineArgumentsIndicatingLandmarkEvent; - assert(() { - debugTimelineArguments = newWidget.toDiagnosticsNode().toTimelineArguments(); - return true; - }()); - Timeline.startSync( - '${newWidget.runtimeType}', - arguments: debugTimelineArguments, - ); - } + // The [debugProfileBuildsEnabled] code for this branch is inside + // [inflateWidget], since some [Element]s call [inflateWidget] directly + // instead of going through [updateChild]. newChild = inflateWidget(newWidget, newSlot); - if (!kReleaseMode && debugProfileBuildsEnabled) - Timeline.finishSync(); } assert(() { @@ -3807,6 +3787,19 @@ abstract class Element extends DiagnosticableTree implements BuildContext { @pragma('vm:prefer-inline') Element inflateWidget(Widget newWidget, Object? newSlot) { assert(newWidget != null); + + if (!kReleaseMode && debugProfileBuildsEnabled) { + Map debugTimelineArguments = timelineArgumentsIndicatingLandmarkEvent; + assert(() { + debugTimelineArguments = newWidget.toDiagnosticsNode().toTimelineArguments(); + return true; + }()); + Timeline.startSync( + '${newWidget.runtimeType}', + arguments: debugTimelineArguments, + ); + } + final Key? key = newWidget.key; if (key is GlobalKey) { final Element? newChild = _retakeInactiveElement(key, newWidget); @@ -3829,6 +3822,10 @@ abstract class Element extends DiagnosticableTree implements BuildContext { }()); newChild.mount(this, newSlot); assert(newChild._lifecycleState == _ElementLifecycle.active); + + if (!kReleaseMode && debugProfileBuildsEnabled) + Timeline.finishSync(); + return newChild; }