Skip to content

[flutter_tools] Fix analyze size on arm64 - #141317

Merged
auto-submit[bot] merged 2 commits into
flutter:masterfrom
chris-forks:fix-analyze-size-on-arm64
Jan 17, 2024
Merged

auto-submit[bot] merged 2 commits into
flutter:masterfrom
chris-forks:fix-analyze-size-on-arm64

Conversation

@christopherfujino

Copy link
Copy Markdown
Contributor

Fixes #140659

@github-actions github-actions Bot added tool Affects the "flutter" command-line tool. See also t: labels. a: desktop Running on desktop labels Jan 10, 2024
<string>1.0</string>
<key>CFBundleVersion</key>
<string>1.0</string>
<key>CFBundleDevelopmentRegion</key>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is an unrelated thing that annoyed me

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

Copy link
Copy Markdown
Contributor Author

friendly ping @andrewkolos

.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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

  1. This isn't a critical feature that's worth a bunch of extra logic.
  2. 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

.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) {

@andrewkolos andrewkolos Jan 16, 2024 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

@andrewkolos andrewkolos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@christopherfujino christopherfujino added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 17, 2024
@auto-submit
auto-submit Bot merged commit 4cd0a32 into flutter:master Jan 17, 2024
@christopherfujino
christopherfujino deleted the fix-analyze-size-on-arm64 branch January 17, 2024 01:25
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 17, 2024
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Jan 17, 2024
flutter/flutter@8e94423...def6af0

2024-01-17 [email protected] Roll Flutter Engine from 73a2de5da53f to c7e328518bc0 (5 revisions) (flutter/flutter#141673)
2024-01-17 [email protected] Manual roll Flutter Engine from 1382ff79dd6d to 73a2de5da53f (2 revisions) (flutter/flutter#141667)
2024-01-17 [email protected] Roll Flutter Engine from d4b6b7ec8e48 to 1382ff79dd6d (7 revisions) (flutter/flutter#141664)
2024-01-17 [email protected] 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 [email protected] [flutter_tools] Fix analyze size on arm64 (flutter/flutter#141317)
2024-01-17 [email protected] Roll Flutter Engine from d4b6b7ec8e48 to 021a5ff5eae6 (5 revisions) (flutter/flutter#141651)
2024-01-16 [email protected] Update TESTOWNERS iskakaushik -> dnfield (flutter/flutter#141649)
2024-01-16 [email protected] Allow selection in composing region (flutter/flutter#140516)
2024-01-16 [email protected] Roll Flutter Engine from eab7bd3b0999 to d4b6b7ec8e48 (1 revision) (flutter/flutter#141643)
2024-01-16 [email protected] Roll Flutter Engine from f20657354d8b to eab7bd3b0999 (12 revisions) (flutter/flutter#141638)
2024-01-16 [email protected] Fixed few typos (flutter/flutter#141543)
2024-01-16 [email protected] Add contexts to mac_ios targets. (flutter/flutter#141494)
2024-01-16 [email protected] handle rc versions of gradle in version compare  (flutter/flutter#141612)
2024-01-16 [email protected] Delete redundant `settings.ext.flutterSdkPath` (flutter/flutter#141509)
2024-01-16 [email protected] Roll Packages from d21f3b8 to 7dd0fcb (2 revisions) (flutter/flutter#141630)
2024-01-16 [email protected] Reference GitHub issue in TODO comment (flutter/flutter#141582)
2024-01-16 [email protected] migrate {min,target,compile}SdkVersion to {min,target,compile}Sdk (flutter/flutter#141537)
2024-01-16 [email protected] Sort Swift imports in templates (flutter/flutter#141487)
2024-01-16 [email protected] Ignore  or fix leaks. (flutter/flutter#141468)
2024-01-16 [email protected] Solve the problem that <Flutter/Flutter.h> cannot be imported when a pod transitive depends on Flutter (flutter/flutter#125610)
2024-01-16 [email protected] Fix #141061: Add 'color' property to `DrawerButton` and `EndDrawerButton` (flutter/flutter#141159)
2024-01-15 [email protected] 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 [email protected],[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

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

bvoq commented Jan 18, 2024

Copy link
Copy Markdown

Awesome, thanks everyone!

creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
flutter/flutter@8e94423...def6af0

2024-01-17 [email protected] Roll Flutter Engine from 73a2de5da53f to c7e328518bc0 (5 revisions) (flutter/flutter#141673)
2024-01-17 [email protected] Manual roll Flutter Engine from 1382ff79dd6d to 73a2de5da53f (2 revisions) (flutter/flutter#141667)
2024-01-17 [email protected] Roll Flutter Engine from d4b6b7ec8e48 to 1382ff79dd6d (7 revisions) (flutter/flutter#141664)
2024-01-17 [email protected] 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 [email protected] [flutter_tools] Fix analyze size on arm64 (flutter/flutter#141317)
2024-01-17 [email protected] Roll Flutter Engine from d4b6b7ec8e48 to 021a5ff5eae6 (5 revisions) (flutter/flutter#141651)
2024-01-16 [email protected] Update TESTOWNERS iskakaushik -> dnfield (flutter/flutter#141649)
2024-01-16 [email protected] Allow selection in composing region (flutter/flutter#140516)
2024-01-16 [email protected] Roll Flutter Engine from eab7bd3b0999 to d4b6b7ec8e48 (1 revision) (flutter/flutter#141643)
2024-01-16 [email protected] Roll Flutter Engine from f20657354d8b to eab7bd3b0999 (12 revisions) (flutter/flutter#141638)
2024-01-16 [email protected] Fixed few typos (flutter/flutter#141543)
2024-01-16 [email protected] Add contexts to mac_ios targets. (flutter/flutter#141494)
2024-01-16 [email protected] handle rc versions of gradle in version compare  (flutter/flutter#141612)
2024-01-16 [email protected] Delete redundant `settings.ext.flutterSdkPath` (flutter/flutter#141509)
2024-01-16 [email protected] Roll Packages from d21f3b8 to 7dd0fcb (2 revisions) (flutter/flutter#141630)
2024-01-16 [email protected] Reference GitHub issue in TODO comment (flutter/flutter#141582)
2024-01-16 [email protected] migrate {min,target,compile}SdkVersion to {min,target,compile}Sdk (flutter/flutter#141537)
2024-01-16 [email protected] Sort Swift imports in templates (flutter/flutter#141487)
2024-01-16 [email protected] Ignore  or fix leaks. (flutter/flutter#141468)
2024-01-16 [email protected] Solve the problem that <Flutter/Flutter.h> cannot be imported when a pod transitive depends on Flutter (flutter/flutter#125610)
2024-01-16 [email protected] Fix #141061: Add 'color' property to `DrawerButton` and `EndDrawerButton` (flutter/flutter#141159)
2024-01-15 [email protected] 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 [email protected],[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

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
bisor0627 pushed a commit to bisor0627/packages that referenced this pull request Jun 19, 2026
flutter/flutter@8e94423...def6af0

2024-01-17 [email protected] Roll Flutter Engine from 73a2de5da53f to c7e328518bc0 (5 revisions) (flutter/flutter#141673)
2024-01-17 [email protected] Manual roll Flutter Engine from 1382ff79dd6d to 73a2de5da53f (2 revisions) (flutter/flutter#141667)
2024-01-17 [email protected] Roll Flutter Engine from d4b6b7ec8e48 to 1382ff79dd6d (7 revisions) (flutter/flutter#141664)
2024-01-17 [email protected] 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 [email protected] [flutter_tools] Fix analyze size on arm64 (flutter/flutter#141317)
2024-01-17 [email protected] Roll Flutter Engine from d4b6b7ec8e48 to 021a5ff5eae6 (5 revisions) (flutter/flutter#141651)
2024-01-16 [email protected] Update TESTOWNERS iskakaushik -> dnfield (flutter/flutter#141649)
2024-01-16 [email protected] Allow selection in composing region (flutter/flutter#140516)
2024-01-16 [email protected] Roll Flutter Engine from eab7bd3b0999 to d4b6b7ec8e48 (1 revision) (flutter/flutter#141643)
2024-01-16 [email protected] Roll Flutter Engine from f20657354d8b to eab7bd3b0999 (12 revisions) (flutter/flutter#141638)
2024-01-16 [email protected] Fixed few typos (flutter/flutter#141543)
2024-01-16 [email protected] Add contexts to mac_ios targets. (flutter/flutter#141494)
2024-01-16 [email protected] handle rc versions of gradle in version compare  (flutter/flutter#141612)
2024-01-16 [email protected] Delete redundant `settings.ext.flutterSdkPath` (flutter/flutter#141509)
2024-01-16 [email protected] Roll Packages from cba0876 to 2fb103d (2 revisions) (flutter/flutter#141630)
2024-01-16 [email protected] Reference GitHub issue in TODO comment (flutter/flutter#141582)
2024-01-16 [email protected] migrate {min,target,compile}SdkVersion to {min,target,compile}Sdk (flutter/flutter#141537)
2024-01-16 [email protected] Sort Swift imports in templates (flutter/flutter#141487)
2024-01-16 [email protected] Ignore  or fix leaks. (flutter/flutter#141468)
2024-01-16 [email protected] Solve the problem that <Flutter/Flutter.h> cannot be imported when a pod transitive depends on Flutter (flutter/flutter#125610)
2024-01-16 [email protected] Fix #141061: Add 'color' property to `DrawerButton` and `EndDrawerButton` (flutter/flutter#141159)
2024-01-15 [email protected] Roll Packages from a49ee07 to cba0876 (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 [email protected],[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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flutter --analyze-build on Mac ARM only builds crashes with PathNotFoundException: Cannot open file

3 participants