Skip to content

Commit d3aaf61

Browse files
yjbanovyjbanov
committed
address comments; more instrumentation
1 parent 3873df7 commit d3aaf61

6 files changed

Lines changed: 95 additions & 53 deletions

File tree

‎.gitignore‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,9 @@ GeneratedPluginRegistrant.java
1717
*instrumentscli*.trace
1818
*.cipd
1919

20+
# Build directories are produced when building using the Flutter CLI.
21+
build
22+
23+
# This file is produced as a back-up when web_benchmarks fails to parse a
24+
# Chrome trace.
25+
chrome-trace.json

‎packages/web_benchmarks/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ that runs in the browser together with the benchmark code. The server serves the
1212
app's code and assets. Additionally, the server communicates with the browser to
1313
extract the performance traces.
1414

15-
[1]: https://github.com/flutter/packages/tree/packages/web_benchmarks/test/web_benchmarks_test.dart
15+
[1]: https://github.com/flutter/packages/blob/master/packages/web_benchmarks/test/web_benchmarks_test.dart

‎packages/web_benchmarks/lib/src/browser.dart‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,20 @@ typedef ChromeErrorCallback = void Function(String);
6060

6161
/// Manages a single Chrome process.
6262
class Chrome {
63-
Chrome._(this._chromeProcess, this._onError, this._debugConnection) {
64-
// If the Chrome process quits before it was asked to quit, notify the
65-
// error listener.
66-
_chromeProcess.exitCode.then((int exitCode) {
67-
if (!_isStopped) {
68-
_onError('Chrome process exited prematurely with exit code $exitCode');
69-
}
70-
});
63+
Chrome._(this._chromeProcess, this._onError, this._debugConnection,
64+
bool headless) {
65+
if (headless) {
66+
// In headless mode, if the Chrome process quits before it was asked to
67+
// quit, notify the error listener. If it's not running headless, the
68+
// developer may close the browser any time, so it's not considered to
69+
// be an error.
70+
_chromeProcess.exitCode.then((int exitCode) {
71+
if (!_isStopped) {
72+
_onError(
73+
'Chrome process exited prematurely with exit code $exitCode');
74+
}
75+
});
76+
}
7177
}
7278

7379
/// Launches Chrome with the give [options].
@@ -116,7 +122,7 @@ class Chrome {
116122
await _connectToChromeDebugPort(chromeProcess, options.debugPort);
117123
}
118124

119-
return Chrome._(chromeProcess, onError, debugConnection);
125+
return Chrome._(chromeProcess, onError, debugConnection, options.headless);
120126
}
121127

122128
final io.Process _chromeProcess;
@@ -200,6 +206,11 @@ class Chrome {
200206
_isStopped = true;
201207
_chromeProcess.kill();
202208
}
209+
210+
/// Resolves when the Chrome process exits.
211+
Future<void> get whenExits async {
212+
await _chromeProcess.exitCode;
213+
}
203214
}
204215

205216
String _findSystemChromeExecutable() {

‎packages/web_benchmarks/lib/src/runner.dart‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,13 @@ class BenchmarkServer {
171171
json.decode(await request.readAsString());
172172
server.close();
173173
// Keep the stack trace as a string. It's thrown in the browser, not this Dart VM.
174-
profileData.completeError(
175-
'${errorDetails['error']}\n${errorDetails['stackTrace']}');
174+
final String errorMessage =
175+
'Caught browser-side error: ${errorDetails['error']}\n${errorDetails['stackTrace']}';
176+
if (!profileData.isCompleted) {
177+
profileData.completeError(errorMessage);
178+
} else {
179+
io.stderr.writeln(errorMessage);
180+
}
176181
return Response.ok('');
177182
} else if (request.requestedUri.path.endsWith('/next-benchmark')) {
178183
if (benchmarks == null) {
@@ -196,11 +201,18 @@ class BenchmarkServer {
196201
print('[Gallery] $message');
197202
return Response.ok('Reported.');
198203
} else {
204+
io.stderr
205+
.writeln('Unrecognized URL path: ${request.requestedUri.path}');
199206
return Response.notFound(
200207
'This request is not handled by the profile-data handler.');
201208
}
202209
} catch (error, stackTrace) {
203-
profileData.completeError(error, stackTrace);
210+
if (!profileData.isCompleted) {
211+
profileData.completeError(error, stackTrace);
212+
} else {
213+
io.stderr.writeln('Caught error: $error');
214+
io.stderr.writeln('$stackTrace');
215+
}
204216
return Response.internalServerError(body: '$error');
205217
}
206218
}).add(createStaticHandler(
@@ -230,7 +242,11 @@ class BenchmarkServer {
230242
whenChromeIsReady = Chrome.launch(
231243
options,
232244
onError: (String error) {
233-
profileData.completeError(Exception(error));
245+
if (!profileData.isCompleted) {
246+
profileData.completeError(Exception(error));
247+
} else {
248+
io.stderr.writeln('Chrome error: $error');
249+
}
234250
},
235251
workingDirectory: benchmarkAppDirectory.path,
236252
);
@@ -272,8 +288,18 @@ class BenchmarkServer {
272288
}
273289
return BenchmarkResults(results);
274290
} finally {
291+
if (headless) {
292+
chrome?.stop();
293+
} else {
294+
// In non-headless mode wait for the developer to close Chrome
295+
// manually. Otherwise, they won't get a chance to debug anything.
296+
print(
297+
'Benchmark finished. Chrome running in windowed mode. Close '
298+
'Chrome manually to continue.',
299+
);
300+
await chrome?.whenExits;
301+
}
275302
server?.close();
276-
chrome?.stop();
277303
}
278304
}
279305
}

