Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unskip the wasm benchmark #8320

Merged
merged 19 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion flutter-candidate.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
8925e1ffdfe880b06a8bc5877f017083d6652f5b
e3828ec9402204bb81fcdb9c5fb7154367b4a6dd
48 changes: 30 additions & 18 deletions packages/devtools_app/benchmark/devtools_benchmarks_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@
import 'dart:convert' show JsonEncoder;
import 'dart:io';

import 'package:collection/collection.dart';
import 'package:test/test.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}) => [
List<String> _metricList({required bool useWasm}) => [
// The skwasm renderer doesn't have preroll or apply frame steps in its
// rendering.
if (!useWasm) ...[
Expand All @@ -24,13 +25,22 @@ List<String> metricList({required bool useWasm}) => [
'drawFrameDuration',
];

final valueList = <String>[
final _valueList = <String>[
'average',
'outlierAverage',
'outlierRatio',
'noise',
];

const _totalUiFrameAverageScore = 'totalUiFrame.average';

const _isWasmScore = 'isWasm';

const _extraScores = [
_totalUiFrameAverageScore,
_isWasmScore,
];

/// Tests that the DevTools web benchmarks are run and reported correctly.
void main() {
for (final useWasm in [true, false]) {
Expand All @@ -40,10 +50,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 +60,13 @@ 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:
'benchmark/test_infra/client/client_${useWasm ? 'wasm' : 'js'}.dart',
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 +76,34 @@ Future<void> _runBenchmarks({bool useWasm = false}) async {
);

for (final benchmarkName in DevToolsBenchmark.values.map((e) => e.id)) {
final expectedMetrics = metricList(useWasm: useWasm);
final expectedMetrics = _metricList(useWasm: useWasm);
final scores = taskResult.scores[benchmarkName] ?? [];
expect(
taskResult.scores[benchmarkName],
hasLength(expectedMetrics.length * valueList.length + 1),
scores,
hasLength(
expectedMetrics.length * _valueList.length + _extraScores.length,
),
);

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

expect(
taskResult.scores[benchmarkName]?.where(
(score) => score.metric == 'totalUiFrame.average',
),
scores.where((score) => score.metric == _totalUiFrameAverageScore),
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 @@ -60,7 +60,7 @@ Future<BenchmarkResults> runBenchmarks({
? 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
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);
}
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));
}
17 changes: 12 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,18 @@

/// 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();

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.0.0
webkit_inspection_protocol: ">=0.5.0 <2.0.0"

flutter:
Expand Down
Loading