From bc5f6375b7cad944a843903b0dcb67bcd4e6096f Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Wed, 8 Sep 2021 15:20:19 -0700 Subject: [PATCH 1/5] Add _shownFirstScript marker to debugger codeview --- packages/devtools_app/lib/src/debugger/codeview.dart | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/devtools_app/lib/src/debugger/codeview.dart b/packages/devtools_app/lib/src/debugger/codeview.dart index 0877649a1a8..a158de1b05f 100644 --- a/packages/devtools_app/lib/src/debugger/codeview.dart +++ b/packages/devtools_app/lib/src/debugger/codeview.dart @@ -80,10 +80,14 @@ class _CodeViewState extends State ParsedScript get parsedScript => widget.parsedScript; + bool _shownFirstScript; + @override void initState() { super.initState(); + _shownFirstScript = false; + verticalController = LinkedScrollControllerGroup(); gutterController = verticalController.addAndGet(); textController = verticalController.addAndGet(); @@ -114,6 +118,7 @@ class _CodeViewState extends State horizontalController.dispose(); widget.controller.scriptLocation .removeListener(_handleScriptLocationChanged); + _shownFirstScript = false; super.dispose(); } @@ -182,6 +187,12 @@ class _CodeViewState extends State return const CenteredCircularProgressIndicator(); } + if (!_shownFirstScript) { + print('shown first script'); + // TODO(annagrin): mark end of IPL timing for debugger page load here. + _shownFirstScript = true; + } + return ValueListenableBuilder( valueListenable: widget.controller.showSearchInFileField, builder: (context, showSearch, _) { From cf43c0dbe603436ae457700ca3ee773834c7af3f Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Wed, 8 Sep 2021 15:21:05 -0700 Subject: [PATCH 2/5] remove print --- packages/devtools_app/lib/src/debugger/codeview.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/devtools_app/lib/src/debugger/codeview.dart b/packages/devtools_app/lib/src/debugger/codeview.dart index a158de1b05f..324157aaef4 100644 --- a/packages/devtools_app/lib/src/debugger/codeview.dart +++ b/packages/devtools_app/lib/src/debugger/codeview.dart @@ -188,7 +188,6 @@ class _CodeViewState extends State } if (!_shownFirstScript) { - print('shown first script'); // TODO(annagrin): mark end of IPL timing for debugger page load here. _shownFirstScript = true; } From 2c62d90f1e2b92fc3a2560ed566e57c78302be59 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Wed, 8 Sep 2021 15:39:48 -0700 Subject: [PATCH 3/5] Move to debugger screen --- packages/devtools_app/lib/src/debugger/codeview.dart | 9 --------- .../devtools_app/lib/src/debugger/debugger_screen.dart | 10 ++++++++++ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/devtools_app/lib/src/debugger/codeview.dart b/packages/devtools_app/lib/src/debugger/codeview.dart index 324157aaef4..d5767636818 100644 --- a/packages/devtools_app/lib/src/debugger/codeview.dart +++ b/packages/devtools_app/lib/src/debugger/codeview.dart @@ -80,14 +80,10 @@ class _CodeViewState extends State ParsedScript get parsedScript => widget.parsedScript; - bool _shownFirstScript; - @override void initState() { super.initState(); - _shownFirstScript = false; - verticalController = LinkedScrollControllerGroup(); gutterController = verticalController.addAndGet(); textController = verticalController.addAndGet(); @@ -187,11 +183,6 @@ class _CodeViewState extends State return const CenteredCircularProgressIndicator(); } - if (!_shownFirstScript) { - // TODO(annagrin): mark end of IPL timing for debugger page load here. - _shownFirstScript = true; - } - return ValueListenableBuilder( valueListenable: widget.controller.showSearchInFileField, builder: (context, showSearch, _) { diff --git a/packages/devtools_app/lib/src/debugger/debugger_screen.dart b/packages/devtools_app/lib/src/debugger/debugger_screen.dart index 0bcde4584a9..016e3f0c501 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_screen.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_screen.dart @@ -80,10 +80,13 @@ class DebuggerScreenBodyState extends State DebuggerController controller; + bool _shownFirstScript; + @override void initState() { super.initState(); ga.screen(DebuggerScreen.id); + _shownFirstScript = false; } @override @@ -97,6 +100,7 @@ class DebuggerScreenBodyState extends State @override void dispose() { + _shownFirstScript = false; super.dispose(); } @@ -114,6 +118,12 @@ class DebuggerScreenBodyState extends State return ValueListenableBuilder( valueListenable: controller.currentParsedScript, builder: (context, parsedScript, _) { + if (scriptRef != null && + parsedScript != null && + !_shownFirstScript) { + // TODO(annagrin): mark end of IPL timing for debugger page here. + _shownFirstScript = true; + } return CodeView( key: DebuggerScreenBody.codeViewKey, controller: controller, From 209e430b7519edd6524e212b50cd93fae2e30186 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Wed, 8 Sep 2021 16:10:59 -0700 Subject: [PATCH 4/5] Review comments and add analytics timing --- .../lib/src/analytics/_analytics_stub.dart | 8 +++ .../lib/src/analytics/_analytics_web.dart | 55 +++++++++++++++++++ .../lib/src/analytics/constants.dart | 3 + .../lib/src/debugger/codeview.dart | 1 - .../lib/src/debugger/debugger_screen.dart | 9 +-- 5 files changed, 69 insertions(+), 7 deletions(-) diff --git a/packages/devtools_app/lib/src/analytics/_analytics_stub.dart b/packages/devtools_app/lib/src/analytics/_analytics_stub.dart index 6237258de2f..1b66ef962c9 100644 --- a/packages/devtools_app/lib/src/analytics/_analytics_stub.dart +++ b/packages/devtools_app/lib/src/analytics/_analytics_stub.dart @@ -26,6 +26,14 @@ void screen( int value = 0, ]) {} +void timeStart(String screenName, String timedOperation) {} + +void timeEnd( + String screenName, + String timedOperation, { + ScreenAnalyticsMetrics Function() screenMetricsProvider, +}) {} + void timeSync( String screenName, String timedOperation, { diff --git a/packages/devtools_app/lib/src/analytics/_analytics_web.dart b/packages/devtools_app/lib/src/analytics/_analytics_web.dart index 6d6eb221ad3..2ca77184cea 100644 --- a/packages/devtools_app/lib/src/analytics/_analytics_web.dart +++ b/packages/devtools_app/lib/src/analytics/_analytics_web.dart @@ -275,6 +275,60 @@ void screen( ); } +String _timedOperationKeyHelper(String screenName, String timedOperation) { + return '$screenName-$timedOperation'; +} + +final _timedOperationsInProgress = {}; + +// Use this method coupled with `timeEnd` when an operation cannot be timed in +// a callback, but rather needs to be timed instead at two disjoint start and +// end marks. +void timeStart(String screenName, String timedOperation) { + final startTime = DateTime.now(); + final operationKey = _timedOperationKeyHelper( + screenName, + timedOperation, + ); + _timedOperationsInProgress[operationKey] = startTime; +} + +// Use this method coupled with `timeStart` when an operation cannot be timed in +// a callback, but rather needs to be timed instead at two disjoint start and +// end marks. +void timeEnd( + String screenName, + String timedOperation, { + ScreenAnalyticsMetrics Function() screenMetricsProvider, +}) { + final endTime = DateTime.now(); + final operationKey = _timedOperationKeyHelper( + screenName, + timedOperation, + ); + final startTime = _timedOperationsInProgress[operationKey]; + if (startTime == null) { + log( + 'Could not time operation "$timedOperation" because a) `timeEnd` was ' + 'called before `timeStart` or b) the `screenName` and `timedOperation`' + 'parameters for the `timeStart` and `timeEnd` calls do not match.', + LogLevel.warning, + ); + return; + } + final durationMicros = + endTime.microsecondsSinceEpoch - startTime.microsecondsSinceEpoch; + _timing( + screenName, + timedOperation, + durationMicros: durationMicros, + screenMetrics: + screenMetricsProvider != null ? screenMetricsProvider() : null, + ); + _timedOperationsInProgress.remove(operationKey); +} + +// Use this when a synchronous operation can be timed in a callback. void timeSync( String screenName, String timedOperation, { @@ -305,6 +359,7 @@ void timeSync( ); } +// Use this when an asynchronous operation can be timed in a callback. Future timeAsync( String screenName, String timedOperation, { diff --git a/packages/devtools_app/lib/src/analytics/constants.dart b/packages/devtools_app/lib/src/analytics/constants.dart index 4d56b60ab55..d571ce88bfe 100644 --- a/packages/devtools_app/lib/src/analytics/constants.dart +++ b/packages/devtools_app/lib/src/analytics/constants.dart @@ -105,3 +105,6 @@ const String export = 'export'; const String expandAll = 'expandAll'; const String collapseAll = 'collapseAll'; const String documentationLink = 'documentationLink'; +// This should track the time from `initState` for a screen to the time when +// the page data has loaded and is ready to interact with. +const String pageReady = 'pageReady'; diff --git a/packages/devtools_app/lib/src/debugger/codeview.dart b/packages/devtools_app/lib/src/debugger/codeview.dart index d5767636818..0877649a1a8 100644 --- a/packages/devtools_app/lib/src/debugger/codeview.dart +++ b/packages/devtools_app/lib/src/debugger/codeview.dart @@ -114,7 +114,6 @@ class _CodeViewState extends State horizontalController.dispose(); widget.controller.scriptLocation .removeListener(_handleScriptLocationChanged); - _shownFirstScript = false; super.dispose(); } diff --git a/packages/devtools_app/lib/src/debugger/debugger_screen.dart b/packages/devtools_app/lib/src/debugger/debugger_screen.dart index 016e3f0c501..9231ed72a79 100644 --- a/packages/devtools_app/lib/src/debugger/debugger_screen.dart +++ b/packages/devtools_app/lib/src/debugger/debugger_screen.dart @@ -9,6 +9,7 @@ import 'package:provider/provider.dart'; import 'package:vm_service/vm_service.dart'; import '../analytics/analytics.dart' as ga; +import '../analytics/constants.dart' as analytics_constants; import '../auto_dispose_mixin.dart'; import '../common_widgets.dart'; import '../dialogs.dart'; @@ -86,6 +87,7 @@ class DebuggerScreenBodyState extends State void initState() { super.initState(); ga.screen(DebuggerScreen.id); + ga.timeStart(DebuggerScreen.id, analytics_constants.pageReady); _shownFirstScript = false; } @@ -98,12 +100,6 @@ class DebuggerScreenBodyState extends State controller = newController; } - @override - void dispose() { - _shownFirstScript = false; - super.dispose(); - } - void _onLocationSelected(ScriptLocation location) { if (location != null) { controller.showScriptLocation(location); @@ -121,6 +117,7 @@ class DebuggerScreenBodyState extends State if (scriptRef != null && parsedScript != null && !_shownFirstScript) { + ga.timeEnd(DebuggerScreen.id, analytics_constants.pageReady); // TODO(annagrin): mark end of IPL timing for debugger page here. _shownFirstScript = true; } From dd635697b7573d61ad7a4129d6cf475f857e3043 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Thu, 9 Sep 2021 09:30:42 -0700 Subject: [PATCH 5/5] review comments --- .../devtools_app/lib/src/analytics/_analytics_web.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/devtools_app/lib/src/analytics/_analytics_web.dart b/packages/devtools_app/lib/src/analytics/_analytics_web.dart index 2ca77184cea..c9e0ace7d8a 100644 --- a/packages/devtools_app/lib/src/analytics/_analytics_web.dart +++ b/packages/devtools_app/lib/src/analytics/_analytics_web.dart @@ -275,7 +275,7 @@ void screen( ); } -String _timedOperationKeyHelper(String screenName, String timedOperation) { +String _operationKey(String screenName, String timedOperation) { return '$screenName-$timedOperation'; } @@ -286,7 +286,7 @@ final _timedOperationsInProgress = {}; // end marks. void timeStart(String screenName, String timedOperation) { final startTime = DateTime.now(); - final operationKey = _timedOperationKeyHelper( + final operationKey = _operationKey( screenName, timedOperation, ); @@ -302,11 +302,12 @@ void timeEnd( ScreenAnalyticsMetrics Function() screenMetricsProvider, }) { final endTime = DateTime.now(); - final operationKey = _timedOperationKeyHelper( + final operationKey = _operationKey( screenName, timedOperation, ); - final startTime = _timedOperationsInProgress[operationKey]; + final startTime = _timedOperationsInProgress.remove(operationKey); + assert(startTime != null); if (startTime == null) { log( 'Could not time operation "$timedOperation" because a) `timeEnd` was ' @@ -325,7 +326,6 @@ void timeEnd( screenMetrics: screenMetricsProvider != null ? screenMetricsProvider() : null, ); - _timedOperationsInProgress.remove(operationKey); } // Use this when a synchronous operation can be timed in a callback.