Skip to content
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

Regression in flutter_gallery iOS release size #139605

Closed
zanderso opened this issue Dec 5, 2023 · 14 comments · Fixed by #140188
Closed

Regression in flutter_gallery iOS release size #139605

zanderso opened this issue Dec 5, 2023 · 14 comments · Fixed by #140188
Assignees
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) perf: app size Performance issues related to app size (binary/code size) or disk space team-ecosystem Owned by Ecosystem team

Comments

@zanderso
Copy link
Member

zanderso commented Dec 5, 2023

On the PR here #139203 from @Hixie that rolled some pub deps forward, SkiaPerf is reporting a ~13MB regression in the iOS release size of the flutter_gallery.

https://flutter-flutter-perf.skia.org/e/?begin=1699661770&end=1701816185&keys=X002a387933ee8947522cae13fbd696c3&requestType=0&selected=commit%3D38182%26name%3D%252Carch%253Dintel%252Cbranch%253Dmaster%252Cconfig%253Ddefault%252Cdevice_type%253DiPhone_11%252Cdevice_version%253Dnone%252Chost_type%253Dmac%252Csub_result%253Drelease_size_bytes%252Ctest%253Dflutter_gallery_ios__compile%252C&xbaroffset=38181

Is this intentional or a benign side-effect of rolling the dependencies forward?

@zanderso zanderso added c: performance Relates to speed or footprint issues (see "perf:" labels) perf: app size Performance issues related to app size (binary/code size) or disk space team-framework Owned by Framework team labels Dec 5, 2023
@Hixie
Copy link
Contributor

Hixie commented Dec 6, 2023

Potential sources:

  • shrine_images rolled from 2.0.1 to 2.0.2
  • video_player moved from 2.2.11 to 2.8.1, a significant change

nothing else looks suspicious.

@Hixie
Copy link
Contributor

Hixie commented Dec 6, 2023

shrine_images is unlikely to be the problem, the only change there was a trivial SDK version bump.

@Hixie
Copy link
Contributor

Hixie commented Dec 6, 2023

@stuartmorgan any idea if bumping video_player that much (2.2.11 to 2.8.1) could be the cause of a ~13MB regression in iOS release size on flutter_gallery?

@stuartmorgan
Copy link
Contributor

I'm not aware of any reason it would; that would certainly be surprising. video_player on iOS wraps the OS player, so doesn't involve any transitive dependencies, and we certainly didn't intentionally add 13 MB of code to the wrapper.

@jason-simmons
Copy link
Member

#139203 updated Gallery's version of the url_launcher_ios plugin.

The new version of url_launcher_ios includes Swift code, and that brought in 10MB of Swift runtime libraries.

@stuartmorgan
Copy link
Contributor

I assume that this is being measured pre-thinning? If so, I think we should just reset the baseline.

Is this our only size baseline? If so, we should seriously consider either replacing it, or at least adding a second one; the default project creation has been Swift/Kotlin for more than four years now, so the utility of a baseline that uses an Obj-C/Java template is questionable.

@zanderso zanderso added the fyi-ios For the attention of iOS platform team label Dec 11, 2023
@vashworth vashworth added the triaged-ios Triaged by iOS platform team label Dec 11, 2023
@flutter-triage-bot flutter-triage-bot bot removed fyi-ios For the attention of iOS platform team triaged-ios Triaged by iOS platform team labels Dec 11, 2023
@zanderso
Copy link
Member Author

The value comes from here:

https://github.com/flutter/flutter/blob/master/dev/devicelab/lib/tasks/perf_tests.dart#L1703

It's a straight up tar gz of the flutter build output directory, it looks like?

@goderbauer goderbauer added team-ecosystem Owned by Ecosystem team and removed team-framework Owned by Framework team labels Dec 13, 2023
@jmagman jmagman self-assigned this Dec 14, 2023
@jmagman
Copy link
Member

jmagman commented Dec 14, 2023

Making the minimum iOS version 12.2 instead of 11.0 prevents the Swift runtime from being pulled in. https://developer.apple.com/documentation/xcode-release-notes/swift-5-release-notes-for-xcode-10_2#App-Thinning

@jmagman
Copy link
Member

jmagman commented Dec 15, 2023

I've talked to @zanderso and the compressed app size isn't an accurate proxy for downloaded app size since the App Store encrypts and then recompresses the app.

When your app is approved for the App Store, it’s encrypted with Digital Rights Management (DRM) and recompressed. App Store file size for your binary may be larger than it is for the binary you uploaded in App Store Connect. The exact final size for your app can't be determined in advance.

https://developer.apple.com/help/app-store-connect/manage-builds/view-builds-and-metadata#view-the-file-sizes-of-a-build

One thing at a time though. #140188 will set the minimum deployment target for iOS compile perf tests to 12.2 which should fix some of the size regression related to the Swift runtime.

@jmagman
Copy link
Member

jmagman commented Dec 15, 2023

I've talked to @zanderso and the compressed app size isn't an accurate proxy for downloaded app size since the App Store encrypts and then recompresses the app.

This comment suggests another reason measuring compressed app size is important is to validate that changes in Dart snapshot format and data layout do not change compression size.

// Validate changes in Dart snapshot format and data layout do not
// change compression size. This also simulates the size of an IPA on iOS.

But then I looked at where we decided that matters and it was past-me and I can't remember why:
https://github.com/flutter/flutter/pull/109891/files#r954405126

@jmagman
Copy link
Member

jmagman commented Dec 15, 2023

It's a straight up tar gz of the flutter build output directory, it looks like?

No, it's a tar gz of the app, not the whole output directory.

String? _findDarwinAppInBuildDirectory(String searchDirectory) {
for (final FileSystemEntity entity in Directory(searchDirectory)
.listSync(recursive: true)) {
if (entity.path.endsWith('.app')) {
return entity.path;
}
}
return null;

final String? appPath =
_findDarwinAppInBuildDirectory(buildDirectory.path);
if (appPath == null) {
throw 'Failed to find app bundle in ${buildDirectory.path}';
}
// Validate changes in Dart snapshot format and data layout do not
// change compression size. This also simulates the size of an IPA on iOS.
await exec('tar', <String>['-zcf', 'build/app.tar.gz', appPath]);

@zanderso
Copy link
Member Author

@jmagman Thanks for investigating!

auto-submit bot pushed a commit that referenced this issue Dec 15, 2023
…140188)

ObjC->Swift plugin migration caused a size regression in the gallery app because the Swift runtime was also pulled in.  

The gallery app minimum target version is iOS 11.0, which predates Swift ABI compatibility.  Pre iOS 12.2 apps embedded the Swift runtime since there wasn't one available to use in the OS.  

Add  `FLUTTER_XCODE_IPHONEOS_DEPLOYMENT_TARGET` to the compile perf test environment, which gets translated by the tool to an Xcode build setting:
```
[2023-12-14 15:52:14.797318] [STDOUT] stdout:                IPHONEOS_DEPLOYMENT_TARGET = 12.2
```

On my machine on main
```
    "release_size_bytes": 43717389,
```
becomes
```
    "release_size_bytes": 40679432,
```

Fixes #139605
@jmagman
Copy link
Member

jmagman commented Dec 15, 2023

Screenshot 2023-12-15 at 12 34 49 PM

Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) perf: app size Performance issues related to app size (binary/code size) or disk space team-ecosystem Owned by Ecosystem team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants