[flutter_tools] Fix analyze size on arm64 #141317

christopherfujino · 2024-01-10T22:47:03Z

christopherfujino · 2024-01-10T22:47:26Z

packages/flutter_tools/lib/src/build_system/targets/macos.dart

-	<string>1.0</string>
-	<key>CFBundleVersion</key>
-	<string>1.0</string>
+  <key>CFBundleDevelopmentRegion</key>


This is an unrelated thing that annoyed me

christopherfujino · 2024-01-11T21:49:04Z

friendly ping @andrewkolos

andrewkolos · 2024-01-16T23:44:34Z

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

-    .childFile('trace.$arch.json');
+  final File? aotSnapshot = DarwinArch.values.map<File?>((DarwinArch arch) {
+    return globals.fs.directory(buildInfo.codeSizeDirectory).childFile('snapshot.${arch.name}.json');
+    // Pick the first if there are multiple for simplicity


This could makes the code size output inaccurate[^1] depending on what binary the end user gets, right? I'm guessing us just picking the first one we find is good enough because

This isn't a critical feature that's worth a bunch of extra logic.

Most users of this (I recklessly assume) are only interested in significant changes in code size (e.g. some new dependency pulls in 5MB of code when compiled). Because of this, outputting additional information per architecture is noisy at best and confusing at worst.

[^1] Maybe "misleading by omission" is more accurate.

Right, so although we might have generated multiple aot snapshots (if Xcode was configured to build with multiple architectures), I think most users are only concerned about the relative binary size cost of the different dart libraries in their snapshot. And even if the arm64 and x64 snapshots were significantly different in size, they would be essentially comparing optimizations in dart compiler backends.

There may be a future reason why we users want/need access to every analysis json that we generated, but that would require further refactoring of tool code.

andrewkolos · 2024-01-16T23:47:10Z

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

-    .childFile('snapshot.$arch.json');
-  final File precompilerTrace = globals.fs.directory(buildInfo.codeSizeDirectory)
-    .childFile('trace.$arch.json');
+  final File? aotSnapshot = DarwinArch.values.map<File?>((DarwinArch arch) {


Theoretically which architecture we use here is dependent on the order in which the enum values are declared, which could change. However, when considering the tiny likelihood of this happening and the tiny impact it would probably have on users, this seems fine.

right, this is also assuming they're comparing size-analysis snapshots across flutter/dart sdk upgrades.

andrewkolos

LGTM

…141317)

flutter/flutter@8e94423...def6af0 2024-01-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 73a2de5da53f to c7e328518bc0 (5 revisions) (flutter/flutter#141673) 2024-01-17 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from 1382ff79dd6d to 73a2de5da53f (2 revisions) (flutter/flutter#141667) 2024-01-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from d4b6b7ec8e48 to 1382ff79dd6d (7 revisions) (flutter/flutter#141664) 2024-01-17 sokolovskyi.konstantin@gmail.com TrainHoppingAnimation should dispatch creation and disposal events. (flutter/flutter#141635) 2024-01-17 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from d4b6b7ec8e48 to 021a5ff5eae6 (5 revisions)" (flutter/flutter#141659) 2024-01-17 christopherfujino@gmail.com [flutter_tools] Fix analyze size on arm64 (flutter/flutter#141317) 2024-01-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from d4b6b7ec8e48 to 021a5ff5eae6 (5 revisions) (flutter/flutter#141651) 2024-01-16 dnfield@google.com Update TESTOWNERS iskakaushik -> dnfield (flutter/flutter#141649) 2024-01-16 31859944+LongCatIsLooong@users.noreply.github.com Allow selection in composing region (flutter/flutter#140516) 2024-01-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from eab7bd3b0999 to d4b6b7ec8e48 (1 revision) (flutter/flutter#141643) 2024-01-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from f20657354d8b to eab7bd3b0999 (12 revisions) (flutter/flutter#141638) 2024-01-16 anis.alibegic@gmail.com Fixed few typos (flutter/flutter#141543) 2024-01-16 godofredoc@google.com Add contexts to mac_ios targets. (flutter/flutter#141494) 2024-01-16 reidbaker@google.com handle rc versions of gradle in version compare (flutter/flutter#141612) 2024-01-16 barpac02@gmail.com Delete redundant `settings.ext.flutterSdkPath` (flutter/flutter#141509) 2024-01-16 engine-flutter-autoroll@skia.org Roll Packages from d21f3b8 to 7dd0fcb (2 revisions) (flutter/flutter#141630) 2024-01-16 barpac02@gmail.com Reference GitHub issue in TODO comment (flutter/flutter#141582) 2024-01-16 barpac02@gmail.com migrate {min,target,compile}SdkVersion to {min,target,compile}Sdk (flutter/flutter#141537) 2024-01-16 magder@google.com Sort Swift imports in templates (flutter/flutter#141487) 2024-01-16 polinach@google.com Ignore or fix leaks. (flutter/flutter#141468) 2024-01-16 intspt@qq.com Solve the problem that <Flutter/Flutter.h> cannot be imported when a pod transitive depends on Flutter (flutter/flutter#125610) 2024-01-16 93368594+imp-sike@users.noreply.github.com Fix #141061: Add 'color' property to `DrawerButton` and `EndDrawerButton` (flutter/flutter#141159) 2024-01-15 engine-flutter-autoroll@skia.org Roll Packages from d74d687 to d21f3b8 (5 revisions) (flutter/flutter#141573) 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 dit@google.com,rmistry@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 To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

bvoq · 2024-01-18T11:17:17Z

Awesome, thanks everyone!

…r#5911) flutter/flutter@8e94423...def6af0 2024-01-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 73a2de5da53f to c7e328518bc0 (5 revisions) (flutter/flutter#141673) 2024-01-17 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from 1382ff79dd6d to 73a2de5da53f (2 revisions) (flutter/flutter#141667) 2024-01-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from d4b6b7ec8e48 to 1382ff79dd6d (7 revisions) (flutter/flutter#141664) 2024-01-17 sokolovskyi.konstantin@gmail.com TrainHoppingAnimation should dispatch creation and disposal events. (flutter/flutter#141635) 2024-01-17 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from d4b6b7ec8e48 to 021a5ff5eae6 (5 revisions)" (flutter/flutter#141659) 2024-01-17 christopherfujino@gmail.com [flutter_tools] Fix analyze size on arm64 (flutter/flutter#141317) 2024-01-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from d4b6b7ec8e48 to 021a5ff5eae6 (5 revisions) (flutter/flutter#141651) 2024-01-16 dnfield@google.com Update TESTOWNERS iskakaushik -> dnfield (flutter/flutter#141649) 2024-01-16 31859944+LongCatIsLooong@users.noreply.github.com Allow selection in composing region (flutter/flutter#140516) 2024-01-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from eab7bd3b0999 to d4b6b7ec8e48 (1 revision) (flutter/flutter#141643) 2024-01-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from f20657354d8b to eab7bd3b0999 (12 revisions) (flutter/flutter#141638) 2024-01-16 anis.alibegic@gmail.com Fixed few typos (flutter/flutter#141543) 2024-01-16 godofredoc@google.com Add contexts to mac_ios targets. (flutter/flutter#141494) 2024-01-16 reidbaker@google.com handle rc versions of gradle in version compare (flutter/flutter#141612) 2024-01-16 barpac02@gmail.com Delete redundant `settings.ext.flutterSdkPath` (flutter/flutter#141509) 2024-01-16 engine-flutter-autoroll@skia.org Roll Packages from d21f3b8 to 7dd0fcb (2 revisions) (flutter/flutter#141630) 2024-01-16 barpac02@gmail.com Reference GitHub issue in TODO comment (flutter/flutter#141582) 2024-01-16 barpac02@gmail.com migrate {min,target,compile}SdkVersion to {min,target,compile}Sdk (flutter/flutter#141537) 2024-01-16 magder@google.com Sort Swift imports in templates (flutter/flutter#141487) 2024-01-16 polinach@google.com Ignore or fix leaks. (flutter/flutter#141468) 2024-01-16 intspt@qq.com Solve the problem that <Flutter/Flutter.h> cannot be imported when a pod transitive depends on Flutter (flutter/flutter#125610) 2024-01-16 93368594+imp-sike@users.noreply.github.com Fix #141061: Add 'color' property to `DrawerButton` and `EndDrawerButton` (flutter/flutter#141159) 2024-01-15 engine-flutter-autoroll@skia.org Roll Packages from d74d687 to d21f3b8 (5 revisions) (flutter/flutter#141573) 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 dit@google.com,rmistry@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 To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

christopherfujino added 2 commits January 10, 2024 13:44

make existing test fail

e0079c5

fix test and add another

b71b1ce

christopherfujino requested a review from andrewkolos January 10, 2024 22:47

github-actions bot added tool Affects the "flutter" command-line tool. See also t: labels. a: desktop Running on desktop labels Jan 10, 2024

christopherfujino commented Jan 10, 2024

View reviewed changes

gspencergoog requested a review from cbracken January 11, 2024 19:16

andrewkolos reviewed Jan 16, 2024

View reviewed changes

andrewkolos approved these changes Jan 16, 2024

View reviewed changes

christopherfujino added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 17, 2024

auto-submit bot merged commit 4cd0a32 into flutter:master Jan 17, 2024
123 checks passed

christopherfujino deleted the fix-analyze-size-on-arm64 branch January 17, 2024 01:25

engine-flutter-autoroll mentioned this pull request Jan 17, 2024

Roll Flutter from 8e94423e6a92 to def6af0b8e6e (23 revisions) flutter/packages#5911

Merged

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

4cd0a3252 [flutter_tools] Fix analyze size on arm64 (flutter/flutter#…

1c14b16

…141317)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[flutter_tools] Fix analyze size on arm64 #141317

[flutter_tools] Fix analyze size on arm64 #141317

christopherfujino commented Jan 10, 2024

christopherfujino Jan 10, 2024

christopherfujino commented Jan 11, 2024

andrewkolos Jan 16, 2024

christopherfujino Jan 17, 2024

andrewkolos Jan 16, 2024 •

edited

Loading

christopherfujino Jan 17, 2024

andrewkolos left a comment

bvoq commented Jan 18, 2024

[flutter_tools] Fix analyze size on arm64 #141317

[flutter_tools] Fix analyze size on arm64 #141317

Conversation

christopherfujino commented Jan 10, 2024

christopherfujino Jan 10, 2024

Choose a reason for hiding this comment

christopherfujino commented Jan 11, 2024

andrewkolos Jan 16, 2024

Choose a reason for hiding this comment

christopherfujino Jan 17, 2024

Choose a reason for hiding this comment

andrewkolos Jan 16, 2024 • edited Loading

Choose a reason for hiding this comment

christopherfujino Jan 17, 2024

Choose a reason for hiding this comment

andrewkolos left a comment

Choose a reason for hiding this comment

bvoq commented Jan 18, 2024

andrewkolos Jan 16, 2024 •

edited

Loading