-
Notifications
You must be signed in to change notification settings - Fork 6k
Run and collect benchmarks #14556
Run and collect benchmarks #14556
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
secret |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
This is a Dart project that runs the engine benchmarks, and send the metrics to | ||
the cloud for storage and analysis. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
// Copyright 2013 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 'dart:convert'; | ||
import 'dart:io'; | ||
|
||
import 'package:git/git.dart'; | ||
import 'package:metrics_center/flutter.dart'; | ||
import 'package:metrics_center/google_benchmark.dart'; | ||
|
||
Future<String> _getGitRevision() async { | ||
final GitDir gitDir = await GitDir.fromExisting('../../'); | ||
// Somehow gitDir.currentBranch() doesn't work in Cirrus with "fatal: 'HEAD' - | ||
// not a valid ref". Therefore, we use "git log" to get the revision manually. | ||
final ProcessResult logResult = | ||
await gitDir.runCommand(<String>['log', '--pretty=format:%H', '-n', '1']); | ||
if (logResult.exitCode != 0) { | ||
throw 'Unexpected exit code ${logResult.exitCode}'; | ||
} | ||
return logResult.stdout.toString(); | ||
} | ||
|
||
Future<List<FlutterEngineMetricPoint>> _parse(String jsonFileName) async { | ||
final String gitRevision = await _getGitRevision(); | ||
final List<MetricPoint> rawPoints = | ||
await GoogleBenchmarkParser.parse(jsonFileName); | ||
final List<FlutterEngineMetricPoint> points = <FlutterEngineMetricPoint>[]; | ||
for (MetricPoint rawPoint in rawPoints) { | ||
points.add(FlutterEngineMetricPoint( | ||
rawPoint.tags[kNameKey], | ||
rawPoint.value, | ||
gitRevision, | ||
moreTags: rawPoint.tags, | ||
)); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: I would also put parsing an item into a sub function. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
return points; | ||
} | ||
|
||
Future<void> main(List<String> args) async { | ||
if (args.length != 1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using ArgParser()..addFlag, https://dart.dev/tutorials/server/cmdline There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe |
||
throw 'Must have one argument: <benchmark_json_file>'; | ||
} | ||
final List<FlutterEngineMetricPoint> points = await _parse(args[0]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One check to make sure the file(args[0]) exists before moving forward? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current code generates the following error when the file does not exist:
Does that look good? |
||
// The data will be sent to the Datastore of the GCP project specified through | ||
// environment variable BENCHMARK_GCP_CREDENTIALS. The engine Cirrus job has | ||
// currently configured the GCP project to flutter-cirrus for test. We'll | ||
// eventually migrate to flutter-infra project once the test is done. | ||
final FlutterDestination destination = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe good to document which DB this link points to. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
await FlutterDestination.makeFromCredentialsJson( | ||
jsonDecode(Platform.environment['BENCHMARK_GCP_CREDENTIALS']), | ||
); | ||
await destination.update(points); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
{ | ||
"context": { | ||
"date": "2019-12-17 15:14:14", | ||
"num_cpus": 56, | ||
"mhz_per_cpu": 2594, | ||
"cpu_scaling_enabled": true, | ||
"library_build_type": "release" | ||
}, | ||
"benchmarks": [ | ||
{ | ||
"name": "BM_PaintRecordInit", | ||
"iterations": 6749079, | ||
"real_time": 101, | ||
"cpu_time": 101, | ||
"time_unit": "ns" | ||
}, | ||
{ | ||
"name": "BM_ParagraphShortLayout", | ||
"iterations": 151761, | ||
"real_time": 4460, | ||
"cpu_time": 4460, | ||
"time_unit": "ns" | ||
}, | ||
{ | ||
"name": "BM_ParagraphStylesBigO_BigO", | ||
"cpu_coefficient": 6548, | ||
"real_coefficient": 6548, | ||
"big_o": "N", | ||
"time_unit": "ns" | ||
} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
name: flutter_engine_benchmark | ||
|
||
dependencies: | ||
git: any | ||
metrics_center: | ||
# TODO(liyuqian): once metrics_center is properly reviewed, add it to | ||
# flutter/packages, publish on pub.dev, and use the published package here. | ||
git: https://github.com/liyuqian/metrics_center.git | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does metrics center do and why not integrating parse_and_send.dart into metrics center? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metric center is mainly responsible for extract, transform, and load from some metric database (e.g., cocoon) to some other metric database (e.g., skiaperf). Metric center is itself a database so a client (the engine repo) can directly write metrics into it. In this case, I intended to only put the client code/logic in the engine repo. That said, I should probably put the transform code ( Thanks for the suggestion! |
||
|
||
dev_dependencies: | ||
test: any | ||
pedantic: ^1.8.0 | ||
|
||
environment: | ||
sdk: ">=2.2.2 <3.0.0" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
// Copyright 2013 The Flutter Authors. All rights reserved. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here. 2019? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same enforcement of 2013 unfortunately... |
||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
import 'dart:io'; | ||
|
||
import 'package:test/test.dart'; | ||
|
||
void main() { | ||
// In order to run this test, one should download a service account | ||
// credentials json from a test GCP project, and put that json as | ||
// `secret/test_gcp_credentials.json`. There's a `flutter-test` project for | ||
// Flutter team members. | ||
test('parse_and_send with example json does not crash.', () async { | ||
final String testCred = | ||
File('secret/test_gcp_credentials.json').readAsStringSync(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This "secret" directory is git-ignored, then how could another one run this test? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One should download a service account credentials json from a test GCP project, and put that json as |
||
Process.runSync('dart', <String>[ | ||
'bin/parse_and_send.dart', | ||
'example/txt_benchmarks.json', | ||
], environment: <String, String>{ | ||
'BENCHMARK_GCP_CREDENTIALS': testCred, | ||
}); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2019?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The engine repo enforces 2013 through the license check script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see.