Skip to content

Added new gallery benchmark test that forces opengles. #131796

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.

Sign up for GitHub

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
gaaclarke merged 4 commits into from
Aug 3, 2023
Merged

Added new gallery benchmark test that forces opengles. #131796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81e0a1a
Added new gallery benchmark test that forces opengles.
gaaclarke Aug 2, 2023
4eaf15b
flutter update-packages --force-upgrade
gaaclarke Aug 2, 2023
76a9927
Made the manifest get reset
gaaclarke Aug 2, 2023
300c785
Merge branch 'master' into opengles-new-gallery-benchmark
XilaiZhang Aug 3, 2023
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
11 changes: 11 additions & 0 deletions .ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2309,6 +2309,17 @@ targets:
["devicelab", "android", "linux", "samsung", "s10"]
task_name: new_gallery_impeller__transition_perf

- name: Linux_samsung_s10 new_gallery_opengles_impeller__transition_perf
bringup: true
recipe: devicelab/devicelab_drone
presubmit: false
timeout: 60
properties:
ignore_flakiness: "true"
tags: >
["devicelab", "android", "linux", "samsung", "s10"]
task_name: new_gallery_opengles_impeller__transition_perf

- name: Linux_android picture_cache_perf__e2e_summary
recipe: devicelab/devicelab_drone
presubmit: false
Expand Down
1 change: 1 addition & 0 deletions TESTOWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@
/dev/devicelab/bin/tasks/microbenchmarks.dart @zanderso @flutter/engine
/dev/devicelab/bin/tasks/new_gallery__transition_perf.dart @zanderso @flutter/engine
/dev/devicelab/bin/tasks/new_gallery_impeller__transition_perf.dart @zanderso @flutter/engine
/dev/devicelab/bin/tasks/new_gallery_opengles_impeller__transition_perf.dart @gaaclarke @flutter/engine
/dev/devicelab/bin/tasks/picture_cache_perf__timeline_summary.dart @zanderso @flutter/engine
/dev/devicelab/bin/tasks/platform_channel_sample_test.dart @zanderso @flutter/engine
/dev/devicelab/bin/tasks/platform_interaction_test.dart @stuartmorgan @flutter/plugin
Expand Down
4 changes: 3 additions & 1 deletion dev/bots/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ dependencies:
mime: 1.0.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
node_preamble: 2.0.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
package_config: 2.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
petitparser: 5.4.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
pool: 1.5.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
pub_semver: 2.1.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
pubspec_parse: 1.2.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
Expand All @@ -68,9 +69,10 @@ dependencies:
web: 0.1.4-beta # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
web_socket_channel: 2.4.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
webkit_inspection_protocol: 1.2.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
xml: 6.3.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
yaml: 3.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"

dev_dependencies:
test_api: 0.6.1

# PUBSPEC CHECKSUM: e586
# PUBSPEC CHECKSUM: 7431
24 changes: 24 additions & 0 deletions dev/devicelab/bin/tasks/new_gallery_opengles_impeller__transition_perf.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2014 The Flutter 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 'dart:io';

import 'package:flutter_devicelab/framework/devices.dart';
import 'package:flutter_devicelab/framework/framework.dart';
import 'package:flutter_devicelab/framework/utils.dart';
import 'package:flutter_devicelab/tasks/new_gallery.dart';
import 'package:path/path.dart' as path;