‎packages/web_benchmarks/pubspec.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ environment:
77
sdk: ">=2.7.0 <3.0.0"
88
flutter: ">=1.17.0 <2.0.0"
99

10+
# Using +2 upper limit on some packages to allow null-safe versions
1011
dependencies:
1112
flutter:
1213
sdk: flutter
@@ -18,4 +19,5 @@ dependencies:
1819
process: ">=3.0.13 <5.0.0"
1920
shelf: ">=0.7.5 <1.0.0"
2021
shelf_static: ">=0.2.8 <1.0.0"
22+
test: ">=1.15.0 <3.0.0"
2123
webkit_inspection_protocol: ">=0.7.3 <1.0.0"

‎packages/web_benchmarks/testing/web_benchmarks_test.dart‎

Lines changed: 35 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5,50 +5,47 @@
55
import 'dart:convert' show JsonEncoder;
66
import 'dart:io';
77

8+
import 'package:test/test.dart';
9+
810
import 'package:web_benchmarks/server.dart';
911

1012
Future<void> main() async {
11-
final BenchmarkResults taskResult = await serveWebBenchmark(
12-
benchmarkAppDirectory: Directory('testing/test_app'),
13-
entryPoint: 'lib/benchmarks/runner.dart',
14-
useCanvasKit: false,
15-
);
13+
test('Can run a web benchmark', () async {
14+
final BenchmarkResults taskResult = await serveWebBenchmark(
15+
benchmarkAppDirectory: Directory('testing/test_app'),
16+
entryPoint: 'lib/benchmarks/runner.dart',
17+
useCanvasKit: false,
18+
);
1619

17-
for (final String benchmarkName in <String>['scroll', 'page', 'tap']) {
18-
for (final String metricName in <String>[
19-
'preroll_frame',
20-
'apply_frame',
21-
'drawFrameDuration'
22-
]) {
23-
for (final String valueName in <String>[
24-
'average',
25-
'outlierAverage',
26-
'outlierRatio',
27-
'noise'
20+
for (final String benchmarkName in <String>['scroll', 'page', 'tap']) {
21+
for (final String metricName in <String>[
22+
'preroll_frame',
23+
'apply_frame',
24+
'drawFrameDuration',
2825
]) {
29-
_expect(
30-
taskResult.scores[benchmarkName]
31-
.where((BenchmarkScore score) =>
32-
score.metric == '$metricName.$valueName')
33-
.length,
34-
1,
35-
);
26+
for (final String valueName in <String>[
27+
'average',
28+
'outlierAverage',
29+
'outlierRatio',
30+
'noise',
31+
]) {
32+
expect(
33+
taskResult.scores[benchmarkName].where((BenchmarkScore score) =>
34+
score.metric == '$metricName.$valueName'),
35+
hasLength(1),
36+
);
37+
}
3638
}
39+
expect(
40+
taskResult.scores[benchmarkName].where(
41+
(BenchmarkScore score) => score.metric == 'totalUiFrame.average'),
42+
hasLength(1),
43+
);
3744
}
38-
_expect(
39-
taskResult.scores[benchmarkName]
40-
.where(
41-
(BenchmarkScore score) => score.metric == 'totalUiFrame.average')
42-
.length,
43-
1,
44-
);
45-
}
4645

47-
print(const JsonEncoder.withIndent(' ').convert(taskResult.toJson()));
48-
}
49-
50-
void _expect(Object actual, Object expected) {
51-
if (actual != expected) {
52-
throw Exception('Values different. Expected $expected, but got $actual');
53-
}
46+
expect(
47+
const JsonEncoder.withIndent(' ').convert(taskResult.toJson()),
48+
isA<String>(),
49+
);
50+
}, timeout: Timeout.none);
5451
}

0 commit comments

Comments
 (0)