Unverified Commit 4e3cf198 authored by Yegor's avatar Yegor Committed by GitHub

disable tracing for non-frame based benchmarks (#54236)

parent d47ad7ec
......@@ -50,6 +50,11 @@ Duration timeAction(VoidCallback action) {
/// 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 @@ class Runner {
/// 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 @@ class Runner {
///
/// 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 Recorder {
/// }
/// ```
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 RawRecorder extends Recorder {
/// }
/// ```
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 SceneBuilderRecorder extends Recorder {
/// }
/// ```
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 @@ abstract class WidgetRecorder extends Recorder implements FrameRecorder {
/// 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.
///
......
......@@ -70,19 +70,14 @@ Future<void> _runBenchmark(String benchmarkName) async {
}
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) {
......
......@@ -282,6 +282,14 @@ class BlinkTraceSummary {
.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 @@ class BlinkTraceSummary {
// 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 @@ class BlinkTraceSummary {
print('Skipped $skipCount non-measured frames.');
if (frames.isEmpty) {
// The benchmark is not measuring frames.
return null;
throw noMeasuredFramesFound();
}
// Compute averages and summarize.
......
......@@ -65,15 +65,14 @@ Future<TaskResult> runWebBenchmark({ @required bool useCanvasKit }) async {
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')) {
......
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