Future<void> main() async {
deviceOperatingSystem = DeviceOperatingSystem.android;

final Directory galleryParentDir = Directory.systemTemp.createTempSync('flutter_new_gallery_test.');
final Directory galleryDir = Directory(path.join(galleryParentDir.path, 'gallery'));

try {
await task(NewGalleryPerfTest(galleryDir, enableImpeller: true, forceOpenGLES: true).run);
} finally {
rmTree(galleryParentDir);
}
}
1 change: 1 addition & 0 deletions dev/devicelab/lib/tasks/new_gallery.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class NewGalleryPerfTest extends PerfTest {
String dartDefine = '',
super.enableImpeller,
super.timeoutSeconds,
super.forceOpenGLES,
}) : super(
galleryDir.path,
'test_driver/transitions_perf.dart',
Expand Down
138 changes: 108 additions & 30 deletions dev/devicelab/lib/tasks/perf_tests.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'dart:math' as math;

import 'package:meta/meta.dart';
import 'package:path/path.dart' as path;
import 'package:xml/xml.dart';

import '../framework/devices.dart';
import '../framework/framework.dart';
Expand Down Expand Up @@ -691,6 +692,63 @@ Map<String, dynamic> _average(List<Map<String, dynamic>> results, int iterations
return tally;
}

/// Opens the file at testDirectory + 'android/app/src/main/AndroidManifest.xml'
/// and adds the following entry to the application.
/// <meta-data
/// android:name="io.flutter.embedding.android.ImpellerBackend"
/// android:value="opengles" />
void _addOpenGLESToManifest(String testDirectory) {
final String manifestPath = path.join(
testDirectory, 'android', 'app', 'src', 'main', 'AndroidManifest.xml');
final File file = File(manifestPath);

if (!file.existsSync()) {
throw Exception('AndroidManifest.xml not found at $manifestPath');
}

final String xmlStr = file.readAsStringSync();
final XmlDocument xmlDoc = XmlDocument.parse(xmlStr);
const String key = 'io.flutter.embedding.android.ImpellerBackend';
const String value = 'opengles';

final XmlElement applicationNode =
xmlDoc.findAllElements('application').first;

// Check if the meta-data node already exists.
final Iterable<XmlElement> existingMetaData = applicationNode
.findAllElements('meta-data')
.where((XmlElement node) => node.getAttribute('android:name') == key);

if (existingMetaData.isNotEmpty) {
final XmlElement existingEntry = existingMetaData.first;
existingEntry.setAttribute('android:value', value);
} else {
final XmlElement metaData = XmlElement(
XmlName('meta-data'),
<XmlAttribute>[
XmlAttribute(XmlName('android:name'), key),
XmlAttribute(XmlName('android:value'), value)
],
);

applicationNode.children.add(metaData);
}

file.writeAsStringSync(xmlDoc.toXmlString(pretty: true, indent: ' '));
}

Future<void> _resetManifest(String testDirectory) async {
final String manifestPath = path.join(
testDirectory, 'android', 'app', 'src', 'main', 'AndroidManifest.xml');
final File file = File(manifestPath);

if (!file.existsSync()) {
throw Exception('AndroidManifest.xml not found at $manifestPath');
}

await exec('git', <String>['checkout', file.path]);
}

/// Measure application startup performance.
class StartupTest {
const StartupTest(this.testDirectory, { this.reportMetrics = true, this.target = 'lib/main.dart' });
Expand Down Expand Up @@ -978,6 +1036,7 @@ class PerfTest {
this.flutterDriveCallback,
this.timeoutSeconds,
this.enableImpeller,
this.forceOpenGLES,
}): _resultFilename = resultFilename;

const PerfTest.e2e(
Expand All @@ -995,6 +1054,7 @@ class PerfTest {
this.flutterDriveCallback,
this.timeoutSeconds,
this.enableImpeller,
this.forceOpenGLES,
}) : saveTraceFile = false, timelineFileName = null, _resultFilename = resultFilename;

/// The directory where the app under test is defined.
Expand Down Expand Up @@ -1031,6 +1091,9 @@ class PerfTest {
/// Whether the perf test should enable Impeller.
final bool? enableImpeller;

/// Whether the perf test force Impeller's OpenGLES backend.
final bool? forceOpenGLES;

/// Number of seconds to time out the test after, allowing debug callbacks to run.
final int? timeoutSeconds;

Expand Down Expand Up @@ -1079,40 +1142,55 @@ class PerfTest {
final String? localEngine = localEngineFromEnv;
final String? localEngineSrcPath = localEngineSrcPathFromEnv;

final List<String> options = <String>[
if (localEngine != null)
...<String>['--local-engine', localEngine],
if (localEngineSrcPath != null)
...<String>['--local-engine-src-path', localEngineSrcPath],
'--no-dds',
'--no-android-gradle-daemon',
'-v',
'--verbose-system-logs',
'--profile',
if (timeoutSeconds != null)
...<String>[
Future<void> Function()? manifestReset;
if (forceOpenGLES ?? false) {
assert(enableImpeller!);
_addOpenGLESToManifest(testDirectory);
Copy link
Member

@zanderso zanderso Aug 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably overly paranoid, but I think restoring the file to its original contents in a finally {} couldn't hurt.

Copy link
Member Author

@gaaclarke gaaclarke Aug 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is an error inside of _addOpenGLESToManifest there is no need to restore the file since the file is only edited in the last line.

It looks like writeAsStringSync is not atomic so if writing somehow failed it could be garbage. But the cases where the writing fail would probably be an environment where the test wouldn't execute properly anyways, no?

This test is modifying its own copy of the gallery in its own temporary directory so this shouldn't effect other tests, see https://github.com/flutter/flutter/pull/131796/files#diff-514ad19bc7a9b9e196b1568127fed85ee10552d62baa32f960d694a98cfc83f1R16.

Let me know if there is a situation I'm not considering.

Copy link
Member

@zanderso zanderso Aug 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I said, I'm maybe being a bit paranoid, but suppose _addOpenGLESToManifest is used by mistake for an some other integration test that lives in flutter/flutter, and that the change to the xml file persists by mistake across multiple benchmark runs.

Copy link
Member Author

@gaaclarke gaaclarke Aug 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, done.

manifestReset = () => _resetManifest(testDirectory);
}

try {
final List<String> options = <String>[
if (localEngine != null) ...<String>['--local-engine', localEngine],
if (localEngineSrcPath != null) ...<String>[
'--local-engine-src-path',
localEngineSrcPath
],
'--no-dds',
'--no-android-gradle-daemon',
'-v',
'--verbose-system-logs',
'--profile',
if (timeoutSeconds != null) ...<String>[
'--timeout',
timeoutSeconds.toString(),
],
if (needsFullTimeline)
'--trace-startup', // Enables "endless" timeline event buffering.
'-t', testTarget,
if (testDriver != null)
...<String>['--driver', testDriver!],
if (existingApp != null)
...<String>['--use-existing-app', existingApp],
if (dartDefine.isNotEmpty)
...<String>['--dart-define', dartDefine],
if (enableImpeller != null && enableImpeller!) '--enable-impeller',
if (enableImpeller != null && !enableImpeller!) '--no-enable-impeller',
'-d',
deviceId,
];
if (flutterDriveCallback != null) {
flutterDriveCallback!(options);
} else {
await flutter('drive', options: options);
if (needsFullTimeline)
'--trace-startup', // Enables "endless" timeline event buffering.
'-t', testTarget,
if (testDriver != null) ...<String>['--driver', testDriver!],
if (existingApp != null) ...<String>[
'--use-existing-app',
existingApp
],
if (dartDefine.isNotEmpty) ...<String>['--dart-define', dartDefine],
if (enableImpeller != null && enableImpeller!) '--enable-impeller',
if (enableImpeller != null && !enableImpeller!)
'--no-enable-impeller',
'-d',
deviceId,
];
if (flutterDriveCallback != null) {
flutterDriveCallback!(options);
} else {
await flutter('drive', options: options);
}
} finally {
if (manifestReset != null) {
await manifestReset();
}
}

final Map<String, dynamic> data = json.decode(
file('${_testOutputDirectory(testDirectory)}/$resultFilename.json').readAsStringSync(),
) as Map<String, dynamic>;
Expand Down
4 changes: 3 additions & 1 deletion dev/devicelab/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ dependencies:
vm_service: 11.8.0
web: 0.1.4-beta
webkit_inspection_protocol: 1.2.0
xml: 6.3.0

_discoveryapis_commons: 1.0.6 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
async: 2.11.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
Expand All @@ -37,6 +38,7 @@ dependencies:
js: 0.6.7 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
json_annotation: 4.8.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
mime: 1.0.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
petitparser: 5.4.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
pub_semver: 2.1.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
retry: 3.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
source_span: 1.10.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
Expand Down Expand Up @@ -71,4 +73,4 @@ dev_dependencies:
watcher: 1.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
web_socket_channel: 2.4.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"

# PUBSPEC CHECKSUM: a9f3
# PUBSPEC CHECKSUM: 289e
7 changes: 4 additions & 3 deletions packages/flutter_tools/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ dependencies:
archive: 3.3.2
args: 2.4.2
browser_launcher: 1.1.1
dds: 2.9.2
dds: 2.9.4
dwds: 19.0.2
completion: 1.0.1
coverage: 1.6.3
Expand Down Expand Up @@ -70,7 +70,7 @@ dependencies:
dap: 1.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
dart_internal: 0.2.9 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
dds_service_extensions: 1.6.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
devtools_shared: 2.25.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
devtools_shared: 2.26.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
fixnum: 1.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
frontend_server_client: 3.2.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
glob: 2.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
Expand All @@ -92,6 +92,7 @@ dependencies:
term_glyph: 1.2.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
typed_data: 1.3.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
watcher: 1.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
yaml_edit: 2.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"

dev_dependencies:
collection: 1.18.0
Expand All @@ -107,4 +108,4 @@ dartdoc:
# Exclude this package from the hosted API docs.
nodoc: true

# PUBSPEC CHECKSUM: 3335
# PUBSPEC CHECKSUM: 3540