-
Notifications
You must be signed in to change notification settings - Fork 29.8k
[flutter_tools] opt iOS/macOS apps out of Metal API validation via migrator, update templates in repo. #159228
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
[flutter_tools] opt iOS/macOS apps out of Metal API validation via migrator, update templates in repo. #159228
Conversation
|
As a follow up, I need to opt all of our benchmarks out of this setting... |
jmagman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
| @@ -0,0 +1,106 @@ | |||
|
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| import 'package:test/fake.dart'; | ||
|
|
||
| import '../../src/common.dart'; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'debugServiceExtension = "internal"' | ||
| '\n enableGPUValidationMode = "1"')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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">
'''
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh the white space is definitely messed up, since the indentation varies. I'm wagering that most folks never actually open this file...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a real programmer and regex'd my way to a solution.
|
Yeah I can migrate in this PR, wasn't sure if it was common to do it separately or not. |
|
is there a way to trigger the migrators that skips building? |
| RemoveBitcodeMigration(app.project, globals.logger), | ||
| XcodeThinBinaryBuildPhaseInputPathsMigration(app.project, globals.logger), | ||
| UIApplicationMainDeprecationMigration(app.project, globals.logger), | ||
| MetalAPIValidationMigrator(app.project, globals.logger), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only migrates iOS, you'll also want to add it here for macOS:
| SwiftPackageManagerGitignoreMigration(flutterProject, globals.logger), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
oh, maybe --config-only |
Not a nice way, though we should. You can get a lot of it by commenting out the Android part of test.dart: Lines 383 to 393 in 17e38a4
add '--config-only' here Line 451 in 17e38a4
and here Line 481 in 17e38a4
then Then I grep to see if there's anything unmigrated left and run |
|
|
||
| @override | ||
| String? migrateLine(String line) { | ||
| return line; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't need to override this.
| @override | |
| String? migrateLine(String line) { | |
| return line; | |
| } |
jmagman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
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)
…n via migrator, update templates in repo. (flutter/flutter#159228)
…n via migrator, update templates in repo. (flutter/flutter#159228)
…n via migrator, update templates in repo. (flutter/flutter#159228)
…n via migrator, update templates in repo. (flutter/flutter#159228)
…n via migrator, update templates in repo. (flutter/flutter#159228)
…n via migrator, update templates in repo. (flutter/flutter#159228)
…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 bkonyi@google.com Added additional logging to `_listCoreDevices` (flutter/flutter#159275) 2024-11-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 78b87f3fe023 to d1a08064e193 (1 revision) (flutter/flutter#159280) 2024-11-21 jason-simmons@users.noreply.github.com Shut down DevTools and DDS processes if flutter_tools is killed by a signal (flutter/flutter#159238) 2024-11-21 matanlurey@users.noreply.github.com Remove `RepaintBoundary` that is no longer needed. (flutter/flutter#159232) 2024-11-21 matanlurey@users.noreply.github.com Try a speculative fix for Gradle OOMs. (flutter/flutter#159234) 2024-11-21 21270878+elliette@users.noreply.github.com On-device Widget Inspector button exits widget selection (flutter/flutter#158219) 2024-11-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 523d381893c8 to 78b87f3fe023 (2 revisions) (flutter/flutter#159270) 2024-11-21 jessiewong401@gmail.com Remove `firebase_abstract_method_smoke_test` (flutter/flutter#159145) 2024-11-21 engine-flutter-autoroll@skia.org Roll Packages from e95f6d8 to 913b99e (7 revisions) (flutter/flutter#159268) 2024-11-21 engine-flutter-autoroll@skia.org Roll Flutter Engine from 69c325513a65 to 523d381893c8 (3 revisions) (flutter/flutter#159263) 2024-11-21 jonahwilliams@google.com [flutter_tools] opt iOS/macOS apps out of Metal API validation via migrator, update templates in repo. (flutter/flutter#159228) 2024-11-21 jmccandless@google.com Scribe Android handwriting text input (flutter/flutter#148784) 2024-11-21 matanlurey@users.noreply.github.com Terminate non-detached test devices on `flutter run` completion (flutter/flutter#159170) 2024-11-21 matanlurey@users.noreply.github.com Un-skip tests that use `flutter build apk`. (flutter/flutter#159231) 2024-11-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2d32cf3a7971 to 69c325513a65 (1 revision) (flutter/flutter#159229) 2024-11-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3828681d1f86 to 2d32cf3a7971 (3 revisions) (flutter/flutter#159226) 2024-11-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3245c8976976 to 3828681d1f86 (1 revision) (flutter/flutter#159217) 2024-11-20 matanlurey@users.noreply.github.com Add `--dry-run` to `dev/bots/test.dart`. (flutter/flutter#158956) 2024-11-20 34871572+gmackall@users.noreply.github.com Add docs for setting up Android Studio to auto format Kotlin code (flutter/flutter#159209) 2024-11-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 80d77505fdde to 3245c8976976 (1 revision) (flutter/flutter#159214) 2024-11-20 stanleycocoa@gmail.com Fix: The enableFeedback property of InkWell cannot be set to a nullab� (flutter/flutter#158907) 2024-11-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3f19207e820e to 80d77505fdde (1 revision) (flutter/flutter#159210) 2024-11-20 engine-flutter-autoroll@skia.org Roll Packages from fc4adc7 to e95f6d8 (6 revisions) (flutter/flutter#159201) 2024-11-20 kustermann@google.com Remove dependency on [Target] and instead operate on [Architecture] (flutter/flutter#159196) 2024-11-20 63278936+hongeSunCoder@users.noreply.github.com Fix git command in Quality-Assurance.md (flutter/flutter#155146) 2024-11-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7eb87547cbc6 to 3f19207e820e (4 revisions) (flutter/flutter#159176) 2024-11-20 matanlurey@users.noreply.github.com Make `runner` non-nullable as it always is. (flutter/flutter#159156) 2024-11-19 tessertaha@gmail.com Update Material 3 `CircularProgressIndicator` for new visual style (flutter/flutter#158104) 2024-11-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4ff696b555dc to 7eb87547cbc6 (3 revisions) (flutter/flutter#159168) 2024-11-19 magder@google.com Add platform-android label for all flutter_tools *android* files (flutter/flutter#159166) 2024-11-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from d5820a638885 to 4ff696b555dc (1 revision) (flutter/flutter#159164) 2024-11-19 bernaferrari2@gmail.com Reland Add UI Benchmarks (flutter/flutter#153368) 2024-11-19 mohellebiabdessalem@gmail.com fix lint usage of `task` inside `resolve_dependecies.gradle` file (flutter/flutter#158022) 2024-11-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from cff1e751f853 to d5820a638885 (5 revisions) (flutter/flutter#159155) 2024-11-19 mohellebiabdessalem@gmail.com Removing redundant backticks in `flutter\packages\flutter_tools\gradle\gradle.kts` (flutter/flutter#159051) 2024-11-19 50643541+Mairramer@users.noreply.github.com Fixes initial validation with AutovalidateMode.always on first build (flutter/flutter#156708) 2024-11-19 tessertaha@gmail.com 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 louisehsu@google.com,stuartmorgan@google.com 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 ...
|
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? |
|
The raster thread is going so much faster we're probably pushing ~50% to ~30% more frames |
|
Here's another regression with garbage collection time, also almost surely related to running more frames, but noting it here |
…n via migrator, update templates in repo. (flutter/flutter#159228)
…n via migrator, update templates in repo. (flutter/flutter#159228)
…n via migrator, update templates in repo. (flutter/flutter#159228)
…n via migrator, update templates in repo. (flutter/flutter#159228)
…n via migrator, update templates in repo. (flutter/flutter#159228)
Setting "enableGPUValidationMode = "1"" disables Metal API validation. I know, I know.
Fixes #159227