Skip to content

Fix benchmark reload bug and migrate away from deprecated js_util APIs #5577

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

Merged
merged 6 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions packages/web_benchmarks/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.1.0+10

* Ensure the benchmark client reloads with the proper `initialPage`.
* Migrates benchmark recorder away from deprecated `js_util` APIs.

## 0.1.0+9

* Updates minimum supported SDK version to Flutter 3.10/Dart 3.0.
Expand Down
19 changes: 17 additions & 2 deletions packages/web_benchmarks/lib/client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ final LocalBenchmarkServerClient _client = LocalBenchmarkServerClient();
///
/// When used without a server, prompts the user to select a benchmark to
/// run next.
Future<void> runBenchmarks(Map<String, RecorderFactory> benchmarks) async {
Future<void> runBenchmarks(
Map<String, RecorderFactory> benchmarks, {
String initialPage = defaultInitialPage,
}) async {
// Set local benchmarks.
_benchmarks = benchmarks;

Expand All @@ -43,7 +46,19 @@ Future<void> runBenchmarks(Map<String, RecorderFactory> benchmarks) async {
}

await _runBenchmark(nextBenchmark);
html.window.location.reload();

final Uri currentUri = Uri.parse(html.window.location.href);
// Create a new URI with the current 'page' value set to [initialPage] to
// ensure the benchmark app is reloaded at the proper location.
final Uri newUri = Uri(
scheme: currentUri.scheme,
host: currentUri.host,
port: currentUri.port,
path: initialPage,
);

// Reloading the window will trigger the next benchmark to run.
html.window.location.replace(newUri.toString());
}

Future<void> _runBenchmark(String? benchmarkName) async {
Expand Down
3 changes: 2 additions & 1 deletion packages/web_benchmarks/lib/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'dart:io' as io;
import 'package:logging/logging.dart';

import 'src/benchmark_result.dart';
import 'src/common.dart';
import 'src/runner.dart';

export 'src/benchmark_result.dart';
Expand Down Expand Up @@ -47,7 +48,7 @@ Future<BenchmarkResults> serveWebBenchmark({
int chromeDebugPort = defaultChromeDebugPort,
bool headless = true,
bool treeShakeIcons = true,
String initialPage = BenchmarkServer.defaultInitialPage,
String initialPage = defaultInitialPage,
}) async {
// Reduce logging level. Otherwise, package:webkit_inspection_protocol is way too spammy.
Logger.root.level = Level.INFO;
Expand Down
4 changes: 4 additions & 0 deletions packages/web_benchmarks/lib/src/common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,7 @@ const int kMeasuredSampleCount = 100;
/// A special value returned by the `/next-benchmark` HTTP POST request when
/// all benchmarks have run and there are no more benchmarks to run.
const String kEndOfBenchmarks = '__end_of_benchmarks__';

/// The default initial page to load upon opening the benchmark app or reloading
/// it in Chrome.
const String defaultInitialPage = 'index.html';
21 changes: 11 additions & 10 deletions packages/web_benchmarks/lib/src/recorder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@

import 'dart:async';
import 'dart:html' as html;
import 'dart:js';
import 'dart:js_util' as js_util;
import 'dart:math' as math;
import 'dart:ui';
import 'dart:ui_web' as ui_web;

import 'package:flutter/foundation.dart';
import 'package:flutter/gestures.dart';
Expand Down Expand Up @@ -1211,19 +1210,21 @@ final Map<String, EngineBenchmarkValueListener> _engineBenchmarkListeners =
///
/// If another listener is already registered, overrides it.
void registerEngineBenchmarkValueListener(
String name, EngineBenchmarkValueListener listener) {
String name,
EngineBenchmarkValueListener listener,
) {
if (_engineBenchmarkListeners.containsKey(name)) {
throw StateError('A listener for "$name" is already registered.\n'
'Call `stopListeningToEngineBenchmarkValues` to unregister the previous '
'listener before registering a new one.');
throw StateError(
'A listener for "$name" is already registered.\n'
'Call `stopListeningToEngineBenchmarkValues` to unregister the previous '
'listener before registering a new one.',
);
}

if (_engineBenchmarkListeners.isEmpty) {
// The first listener is being registered. Register the global listener.
js_util.setProperty(html.window, '_flutter_internal_on_benchmark',
allowInterop(_dispatchEngineBenchmarkValue));
ui_web.benchmarkValueCallback = _dispatchEngineBenchmarkValue;
}

_engineBenchmarkListeners[name] = listener;
}

Expand All @@ -1232,7 +1233,7 @@ void stopListeningToEngineBenchmarkValues(String name) {
_engineBenchmarkListeners.remove(name);
if (_engineBenchmarkListeners.isEmpty) {
// The last listener unregistered. Remove the global listener.
js_util.setProperty(html.window, '_flutter_internal_on_benchmark', null);
ui_web.benchmarkValueCallback = null;
}
}

Expand Down
5 changes: 0 additions & 5 deletions packages/web_benchmarks/lib/src/runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ class BenchmarkServer {
this.initialPage = defaultInitialPage,
});

/// The default initial page to load upon opening the benchmark app in Chrome.
///
/// This value will be used by default if no value is passed to [initialPage].
static const String defaultInitialPage = 'index.html';

final ProcessManager _processManager = const LocalProcessManager();

/// The directory containing the app that's being benchmarked.
Expand Down
6 changes: 3 additions & 3 deletions packages/web_benchmarks/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ name: web_benchmarks
description: A benchmark harness for performance-testing Flutter apps in Chrome.
repository: https://github.com/flutter/packages/tree/main/packages/web_benchmarks
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+web_benchmarks%22
version: 0.1.0+9
version: 0.1.0+10

environment:
sdk: ">=3.0.0 <4.0.0"
flutter: ">=3.10.0"
sdk: ">=3.2.0 <4.0.0"
flutter: ">=3.16.0"

dependencies:
flutter:
Expand Down
4 changes: 2 additions & 2 deletions packages/web_benchmarks/testing/web_benchmarks_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import 'dart:io';
import 'package:test/test.dart';

import 'package:web_benchmarks/server.dart';
import 'package:web_benchmarks/src/runner.dart';
import 'package:web_benchmarks/src/common.dart';

Future<void> main() async {
test('Can run a web benchmark', () async {
Expand All @@ -30,7 +30,7 @@ Future<void> main() async {
Future<void> _runBenchmarks({
required List<String> benchmarkNames,
required String entryPoint,
String initialPage = BenchmarkServer.defaultInitialPage,
String initialPage = defaultInitialPage,
}) async {
final BenchmarkResults taskResult = await serveWebBenchmark(
benchmarkAppDirectory: Directory('testing/test_app'),
Expand Down