Skip to content

Commit

Permalink
Unskip the wasm benchmark (#8320)
Browse files Browse the repository at this point in the history
  • Loading branch information
kenzieschmoll authored Oct 9, 2024
1 parent 228289f commit 06a27e1
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 52 deletions.
57 changes: 28 additions & 29 deletions packages/devtools_app/benchmark/devtools_benchmarks_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,19 @@
import 'dart:convert' show JsonEncoder;
import 'dart:io';

import 'package:collection/collection.dart';
import 'package:test/test.dart';
import 'package:web_benchmarks/metrics.dart';
import 'package:web_benchmarks/server.dart';

import 'test_infra/common.dart';
import 'test_infra/project_root_directory.dart';

List<String> metricList({required bool useWasm}) => [
// The skwasm renderer doesn't have preroll or apply frame steps in its
// rendering.
if (!useWasm) ...[
'preroll_frame',
'apply_frame',
],
'drawFrameDuration',
];
const _isWasmScore = 'isWasm';

final valueList = <String>[
'average',
'outlierAverage',
'outlierRatio',
'noise',
const _extraScores = [
totalUiFrameAverage,
_isWasmScore,
];

/// Tests that the DevTools web benchmarks are run and reported correctly.
Expand All @@ -40,10 +32,6 @@ void main() {
await _runBenchmarks(useWasm: useWasm);
},
timeout: const Timeout(Duration(minutes: 10)),
// TODO(https://github.com/dart-lang/sdk/issues/56664): unskip the wasm
// benchmarks once this issue is resolved and we can bump DevTools to a
// version of Flutter that includes the fix.
skip: useWasm,
);
}

