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

[web_benchmarks] Replace initialPage parameter with benchmarkPath #7743

Merged
merged 24 commits into from
Sep 30, 2024
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
1 change: 1 addition & 0 deletions packages/web_benchmarks/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
logs/
5 changes: 4 additions & 1 deletion packages/web_benchmarks/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## 2.1.0-wip
## 3.0.0

* **Breaking change:** removed the `initialPage` parameter from the `serveWebBenchmark`
method and `runBenchmarks` method. Replaced this parameter with `benchmarkPath`, which
allows for passing the combined value of the URL path segments, fragment, and query parameters.
* Restructure the `testing/test_app` to make the example benchmarks easier to follow.

## 2.0.2
Expand Down
22 changes: 13 additions & 9 deletions packages/web_benchmarks/lib/client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,12 @@ extension on HTMLElement {
///
/// When used without a server, prompts the user to select a benchmark to
/// run next.
///
/// [benchmarkPath] specifies the path for the URL that will be loaded in Chrome
/// when reloading the window for subsequent benchmark runs.
Future<void> runBenchmarks(
Map<String, RecorderFactory> benchmarks, {
String initialPage = defaultInitialPage,
String benchmarkPath = defaultInitialPath,
}) async {
// Set local benchmarks.
_benchmarks = benchmarks;
Expand All @@ -60,14 +63,15 @@ Future<void> runBenchmarks(
await _runBenchmark(nextBenchmark);

final Uri currentUri = Uri.parse(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 String newUri = Uri(
scheme: currentUri.scheme,
host: currentUri.host,
port: currentUri.port,
path: initialPage,
).toString();
// Create a new URI with the parsed value of [benchmarkPath] to ensure the
// benchmark app is reloaded with the proper configuration.
final String newUri = Uri.parse(benchmarkPath)
.replace(
scheme: currentUri.scheme,
host: currentUri.host,
port: currentUri.port,
)
.toString();

// Reloading the window will trigger the next benchmark to run.
await _client.printToConsole(
Expand Down
7 changes: 5 additions & 2 deletions packages/web_benchmarks/lib/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,18 @@ const int defaultChromeDebugPort = 10000;
/// [compilationOptions] specify the compiler and renderer to use for the
/// benchmark app. This can either use dart2wasm & skwasm or
/// dart2js & canvaskit.
///
/// [benchmarkPath] specifies the path for the URL that will be loaded upon
/// opening the benchmark app in Chrome.
Future<BenchmarkResults> serveWebBenchmark({
required io.Directory benchmarkAppDirectory,
required String entryPoint,
int benchmarkServerPort = defaultBenchmarkServerPort,
int chromeDebugPort = defaultChromeDebugPort,
bool headless = true,
bool treeShakeIcons = true,
String initialPage = defaultInitialPage,
CompilationOptions compilationOptions = const CompilationOptions.js(),
String benchmarkPath = defaultInitialPath,
}) async {
// Reduce logging level. Otherwise, package:webkit_inspection_protocol is way too spammy.
Logger.root.level = Level.INFO;
Expand All @@ -64,10 +67,10 @@ Future<BenchmarkResults> serveWebBenchmark({
benchmarkAppDirectory: benchmarkAppDirectory,
entryPoint: entryPoint,
benchmarkServerPort: benchmarkServerPort,
benchmarkPath: benchmarkPath,
chromeDebugPort: chromeDebugPort,
headless: headless,
compilationOptions: compilationOptions,
treeShakeIcons: treeShakeIcons,
initialPage: initialPage,
).run();
}
6 changes: 3 additions & 3 deletions packages/web_benchmarks/lib/src/common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ const int kMeasuredSampleCount = 100;
/// 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';
/// The default initial path for the URL that will be loaded upon opening the
/// benchmark app or reloading it in Chrome.
const String defaultInitialPath = 'index.html';
45 changes: 36 additions & 9 deletions packages/web_benchmarks/lib/src/runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ class BenchmarkServer {
/// [compilationOptions] specify the compiler and renderer to use for the
/// benchmark app. This can either use dart2wasm & skwasm or
/// dart2js & canvaskit.
///
/// [benchmarkPath] specifies the path for the URL that will be loaded upon
/// opening the benchmark app in Chrome.
BenchmarkServer({
required this.benchmarkAppDirectory,
required this.entryPoint,
Expand All @@ -61,7 +64,7 @@ class BenchmarkServer {
required this.headless,
required this.treeShakeIcons,
this.compilationOptions = const CompilationOptions.js(),
this.initialPage = defaultInitialPage,
this.benchmarkPath = defaultInitialPath,
});

final ProcessManager _processManager = const LocalProcessManager();
Expand Down Expand Up @@ -97,13 +100,25 @@ class BenchmarkServer {
/// When false, '--no-tree-shake-icons' will be passed as a build argument.
final bool treeShakeIcons;

/// The initial page to load upon opening the benchmark app in Chrome.
/// The initial path for the URL that will be loaded upon opening the
/// benchmark app in Chrome.
///
/// This path should contain the path segments, fragment, and/or query
/// parameters that are required for the benchmark. This value will be parsed
/// by `Uri.parse` and combined with the benchmark URI scheme ('http'), host
/// ('localhost'), and port [benchmarkServerPort] to create the URL for
/// loading in Chrome. See [_benchmarkAppUrl].
///
/// The default value is [defaultInitialPage].
final String initialPage;
/// The default value is [defaultInitialPath].
final String benchmarkPath;

String get _benchmarkAppUrl =>
'http://localhost:$benchmarkServerPort/$initialPage';
String get _benchmarkAppUrl => Uri.parse(benchmarkPath)
.replace(
scheme: 'http',
host: 'localhost',
port: benchmarkServerPort,
)
.toString();

/// Builds and serves the benchmark app, and collects benchmark results.
Future<BenchmarkResults> run() async {
Expand Down Expand Up @@ -169,6 +184,7 @@ class BenchmarkServer {
path.join(benchmarkAppDirectory.path, 'build', 'web'),
defaultDocument: 'index.html',
);

// We want our page to be crossOriginIsolated. This will allow us to run the
// skwasm renderer, which uses a SharedArrayBuffer, which requires the page
// to be crossOriginIsolated. But also, even in the non-skwasm case, running
Expand Down Expand Up @@ -277,9 +293,20 @@ class BenchmarkServer {
}
});

// If all previous handlers returned HTTP 404, this is the last handler
// that simply warns about the unrecognized path.
cascade = cascade.add((Request request) {
// If all previous handlers returned HTTP 404, this handler either serves
// the static handler at the default document (for GET requests only) or
// warns about the unrecognized path.
cascade = cascade.add((Request request) async {
if (request.method == 'GET') {
final Uri newRequestUri = request.requestedUri.replace(path: '/');
final Request newRequest = Request(
request.method,
newRequestUri,
headers: request.headers,
);
return await buildFolderHandler(newRequest);
}

io.stderr.writeln('Unrecognized URL path: ${request.requestedUri.path}');
return Response.notFound('Not found: ${request.requestedUri.path}');
});
Expand Down
2 changes: 1 addition & 1 deletion packages/web_benchmarks/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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: 2.1.0-wip
version: 3.0.0

environment:
sdk: ^3.3.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ class Automator {
await _handleAppTap();
case BenchmarkName.simpleCompilationCheck:
_handleSimpleCompilationCheck();
case BenchmarkName.simpleInitialPageCheck:
_handleSimpleInitialPageCheck();
case BenchmarkName.simpleBenchmarkPathCheck:
_handleSimpleBenchmarkPathCheck();
}

// At the end of the test, mark as finished.
Expand Down Expand Up @@ -117,12 +117,12 @@ class Automator {
profile.extraData['isWasm'] = kIsWasm ? 1 : 0;
}

void _handleSimpleInitialPageCheck() {
// Record whether the URL contains the expected initial page so we can
// verify the behavior of setting the `initialPage` on the benchmark server.
final bool containsExpectedPage =
window.location.toString().contains(testBenchmarkInitialPage);
profile.extraData['expectedUrl'] = containsExpectedPage ? 1 : 0;
void _handleSimpleBenchmarkPathCheck() {
// Record whether the URL contains the expected path so we can verify the
// behavior of setting the `benchmarkPath` on the benchmark server.
final bool containsExpectedPath =
window.location.toString().contains(testBenchmarkPath);
profile.extraData['expectedUrl'] = containsExpectedPath ? 1 : 0;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import '../recorder.dart';
Future<void> main() async {
await runBenchmarks(
<String, RecorderFactory>{
BenchmarkName.simpleInitialPageCheck.name: () => TestAppRecorder(
benchmark: BenchmarkName.simpleInitialPageCheck,
BenchmarkName.simpleBenchmarkPathCheck.name: () => TestAppRecorder(
benchmark: BenchmarkName.simpleBenchmarkPathCheck,
),
},
benchmarkPath: testBenchmarkPath,
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

const String testBenchmarkInitialPage = 'index.html#about';
/// The benchmark path to load in the URL when loading or reloading the
/// benchmark app in Chrome.
const String testBenchmarkPath = 'about';

enum BenchmarkName {
appNavigate,
appScroll,
appTap,
simpleInitialPageCheck,
simpleBenchmarkPathCheck,
simpleCompilationCheck;
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,25 @@ Future<void> main() async {
);

test(
'Can run a web benchmark with an alternate initial page',
'Can run a web benchmark with an alternate benchmarkPath',
() async {
final BenchmarkResults results = await _runBenchmarks(
benchmarkNames: <String>[BenchmarkName.simpleInitialPageCheck.name],
benchmarkNames: <String>[BenchmarkName.simpleBenchmarkPathCheck.name],
entryPoint:
'benchmark/test_infra/client/simple_initial_page_client.dart',
initialPage: testBenchmarkInitialPage,
'benchmark/test_infra/client/simple_benchmark_path_client.dart',
benchmarkPath: testBenchmarkPath,
);

// The runner puts an `expectedUrl` metric in the results so that we can
// verify the initial page value that should be passed on initial load
// and on reloads.
final List<BenchmarkScore>? scores =
results.scores[BenchmarkName.simpleInitialPageCheck.name];
results.scores[BenchmarkName.simpleBenchmarkPathCheck.name];
expect(scores, isNotNull);

final BenchmarkScore isWasmScore = scores!
// The runner puts an `expectedUrl` metric in the results so that we can
// verify the initial page value that should be passed on initial load
// and on reloads.
final BenchmarkScore expectedUrlScore = scores!
.firstWhere((BenchmarkScore score) => score.metric == 'expectedUrl');
expect(isWasmScore.value, 1);
expect(expectedUrlScore.value, 1);
},
timeout: Timeout.none,
);
Expand Down Expand Up @@ -79,14 +79,14 @@ Future<void> main() async {
Future<BenchmarkResults> _runBenchmarks({
required List<String> benchmarkNames,
required String entryPoint,
String initialPage = defaultInitialPage,
String benchmarkPath = defaultInitialPath,
CompilationOptions compilationOptions = const CompilationOptions.js(),
}) async {
final BenchmarkResults taskResult = await serveWebBenchmark(
benchmarkAppDirectory: Directory('testing/test_app'),
entryPoint: entryPoint,
treeShakeIcons: false,
initialPage: initialPage,
benchmarkPath: benchmarkPath,
compilationOptions: compilationOptions,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

import 'package:flutter/material.dart';
import 'package:go_router/go_router.dart';

class AboutPage extends StatelessWidget {
const AboutPage({super.key});
Expand All @@ -12,7 +13,7 @@ class AboutPage extends StatelessWidget {
return Scaffold(
appBar: AppBar(
leading: BackButton(
onPressed: () => Navigator.of(context).pop(),
onPressed: () => context.canPop() ? context.pop() : context.go('/'),
),
),
body: Center(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

import 'package:flutter/material.dart';
import 'package:go_router/go_router.dart';

const ValueKey<String> textKey = ValueKey<String>('textKey');
const ValueKey<String> aboutPageKey = ValueKey<String>('aboutPageKey');
Expand Down Expand Up @@ -34,7 +35,7 @@ class _HomePageState extends State<HomePage> {
IconButton(
key: aboutPageKey,
icon: const Icon(Icons.help_outline),
onPressed: () => Navigator.of(context).pushNamed('about'),
onPressed: () => context.go('/about'),
),
],
),
Expand Down
24 changes: 18 additions & 6 deletions packages/web_benchmarks/testing/test_app/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,44 @@
// found in the LICENSE file.

import 'package:flutter/material.dart';
import 'package:flutter_web_plugins/url_strategy.dart';
import 'package:go_router/go_router.dart';

import 'about_page.dart';
import 'home_page.dart';

void main() {
usePathUrlStrategy();
runApp(const MyApp());
}

final GoRouter _router = GoRouter(
routes: <GoRoute>[
GoRoute(
path: '/',
builder: (_, __) => const HomePage(title: 'Flutter Demo Home Page'),
),
GoRoute(
path: '/about',
builder: (_, __) => const AboutPage(),
),
],
);

class MyApp extends StatelessWidget {
const MyApp({super.key});

@override
Widget build(BuildContext context) {
return MaterialApp(
return MaterialApp.router(
title: 'Flutter Demo',
theme: ThemeData(
primarySwatch: Colors.blue,
visualDensity: VisualDensity.adaptivePlatformDensity,
),
routerConfig: _router,
// This blocks the About page button.
debugShowCheckedModeBanner: false,
initialRoute: 'home',
routes: <String, WidgetBuilder>{
'home': (_) => const HomePage(title: 'Flutter Demo Home Page'),
'about': (_) => const AboutPage(),
},
);
}
}
3 changes: 3 additions & 0 deletions packages/web_benchmarks/testing/test_app/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ environment:
dependencies:
flutter:
sdk: flutter
flutter_web_plugins:
sdk: flutter
go_router: ^14.2.7

dev_dependencies:
flutter_lints: ^4.0.0
Expand Down