[flutter_tools] opt iOS/macOS apps out of Metal API validation via migrator, update templates in repo. by jonahwilliams · Pull Request #159228 · flutter/flutter

jonahwilliams · 2024-11-20T22:26:04Z

Setting "enableGPUValidationMode = "1"" disables Metal API validation. I know, I know.

… default.

jonahwilliams · 2024-11-20T23:11:16Z

As a follow up, I need to opt all of our benchmarks out of this setting...

jmagman

Usually when we do a migration we also build (and therefore automigrate) all the example and integration test projects to prove out that it's working.

Example: #140256

jmagman · 2024-11-20T23:14:55Z

packages/flutter_tools/test/general.shard/ios/metal_api_validator_test.dart

@@ -0,0 +1,106 @@
+


Suggested change

jmagman · 2024-11-20T23:15:01Z

packages/flutter_tools/test/general.shard/ios/metal_api_validator_test.dart

+import 'package:test/fake.dart';
+
+import '../../src/common.dart';
+


Suggested change

jmagman · 2024-11-20T23:17:49Z

packages/flutter_tools/test/general.shard/ios/metal_api_validator_test.dart

+      'debugServiceExtension = "internal"'
+    '\n    enableGPUValidationMode = "1"'));


Can you check the entire XML file to make sure other whitespace doesn't get messed up.

''' <?xml version="1.0" encoding="UTF-8"?> <LaunchAction buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" launchStyle = "0" useCustomWorkingDirectory = "NO" ignoresPersistentStateOnLaunch = "NO" debugDocumentVersioning = "YES" debugServiceExtension = "internal" enableGPUValidationMode = "1" allowLocationSimulation = "YES"> '''

oh the white space is definitely messed up, since the indentation varies. I'm wagering that most folks never actually open this file...

Ideally given the normal generated template scheme that's never otherwise been updated, the migration adds the right whitespace. Otherwise Xcode will try to format it if the whitespace is off the first time it opens, which will dirty the working copy again.

I'm a real programmer and regex'd my way to a solution.

jonahwilliams · 2024-11-20T23:25:56Z

Yeah I can migrate in this PR, wasn't sure if it was common to do it separately or not.

jonahwilliams · 2024-11-20T23:30:03Z

is there a way to trigger the migrators that skips building?

jmagman · 2024-11-20T23:34:50Z

packages/flutter_tools/lib/src/ios/mac.dart

    RemoveBitcodeMigration(app.project, globals.logger),
    XcodeThinBinaryBuildPhaseInputPathsMigration(app.project, globals.logger),
    UIApplicationMainDeprecationMigration(app.project, globals.logger),
+    MetalAPIValidationMigrator(app.project, globals.logger),


This only migrates iOS, you'll also want to add it here for macOS:

flutter/packages/flutter_tools/lib/src/macos/build_macos.dart

Line 105 in 364e4be

SwiftPackageManagerGitignoreMigration(flutterProject, globals.logger),

jonahwilliams · 2024-11-20T23:39:53Z

oh, maybe --config-only

jmagman · 2024-11-20T23:40:01Z

is there a way to trigger the migrators that skips building?

Not a nice way, though we should. You can get a lot of it by commenting out the Android part of test.dart:

flutter/dev/bots/test.dart

Lines 383 to 393 in 17e38a4

    
           final bool verifyCaching = exampleDirectory.path.contains('flutter_gallery'); 
        
           final String examplePath = path.relative(exampleDirectory.path, from: Directory.current.path); 
        
           final List<String> additionalArgs = <String>[ 
        
             if (mainFile != null) path.relative(mainFile.path, from: exampleDirectory.absolute.path), 
        
           ]; 
        
           if (Directory(path.join(examplePath, 'android')).existsSync()) { 
        
             await _flutterBuildApk(examplePath, release: false, additionalArgs: additionalArgs, verifyCaching: verifyCaching); 
        
             await _flutterBuildApk(examplePath, release: true, additionalArgs: additionalArgs, verifyCaching: verifyCaching); 
        
           } else { 
        
             print('Example project ${path.basename(examplePath)} has no android directory, skipping apk'); 
        
           }

add '--config-only' here

flutter/dev/bots/test.dart

Line 451 in 17e38a4

additionalArgs: <String>[...additionalArgs, '--no-codesign'],

and here

flutter/dev/bots/test.dart

Line 481 in 17e38a4

additionalArgs: additionalArgs

then

$ cd dev/bots
$ flutter pub get
$ SHARD=build_tests dart test.dart

Then I grep to see if there's anything unmigrated left and run build --config-only manually in those directories.

jmagman · 2024-11-21T00:43:27Z

packages/flutter_tools/lib/src/ios/migrations/metal_api_validation_migration.dart

