disable tracing for non-frame based benchmarks (#54236)
diff --git a/dev/benchmarks/macrobenchmarks/lib/src/web/recorder.dart b/dev/benchmarks/macrobenchmarks/lib/src/web/recorder.dart index 37dcdd3..2d0a02f 100644 --- a/dev/benchmarks/macrobenchmarks/lib/src/web/recorder.dart +++ b/dev/benchmarks/macrobenchmarks/lib/src/web/recorder.dart
@@ -50,6 +50,11 @@ /// A function that performs asynchronous work. typedef AsyncVoidCallback = Future<void> Function(); +/// An [AsyncVoidCallback] that doesn't do anything. +/// +/// This is used just so we don't have to deal with null all over the place. +Future<void> _dummyAsyncVoidCallback() async {} + /// Runs the benchmark using the given [recorder]. /// /// Notifies about "set up" and "tear down" events via the [setUpAllDidRun] @@ -61,8 +66,8 @@ /// All arguments must not be null. Runner({ @required this.recorder, - @required this.setUpAllDidRun, - @required this.tearDownAllWillRun, + this.setUpAllDidRun = _dummyAsyncVoidCallback, + this.tearDownAllWillRun = _dummyAsyncVoidCallback, }); /// The recorder that will run and record the benchmark. @@ -95,7 +100,11 @@ /// /// Each benchmark recorder has a [name] and a [run] method at a minimum. abstract class Recorder { - Recorder._(this.name); + Recorder._(this.name, this.isTracingEnabled); + + /// Whether this recorder requires tracing using Chrome's DevTools Protocol's + /// "Tracing" API. + final bool isTracingEnabled; /// The name of the benchmark. /// @@ -143,7 +152,7 @@ /// } /// ``` abstract class RawRecorder extends Recorder { - RawRecorder({@required String name}) : super._(name); + RawRecorder({@required String name}) : super._(name, false); /// The body of the benchmark. /// @@ -188,7 +197,7 @@ /// } /// ``` abstract class SceneBuilderRecorder extends Recorder { - SceneBuilderRecorder({@required String name}) : super._(name); + SceneBuilderRecorder({@required String name}) : super._(name, true); /// Called from [Window.onBeginFrame]. @mustCallSuper @@ -306,7 +315,7 @@ /// } /// ``` abstract class WidgetRecorder extends Recorder implements FrameRecorder { - WidgetRecorder({@required String name}) : super._(name); + WidgetRecorder({@required String name}) : super._(name, true); /// Creates a widget to be benchmarked. /// @@ -377,7 +386,7 @@ /// performance of frames that render the widget and ignoring the frames that /// clear the screen. abstract class WidgetBuildRecorder extends Recorder implements FrameRecorder { - WidgetBuildRecorder({@required String name}) : super._(name); + WidgetBuildRecorder({@required String name}) : super._(name, true); /// Creates a widget to be benchmarked. ///
diff --git a/dev/benchmarks/macrobenchmarks/lib/web_benchmarks.dart b/dev/benchmarks/macrobenchmarks/lib/web_benchmarks.dart index 77b9df1..347d885 100644 --- a/dev/benchmarks/macrobenchmarks/lib/web_benchmarks.dart +++ b/dev/benchmarks/macrobenchmarks/lib/web_benchmarks.dart
@@ -70,19 +70,14 @@ } try { - final Runner runner = Runner( - recorder: recorderFactory(), - setUpAllDidRun: () async { - if (!_client.isInManualMode) { - await _client.startPerformanceTracing(benchmarkName); - } - }, - tearDownAllWillRun: () async { - if (!_client.isInManualMode) { - await _client.stopPerformanceTracing(); - } - }, - ); + final Recorder recorder = recorderFactory(); + final Runner runner = recorder.isTracingEnabled && !_client.isInManualMode + ? Runner( + recorder: recorder, + setUpAllDidRun: () => _client.startPerformanceTracing(benchmarkName), + tearDownAllWillRun: _client.stopPerformanceTracing, + ) + : Runner(recorder: recorder); final Profile profile = await runner.run(); if (!_client.isInManualMode) {
diff --git a/dev/devicelab/lib/framework/browser.dart b/dev/devicelab/lib/framework/browser.dart index c4e4116..7e0def9 100644 --- a/dev/devicelab/lib/framework/browser.dart +++ b/dev/devicelab/lib/framework/browser.dart
@@ -282,6 +282,14 @@ .toList() ..sort((BlinkTraceEvent a, BlinkTraceEvent b) => a.ts - b.ts); + Exception noMeasuredFramesFound() => Exception( + 'No measured frames found in benchmark tracing data. This likely ' + 'indicates a bug in the benchmark. For example, the benchmark failed ' + 'to pump enough frames. It may also indicate a change in Chrome\'s ' + 'tracing data format. Check if Chrome version changed recently and ' + 'adjust the parsing code accordingly.', + ); + // Use the pid from the first "measured_frame" event since the event is // emitted by the script running on the process we're interested in. // @@ -290,7 +298,7 @@ // sometimes, causing to flakes. final BlinkTraceEvent firstMeasuredFrameEvent = events.firstWhere( (BlinkTraceEvent event) => event.isBeginMeasuredFrame, - orElse: () => null, + orElse: () => throw noMeasuredFramesFound(), ); if (firstMeasuredFrameEvent == null) { @@ -330,8 +338,7 @@ print('Skipped $skipCount non-measured frames.'); if (frames.isEmpty) { - // The benchmark is not measuring frames. - return null; + throw noMeasuredFramesFound(); } // Compute averages and summarize.
diff --git a/dev/devicelab/lib/tasks/web_benchmarks.dart b/dev/devicelab/lib/tasks/web_benchmarks.dart index b2e7beb..cecce4b 100644 --- a/dev/devicelab/lib/tasks/web_benchmarks.dart +++ b/dev/devicelab/lib/tasks/web_benchmarks.dart
@@ -65,15 +65,14 @@ server.close(); } - final BlinkTraceSummary traceSummary = BlinkTraceSummary.fromJson(latestPerformanceTrace); - - // Trace summary can be null if the benchmark is not frame-based, such as RawRecorder. - if (traceSummary != null) { + // Trace data is null when the benchmark is not frame-based, such as RawRecorder. + if (latestPerformanceTrace != null) { + final BlinkTraceSummary traceSummary = BlinkTraceSummary.fromJson(latestPerformanceTrace); profile['totalUiFrame.average'] = traceSummary.averageTotalUIFrameTime.inMicroseconds; profile['scoreKeys'] ??= <dynamic>[]; // using dynamic for consistency with JSON profile['scoreKeys'].add('totalUiFrame.average'); + latestPerformanceTrace = null; } - latestPerformanceTrace = null; collectedProfiles.add(profile); return Response.ok('Profile received'); } else if (request.requestedUri.path.endsWith('/start-performance-tracing')) {