Expand All @@ -54,12 +42,12 @@ Future<void> _runBenchmarks({bool useWasm = false}) async {
stdout.writeln('Starting web benchmark tests ...');
final taskResult = await serveWebBenchmark(
benchmarkAppDirectory: projectRootDirectory(),
entryPoint: 'benchmark/test_infra/client.dart',
entryPoint: generateBenchmarkEntryPoint(useWasm: useWasm),
compilationOptions: useWasm
? const CompilationOptions.wasm()
: const CompilationOptions.js(),
treeShakeIcons: false,
initialPage: benchmarkInitialPage,
benchmarkPath: benchmarkPath(useWasm: useWasm),
);
stdout.writeln('Web benchmark tests finished.');

Expand All @@ -69,29 +57,40 @@ Future<void> _runBenchmarks({bool useWasm = false}) async {
);

for (final benchmarkName in DevToolsBenchmark.values.map((e) => e.id)) {
final expectedMetrics = metricList(useWasm: useWasm);
final expectedMetrics = expectedBenchmarkMetrics(useWasm: useWasm)
.map((BenchmarkMetric metric) => metric.label)
.toList();
const expectedComputations = BenchmarkMetricComputation.values;
final scores = taskResult.scores[benchmarkName] ?? [];
expect(
taskResult.scores[benchmarkName],
hasLength(expectedMetrics.length * valueList.length + 1),
scores,
hasLength(
expectedMetrics.length * expectedComputations.length +
_extraScores.length,
),
);

for (final metricName in expectedMetrics) {
for (final valueName in valueList) {
for (final computation in expectedComputations) {
expect(
taskResult.scores[benchmarkName]?.where(
(score) => score.metric == '$metricName.$valueName',
scores.where(
(score) => score.metric == '$metricName.${computation.name}',
),
hasLength(1),
);
}
}

expect(
taskResult.scores[benchmarkName]?.where(
(score) => score.metric == 'totalUiFrame.average',
),
scores.where((score) => score.metric == totalUiFrameAverage),
hasLength(1),
);

final isWasmScore = scores.firstWhereOrNull(
(BenchmarkScore score) => score.metric == _isWasmScore,
);
expect(isWasmScore, isNotNull);
expect(isWasmScore!.value, useWasm ? 1 : 0);
}

expect(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ import 'utils.dart';
/// using the '--baseline' or '--test' arguments.
///
/// Example usage:
/// * dart run benchmark/scripts/run_benchmarks.dart
/// * dart run benchmark/scripts/dart2wasm_performance_diff.dart
///
/// Example usage that averages benchmark results over 5 runs:
/// * dart run benchmark/scripts/run_benchmarks.dart --average-of=5
/// * dart run benchmark/scripts/dart2wasm_performance_diff.dart --average-of=5
///
/// Example usage that diffs against an existing basline:
/// * dart run benchmark/scripts/run_benchmarks.dart --baseline=/Users/me/Downloads/baseline_run.json
/// * dart run benchmark/scripts/dart2wasm_performance_diff.dart --baseline=/Users/me/Downloads/baseline_run.json
void main(List<String> args) async {
if (!Directory.current.path.contains('devtools_app')) {
stderr.writeln(
Expand Down
4 changes: 2 additions & 2 deletions packages/devtools_app/benchmark/scripts/run_benchmarks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ Future<BenchmarkResults> runBenchmarks({
benchmarkResults.add(
await serveWebBenchmark(
benchmarkAppDirectory: projectRootDirectory(),
entryPoint: 'benchmark/test_infra/client.dart',
entryPoint: generateBenchmarkEntryPoint(useWasm: useWasm),
compilationOptions: useWasm
? const CompilationOptions.wasm()
: const CompilationOptions.js(),
treeShakeIcons: false,
initialPage: benchmarkInitialPage,
benchmarkPath: benchmarkPath(useWasm: useWasm),
headless: !useBrowser,
),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import 'dart:async';
import 'package:devtools_app/devtools_app.dart';
import 'package:devtools_test/helpers.dart';
import 'package:devtools_test/test_data.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:web_benchmarks/client.dart';

import '../common.dart';
import '_cpu_profiler_automator.dart';
Expand All @@ -21,6 +23,7 @@ class DevToolsAutomater {
DevToolsAutomater({
required this.benchmark,
required this.stopWarmingUpCallback,
required this.profile,
});

/// The current benchmark.
Expand All @@ -32,6 +35,9 @@ class DevToolsAutomater {
/// as over.
final void Function() stopWarmingUpCallback;

/// The profile collected for the running benchmark
final Profile profile;

/// Whether the automation has ended.
bool finished = false;

Expand Down Expand Up @@ -65,6 +71,12 @@ class DevToolsAutomater {
await PerformanceScreenAutomator(controller).run();
}

// Record whether we are in wasm mode or not. Ideally, we'd have a more
// first-class way to add metadata like this, but this will work for us to
// pass information about the environment back to the server for the
// purposes of our own tests.
profile.extraData['isWasm'] = kIsWasm ? 1 : 0;

// At the end of the test, mark as finished.
finished = true;
}
Expand Down
17 changes: 17 additions & 0 deletions packages/devtools_app/benchmark/test_infra/client/client_js.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright 2024 The Chromium 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 'package:web_benchmarks/client.dart';

import '../common.dart';
import 'client_shared.dart';

/// Runs the client of the DevTools web benchmarks.
///
/// When the DevTools web benchmarks are run, the server builds an app with this
/// file as the entry point (see `run_benchmarks.dart`). The app automates
/// the DevTools web app, records some performance data, and reports them.
Future<void> main() async {
await runBenchmarks(benchmarks, benchmarkPath: benchmarkPath(useWasm: false));
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// Copyright 2023 The Chromium Authors. All rights reserved.
// Copyright 2024 The Chromium 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 'package:web_benchmarks/client.dart';

import 'common.dart';
import 'devtools_recorder.dart';
import '../common.dart';
import '../devtools_recorder.dart';

typedef RecorderFactory = Recorder Function();

Expand All @@ -20,12 +20,3 @@ final benchmarks = <String, RecorderFactory>{
benchmark: DevToolsBenchmark.offlinePerformanceScreen,
),
};

/// Runs the client of the DevTools web benchmarks.
///
/// When the DevTools web benchmarks are run, the server builds an app with this
/// file as the entry point (see `run_benchmarks.dart`). The app automates
/// the DevTools web app, records some performance data, and reports them.
Future<void> main() async {
await runBenchmarks(benchmarks, initialPage: benchmarkInitialPage);
}
17 changes: 17 additions & 0 deletions packages/devtools_app/benchmark/test_infra/client/client_wasm.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright 2024 The Chromium 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 'package:web_benchmarks/client.dart';

import '../common.dart';
import 'client_shared.dart';

/// Runs the client of the DevTools web benchmarks.
///
/// When the DevTools web benchmarks are run, the server builds an app with this
/// file as the entry point (see `run_benchmarks.dart`). The app automates
/// the DevTools web app, records some performance data, and reports them.
Future<void> main() async {
await runBenchmarks(benchmarks, benchmarkPath: benchmarkPath(useWasm: true));
}
21 changes: 16 additions & 5 deletions packages/devtools_app/benchmark/test_infra/common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,22 @@

/// The initial page to load upon opening the DevTools benchmark app or
/// reloading it in Chrome.
//
// We use an empty initial page so that the benchmark server does not attempt
// to load the default page 'index.html', which will show up as "page not
// found" in DevTools.
const benchmarkInitialPage = '';
///
/// We use an empty initial page so that the benchmark server does not attempt
/// to load the default page 'index.html', which will show up as "page not
/// found" in DevTools.
const _benchmarkInitialPage = '';

const _wasmQueryParameters = {'wasm': 'true'};

String benchmarkPath({required bool useWasm}) => Uri(
path: _benchmarkInitialPage,
queryParameters: useWasm ? _wasmQueryParameters : null,
).toString();

String generateBenchmarkEntryPoint({required bool useWasm}) {
return 'benchmark/test_infra/client/client_${useWasm ? 'wasm' : 'js'}.dart';
}

const devtoolsBenchmarkPrefix = 'devtools';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class DevToolsRecorder extends WidgetRecorder {
_devToolsAutomator = DevToolsAutomater(
benchmark: benchmark,
stopWarmingUpCallback: profile.stopWarmingUp,
profile: profile,
);
return _devToolsAutomator!.createWidget();
}
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools_app/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ dev_dependencies:
mockito: ^5.4.1
stager: ^1.0.1
test: ^1.21.0
web_benchmarks: ^2.0.0
web_benchmarks: ^3.1.0
webkit_inspection_protocol: ">=0.5.0 <2.0.0"

flutter:
Expand Down

0 comments on commit 06a27e1

Please sign in to comment.