+
+  @override
+  String? migrateLine(String line) {
+    return line;
+  }


You shouldn't need to override this.

Suggested change

@override

String? migrateLine(String line) {

return line;

}

jmagman

LGTM

auto-submit · 2024-11-21T01:35:07Z

auto label is removed for flutter/flutter/159228, Failed to enqueue flutter/flutter/159228 with HTTP 400: GraphQL mutate failed.

…n via migrator, update templates in repo. (flutter/flutter#159228)

Roll Flutter from 8536b96 to 93d772c (37 revisions) flutter/flutter@8536b96...93d772c 2024-11-21 [email protected] Added additional logging to `_listCoreDevices` (flutter/flutter#159275) 2024-11-21 [email protected] Roll Flutter Engine from 78b87f3fe023 to d1a08064e193 (1 revision) (flutter/flutter#159280) 2024-11-21 [email protected] Shut down DevTools and DDS processes if flutter_tools is killed by a signal (flutter/flutter#159238) 2024-11-21 [email protected] Remove `RepaintBoundary` that is no longer needed. (flutter/flutter#159232) 2024-11-21 [email protected] Try a speculative fix for Gradle OOMs. (flutter/flutter#159234) 2024-11-21 [email protected] On-device Widget Inspector button exits widget selection (flutter/flutter#158219) 2024-11-21 [email protected] Roll Flutter Engine from 523d381893c8 to 78b87f3fe023 (2 revisions) (flutter/flutter#159270) 2024-11-21 [email protected] Remove `firebase_abstract_method_smoke_test` (flutter/flutter#159145) 2024-11-21 [email protected] Roll Packages from e95f6d8 to 913b99e (7 revisions) (flutter/flutter#159268) 2024-11-21 [email protected] Roll Flutter Engine from 69c325513a65 to 523d381893c8 (3 revisions) (flutter/flutter#159263) 2024-11-21 [email protected] [flutter_tools] opt iOS/macOS apps out of Metal API validation via migrator, update templates in repo. (flutter/flutter#159228) 2024-11-21 [email protected] Scribe Android handwriting text input (flutter/flutter#148784) 2024-11-21 [email protected] Terminate non-detached test devices on `flutter run` completion (flutter/flutter#159170) 2024-11-21 [email protected] Un-skip tests that use `flutter build apk`. (flutter/flutter#159231) 2024-11-20 [email protected] Roll Flutter Engine from 2d32cf3a7971 to 69c325513a65 (1 revision) (flutter/flutter#159229) 2024-11-20 [email protected] Roll Flutter Engine from 3828681d1f86 to 2d32cf3a7971 (3 revisions) (flutter/flutter#159226) 2024-11-20 [email protected] Roll Flutter Engine from 3245c8976976 to 3828681d1f86 (1 revision) (flutter/flutter#159217) 2024-11-20 [email protected] Add `--dry-run` to `dev/bots/test.dart`. (flutter/flutter#158956) 2024-11-20 [email protected] Add docs for setting up Android Studio to auto format Kotlin code (flutter/flutter#159209) 2024-11-20 [email protected] Roll Flutter Engine from 80d77505fdde to 3245c8976976 (1 revision) (flutter/flutter#159214) 2024-11-20 [email protected] Fix: The enableFeedback property of InkWell cannot be set to a nullabâ�¦ (flutter/flutter#158907) 2024-11-20 [email protected] Roll Flutter Engine from 3f19207e820e to 80d77505fdde (1 revision) (flutter/flutter#159210) 2024-11-20 [email protected] Roll Packages from fc4adc7 to e95f6d8 (6 revisions) (flutter/flutter#159201) 2024-11-20 [email protected] Remove dependency on [Target] and instead operate on [Architecture] (flutter/flutter#159196) 2024-11-20 [email protected] Fix git command in Quality-Assurance.md (flutter/flutter#155146) 2024-11-20 [email protected] Roll Flutter Engine from 7eb87547cbc6 to 3f19207e820e (4 revisions) (flutter/flutter#159176) 2024-11-20 [email protected] Make `runner` non-nullable as it always is. (flutter/flutter#159156) 2024-11-19 [email protected] Update Material 3 `CircularProgressIndicator` for new visual style (flutter/flutter#158104) 2024-11-19 [email protected] Roll Flutter Engine from 4ff696b555dc to 7eb87547cbc6 (3 revisions) (flutter/flutter#159168) 2024-11-19 [email protected] Add platform-android label for all flutter_tools *android* files (flutter/flutter#159166) 2024-11-19 [email protected] Roll Flutter Engine from d5820a638885 to 4ff696b555dc (1 revision) (flutter/flutter#159164) 2024-11-19 [email protected] Reland Add UI Benchmarks (flutter/flutter#153368) 2024-11-19 [email protected] fix lint usage of `task` inside `resolve_dependecies.gradle` file (flutter/flutter#158022) 2024-11-19 [email protected] Roll Flutter Engine from cff1e751f853 to d5820a638885 (5 revisions) (flutter/flutter#159155) 2024-11-19 [email protected] Removing redundant backticks in `flutter\packages\flutter_tools\gradle\gradle.kts` (flutter/flutter#159051) 2024-11-19 [email protected] Fixes initial validation with AutovalidateMode.always on first build (flutter/flutter#156708) 2024-11-19 [email protected] Introduce new Material 3 `Slider` shapes (flutter/flutter#152237) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose ...

gaaclarke · 2024-11-22T18:06:29Z

This did cause an increase in cpu usage for some benchmarks: https://flutter-flutter-perf.skia.org/e/?begin=1732136514&end=1732211697&keys=X689df53096362e272afcf2aaadfdb3ab&selected=commit%3D43461%26name%3D%252Carch%253Dintel%252Cbranch%253Dmaster%252Cconfig%253Ddefault%252Cdevice_type%253DiPhone_11%252Cdevice_version%253Dnone%252Chost_type%253Dmac%252Csub_result%253Daverage_cpu_usage%252Ctest%253Dbackdrop_filter_perf_ios__timeline_summary%252C&xbaroffset=43461

I'm guessing the cpu can just run hotter since it isn't waiting on some sort of gpu synchronization for the validator?

jonahwilliams · 2024-11-22T18:12:09Z

https://flutter-flutter-perf.skia.org/e/?queries=sub_result%3D90th_percentile_frame_rasterizer_time_millis%26sub_result%3Daverage_frame_rasterizer_time_millis%26test%3Dbackdrop_filter_perf_ios__timeline_summary&selected=commit%3D43461%26name%3D%252Carch%253Dintel%252Cbranch%253Dmaster%252Cconfig%253Ddefault%252Cdevice_type%253DiPhone_11%252Cdevice_version%253Dnone%252Chost_type%253Dmac%252Csub_result%253Daverage_frame_rasterizer_time_millis%252Ctest%253Dbackdrop_filter_perf_ios__timeline_summary%252C&xbaroffset=43461

The raster thread is going so much faster we're probably pushing ~50% to ~30% more frames

gaaclarke · 2024-11-26T16:48:30Z

Here's another regression with garbage collection time, also almost surely related to running more frames, but noting it here

https://flutter-flutter-perf.skia.org/e/?begin=1732048325&end=1732302020&keys=Xb641568324e71c153cd5b5197c7aa8d0&selected=commit%3D43461%26name%3D%252Carch%253Dintel%252Cbranch%253Dmaster%252Cconfig%253Ddefault%252Cdevice_type%253DiPhone_11%252Cdevice_version%253Dnone%252Chost_type%253Dmac%252Csub_result%253Dtotal_ui_gc_time%252Ctest%253Ddynamic_path_tessellation_perf_ios__timeline_summary%252C&xbaroffset=43459

…n via migrator, update templates in repo. (flutter/flutter#159228)

[flutter_tools] opt new iOS/macOS apps out of Metal API validation by…

b6f1b2d

… default.

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Nov 20, 2024

add unit testing and migrator.

7a5d4ae

github-actions bot added the platform-ios iOS applications specifically label Nov 20, 2024

dont rerun over and over again.

8b94c70

jonahwilliams marked this pull request as ready for review November 20, 2024 23:08

jonahwilliams requested review from andrewkolos and jmagman November 20, 2024 23:08

jmagman reviewed Nov 20, 2024

View reviewed changes

jonahwilliams added 2 commits November 20, 2024 15:40

wire up macOS tool.

1c69a8a

preserve indentation.

8b08067

github-actions bot added the a: desktop Running on desktop label Nov 21, 2024

migrate it.

a87bec2

github-actions bot added the d: examples Sample code and demos label Nov 21, 2024

how did this get there.

923deb2

jmagman reviewed Nov 21, 2024

View reviewed changes

jmagman approved these changes Nov 21, 2024

View reviewed changes

remove extra override.

6aec3ad

jonahwilliams changed the title ~~[flutter_tools] opt new iOS/macOS apps out of Metal API validation.~~ [flutter_tools] opt iOS/macOS apps out of Metal API validation via migrator, update templates in repo. Nov 21, 2024

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 21, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 21, 2024

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 21, 2024

auto-submit bot added this pull request to the merge queue Nov 21, 2024

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 21, 2024

jonahwilliams deleted the opt-Out-of-shader-validation branch November 21, 2024 01:39

engine-flutter-autoroll mentioned this pull request Nov 21, 2024

Roll Flutter from 8536b96ebb3e to 1578d7d5f34e (28 revisions) flutter/packages#8139

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024

f7869a4 [flutter_tools] opt iOS/macOS apps out of Metal API validatio…

6f5530f

…n via migrator, update templates in repo. (flutter/flutter#159228)

engine-flutter-autoroll mentioned this pull request Nov 21, 2024

Roll Flutter from 8536b96ebb3e to 7662e7eae548 (29 revisions) flutter/packages#8140

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024

f7869a4 [flutter_tools] opt iOS/macOS apps out of Metal API validatio…

646cb4b

…n via migrator, update templates in repo. (flutter/flutter#159228)

engine-flutter-autoroll mentioned this pull request Nov 21, 2024

Roll Flutter from 8536b96ebb3e to 81d2bf5d7934 (30 revisions) flutter/packages#8141

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024

f7869a4 [flutter_tools] opt iOS/macOS apps out of Metal API validatio…

1327b14

…n via migrator, update templates in repo. (flutter/flutter#159228)

engine-flutter-autoroll mentioned this pull request Nov 21, 2024

Roll Flutter from 8536b96ebb3e to 4e03220a8691 (31 revisions) flutter/packages#8143

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024

f7869a4 [flutter_tools] opt iOS/macOS apps out of Metal API validatio…

a8c56b0

…n via migrator, update templates in repo. (flutter/flutter#159228)

engine-flutter-autoroll mentioned this pull request Nov 21, 2024

Roll Flutter from 8536b96ebb3e to a051be858576 (32 revisions) flutter/packages#8144

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024

f7869a4 [flutter_tools] opt iOS/macOS apps out of Metal API validatio…

b9fa4e8

…n via migrator, update templates in repo. (flutter/flutter#159228)

engine-flutter-autoroll mentioned this pull request Nov 21, 2024

Roll Flutter from 8536b96ebb3e to d1e2c5fba748 (33 revisions) flutter/packages#8145

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024

f7869a4 [flutter_tools] opt iOS/macOS apps out of Metal API validatio…

c99f633

…n via migrator, update templates in repo. (flutter/flutter#159228)

engine-flutter-autoroll mentioned this pull request Nov 21, 2024

Roll Flutter from 8536b96ebb3e to 399403ae6a38 (36 revisions) flutter/packages#8146

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024

f7869a4 [flutter_tools] opt iOS/macOS apps out of Metal API validatio…

9721505

…n via migrator, update templates in repo. (flutter/flutter#159228)

engine-flutter-autoroll mentioned this pull request Nov 21, 2024

Roll Flutter from 8536b96ebb3e to 93d772c5cdd8 (37 revisions) flutter/packages#8147

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 21, 2024

f7869a4 [flutter_tools] opt iOS/macOS apps out of Metal API validatio…

12954ad

…n via migrator, update templates in repo. (flutter/flutter#159228)

gaaclarke mentioned this pull request Nov 27, 2024

2x regression in total_ui_gc_time for backdrop_filter_perf_ios__timeline_summary #159337

Closed

reidbaker mentioned this pull request Dec 13, 2024

Cutout rotation integration test #160255

Closed

11 tasks

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025

f7869a4 [flutter_tools] opt iOS/macOS apps out of Metal API validatio…

1bf149f

…n via migrator, update templates in repo. (flutter/flutter#159228)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

f7869a4 [flutter_tools] opt iOS/macOS apps out of Metal API validatio…

ce5c811

…n via migrator, update templates in repo. (flutter/flutter#159228)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

f7869a4 [flutter_tools] opt iOS/macOS apps out of Metal API validatio…

4e17694

…n via migrator, update templates in repo. (flutter/flutter#159228)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

f7869a4 [flutter_tools] opt iOS/macOS apps out of Metal API validatio…

519ca4f

…n via migrator, update templates in repo. (flutter/flutter#159228)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025

f7869a4 [flutter_tools] opt iOS/macOS apps out of Metal API validatio…

ed5c1e6

…n via migrator, update templates in repo. (flutter/flutter#159228)

		import 'package:test/fake.dart';

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

		'debugServiceExtension = "internal"'
		'\n enableGPUValidationMode = "1"'));

Conversation

jonahwilliams commented Nov 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonahwilliams commented Nov 20, 2024

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Nov 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Nov 20, 2024

Uh oh!

jonahwilliams commented Nov 20, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Nov 20, 2024

Uh oh!

jmagman commented Nov 20, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Nov 21, 2024

Uh oh!

gaaclarke commented Nov 22, 2024

Uh oh!

jonahwilliams commented Nov 22, 2024

Uh oh!

gaaclarke commented Nov 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jonahwilliams commented Nov 20, 2024 •

edited

Loading

jonahwilliams Nov 20, 2024 •

edited

Loading