Unverified Commit 30d3866a authored by keyonghan's avatar keyonghan Committed by GitHub

Add extra benchmark metrics to test name in addition to builder name (#92530)

parent 53de8899
...@@ -54,7 +54,7 @@ Future<FlutterDestination> connectFlutterDestination() async { ...@@ -54,7 +54,7 @@ Future<FlutterDestination> connectFlutterDestination() async {
/// "host_type": "linux", /// "host_type": "linux",
/// "host_version": "debian-10.11" /// "host_version": "debian-10.11"
/// } /// }
List<MetricPoint> parse(Map<String, dynamic> resultsJson, Map<String, dynamic> benchmarkTags) { List<MetricPoint> parse(Map<String, dynamic> resultsJson, Map<String, dynamic> benchmarkTags, String taskName) {
print('Results to upload to skia perf: $resultsJson'); print('Results to upload to skia perf: $resultsJson');
print('Benchmark tags to upload to skia perf: $benchmarkTags'); print('Benchmark tags to upload to skia perf: $benchmarkTags');
final List<String> scoreKeys = final List<String> scoreKeys =
...@@ -82,6 +82,18 @@ List<MetricPoint> parse(Map<String, dynamic> resultsJson, Map<String, dynamic> b ...@@ -82,6 +82,18 @@ List<MetricPoint> parse(Map<String, dynamic> resultsJson, Map<String, dynamic> b
tags, tags,
), ),
); );
// Add an extra entry under test name. This way we have duplicate metrics
// under both builder name and test name. Once we have enough data and update
// existing alerts to point to test name, we can deprecate builder name ones.
// https://github.com/flutter/flutter/issues/74522#issuecomment-942575581
tags[kNameKey] = taskName;
metricPoints.add(
MetricPoint(
(resultData[scoreKey] as num).toDouble(),
tags,
),
);
} }
return metricPoints; return metricPoints;
} }
...@@ -99,7 +111,7 @@ Future<void> upload( ...@@ -99,7 +111,7 @@ Future<void> upload(
FlutterDestination metricsDestination, FlutterDestination metricsDestination,
List<MetricPoint> metricPoints, List<MetricPoint> metricPoints,
int commitTimeSinceEpoch, int commitTimeSinceEpoch,
String? taskName, String taskName,
) async { ) async {
await metricsDestination.update( await metricsDestination.update(
metricPoints, metricPoints,
...@@ -107,7 +119,7 @@ Future<void> upload( ...@@ -107,7 +119,7 @@ Future<void> upload(
commitTimeSinceEpoch, commitTimeSinceEpoch,
isUtc: true, isUtc: true,
), ),
taskName ?? 'default', taskName,
); );
} }
...@@ -127,11 +139,12 @@ Future<void> uploadToSkiaPerf(String? resultsPath, String? commitTime, String? t ...@@ -127,11 +139,12 @@ Future<void> uploadToSkiaPerf(String? resultsPath, String? commitTime, String? t
} else { } else {
commitTimeSinceEpoch = DateTime.now().millisecondsSinceEpoch; commitTimeSinceEpoch = DateTime.now().millisecondsSinceEpoch;
} }
taskName = taskName ?? 'default';
final Map<String, dynamic> benchmarkTagsMap = jsonDecode(benchmarkTags ?? '{}') as Map<String, dynamic>; final Map<String, dynamic> benchmarkTagsMap = jsonDecode(benchmarkTags ?? '{}') as Map<String, dynamic>;
final File resultFile = File(resultsPath); final File resultFile = File(resultsPath);
Map<String, dynamic> resultsJson = <String, dynamic>{}; Map<String, dynamic> resultsJson = <String, dynamic>{};
resultsJson = json.decode(await resultFile.readAsString()) as Map<String, dynamic>; resultsJson = json.decode(await resultFile.readAsString()) as Map<String, dynamic>;
final List<MetricPoint> metricPoints = parse(resultsJson, benchmarkTagsMap); final List<MetricPoint> metricPoints = parse(resultsJson, benchmarkTagsMap, taskName);
final FlutterDestination metricsDestination = await connectFlutterDestination(); final FlutterDestination metricsDestination = await connectFlutterDestination();
await upload(metricsDestination, metricPoints, commitTimeSinceEpoch, taskName); await upload(metricsDestination, metricPoints, commitTimeSinceEpoch, taskName);
} }
...@@ -24,6 +24,27 @@ class FakeFlutterDestination implements FlutterDestination { ...@@ -24,6 +24,27 @@ class FakeFlutterDestination implements FlutterDestination {
void main() { void main() {
group('Parse', () { group('Parse', () {
test('duplicate entries for both builder name and test name', () {
final Map<String, dynamic> results = <String, dynamic>{
'CommitBranch': 'master',
'CommitSha': 'abc',
'BuilderName': 'Linux test',
'ResultData': <String, dynamic>{
'average_frame_build_time_millis': 0.4550425531914895,
},
'BenchmarkScoreKeys': <String>[
'average_frame_build_time_millis',
],
};
final List<MetricPoint> metricPoints = parse(results, <String, String>{}, 'test');
expect(metricPoints.length, 2);
expect(metricPoints[0].value, equals(0.4550425531914895));
expect(metricPoints[0].tags[kNameKey], 'Linux test');
expect(metricPoints[1].value, equals(0.4550425531914895));
expect(metricPoints[1].tags[kNameKey], 'test');
});
test('without additional benchmark tags', () { test('without additional benchmark tags', () {
final Map<String, dynamic> results = <String, dynamic>{ final Map<String, dynamic> results = <String, dynamic>{
'CommitBranch': 'master', 'CommitBranch': 'master',
...@@ -38,10 +59,10 @@ void main() { ...@@ -38,10 +59,10 @@ void main() {
'90th_percentile_frame_build_time_millis', '90th_percentile_frame_build_time_millis',
], ],
}; };
final List<MetricPoint> metricPoints = parse(results, <String, String>{}); final List<MetricPoint> metricPoints = parse(results, <String, String>{}, 'task abc');
expect(metricPoints[0].value, equals(0.4550425531914895)); expect(metricPoints[0].value, equals(0.4550425531914895));
expect(metricPoints[1].value, equals(0.473)); expect(metricPoints[2].value, equals(0.473));
}); });
test('with additional benchmark tags', () { test('with additional benchmark tags', () {
...@@ -65,12 +86,12 @@ void main() { ...@@ -65,12 +86,12 @@ void main() {
'host_type': 'linux', 'host_type': 'linux',
'host_version': 'debian-10.11' 'host_version': 'debian-10.11'
}; };
final List<MetricPoint> metricPoints = parse(results, tags); final List<MetricPoint> metricPoints = parse(results, tags, 'task abc');
expect(metricPoints[0].value, equals(0.4550425531914895)); expect(metricPoints[0].value, equals(0.4550425531914895));
expect(metricPoints[0].tags.keys.contains('arch'), isTrue); expect(metricPoints[0].tags.keys.contains('arch'), isTrue);
expect(metricPoints[1].value, equals(0.473)); expect(metricPoints[2].value, equals(0.473));
expect(metricPoints[1].tags.keys.contains('device_type'), isTrue); expect(metricPoints[2].tags.keys.contains('device_type'), isTrue);
}); });
test('succeeds - null ResultData', () { test('succeeds - null ResultData', () {
...@@ -81,7 +102,7 @@ void main() { ...@@ -81,7 +102,7 @@ void main() {
'ResultData': null, 'ResultData': null,
'BenchmarkScoreKeys': null, 'BenchmarkScoreKeys': null,
}; };
final List<MetricPoint> metricPoints = parse(results, <String, String>{}); final List<MetricPoint> metricPoints = parse(results, <String, String>{}, 'tetask abcst');
expect(metricPoints.length, 0); expect(metricPoints.length, 0);
}); });
...@@ -102,9 +123,9 @@ void main() { ...@@ -102,9 +123,9 @@ void main() {
'90th_percentile_frame_build_time_millis', '90th_percentile_frame_build_time_millis',
], ],
}; };
final List<MetricPoint> metricPoints = parse(results, <String, String>{}); final List<MetricPoint> metricPoints = parse(results, <String, String>{}, 'task abc');
final FakeFlutterDestination flutterDestination = FakeFlutterDestination(); final FakeFlutterDestination flutterDestination = FakeFlutterDestination();
String? taskName; const String taskName = 'default';
const int commitTimeSinceEpoch = 1629220312; const int commitTimeSinceEpoch = 1629220312;
await upload(flutterDestination, metricPoints, commitTimeSinceEpoch, taskName); await upload(flutterDestination, metricPoints, commitTimeSinceEpoch, taskName);
...@@ -126,7 +147,7 @@ void main() { ...@@ -126,7 +147,7 @@ void main() {
'90th_percentile_frame_build_time_millis', '90th_percentile_frame_build_time_millis',
], ],
}; };
final List<MetricPoint> metricPoints = parse(results, <String, String>{}); final List<MetricPoint> metricPoints = parse(results, <String, String>{}, 'task abc');
final FakeFlutterDestination flutterDestination = FakeFlutterDestination(); final FakeFlutterDestination flutterDestination = FakeFlutterDestination();
const String taskName = 'test'; const String taskName = 'test';
const int commitTimeSinceEpoch = 1629220312; const int commitTimeSinceEpoch = 1629220312;
......
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