Improve performance of build APK (~50%) by running gen_snapshot concurrently #44534

jonahwilliams · 2019-11-10T03:20:23Z

Description

Support concurrent build via flutter assemble. Compared to current master, a release build of an APK for target platforms android-arm, android-arm64, and android-x64 takes ~60 seconds, down from ~140 seconds tested on my local macOS laptop.

Adds TargetPlatform.android enum. Longer-term, usage of TargetPlatform should be updated to remove arch specific flavors, these can be expressed with AndroidArch

Updated logs: https://gist.github.com/jonahwilliams/24aa5cf2544fd46e36a9b69a78d26ff4

packages/flutter_tools/lib/src/build_system/file_hash_store.dart

…_stuff

jonahwilliams · 2019-11-18T19:43:25Z

packages/flutter_tools/gradle/flutter.gradle

-                    extraFrontEndOptions extraFrontEndOptionsValue
-                    extraGenSnapshotOptions extraGenSnapshotOptionsValue
-                }
+            String taskName = toCammelCase(["compile", FLUTTER_BUILD_PREFIX, variant.name])


Instead of creating a task per ABI, I create a single task which builds all of the requested ABIs. This allows assemble to run concurrent gen_snapshots

zanderso · 2019-11-18T20:41:28Z

packages/flutter_tools/gradle/flutter.gradle

@@ -729,23 +733,15 @@ abstract class BaseFlutterTask extends DefaultTask {
            args "--depfile", "${intermediateDir}/flutter_build.d"
            args "--output", "${intermediateDir}"
            args "-dTargetFile=${targetPath}"
-            args "-dTargetPlatform=${targetPlatform}"
+            args "-dTargetPlatform=TargetPlatform.android"


If the target platform is extracted from the target name, then why is this needed?

KernelSnapshot requires a target platform per your fuchsia changes. We could loosen that restriction a bit by making a missing platform default to non-fuchsia

We'd also still need it if we wanted to do platform specific assets at some point

blasten · 2019-11-18T22:05:23Z

packages/flutter_tools/gradle/flutter.gradle

+        if (buildMode == "debug") {
+          ruleNames = ["debug_android_application"]
+        } else {
+          ruleNames = targetPlatformValues.collect({ targetPlatform ->


nit:

ruleNames = targetPlatformValues.collect { "android_aot_bundle_${buildMode}_$it" }

blasten · 2019-11-18T22:07:34Z

packages/flutter_tools/gradle/flutter.gradle

-                include "app.so"
+                targetPlatformValues.each {
+                  String abi =  PLATFORM_ARCH_MAP[targetArch]
+                  include "${abi}/app.so"


nit:

include "${PLATFORM_ARCH_MAP[targetArch]}/app.so"

blasten · 2019-11-18T22:10:17Z

packages/flutter_tools/lib/src/build_info.dart

@@ -306,6 +307,7 @@ String getNameForHostPlatform(HostPlatform platform) {
 }

 enum TargetPlatform {
+  android,


maybe add comment about what the difference is between android, android_arm and so on.

we should probably just have android right?

Added a comment on the usage of android vs android w/ arch. Refactoring this is similar to the iOS refactoring

blasten · 2019-11-18T22:12:55Z

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

+
+  @override
+  List<Source> get inputs => <Source>[
+    const Source.pattern('{FLUTTER_ROOT}/packages/flutter_tools/lib/src/build_system/targets/dart.dart'),


q: why is this one part of the inputs?

Sort of a work around to allow running from source to handle source file changes to the tool. It doesn't add much/any overhead to the build so it is okay to leave in.

zanderso · 2019-11-18T22:53:25Z

dev/devicelab/bin/tasks/module_test.dart

@@ -216,12 +216,12 @@ Future<void> main() async {
      }

      final String analyticsOutput = analyticsOutputFile.readAsStringSync();
-      if (!analyticsOutput.contains('cd24: android-arm64')
+      if (!analyticsOutput.contains('cd24: TargetPlatform.android')


Is the real target architecture communicated in some other way to analytics?

If not, we should figure out a way to retain that so we can understand our analytics across this change.

It should be part of the build apk/appbundle/aar commands:

https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/commands/build_apk.dart#L58

@blasten can you provide some additional context on why we send this second analytics call? Its for the add2app tracking, right?

I don't think this is critical, but cc'ing @xster to confirm.

ultranit: not having the enum 'outer class' name in there would be nice as a bonus.

https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/commands/build_apk.dart#L58 that you're linking to is orthogonal to this right? This is to detect top level calls to flutter build bundles from add-to-app scripts

the dashboard to monitor is https://dashboards.corp.google.com/_8211ecd6_e372_4f1c_b4f8_a1f4f0411822#xhaf6f3g0

blasten · 2019-11-18T23:01:44Z

The post update APK size is 43.5MB whereas it used to be 53.5MB. Do you know why it decreased in size?

jonahwilliams · 2019-11-18T23:32:34Z

I get 53.6MB building flutter gallery locally. Which app were you building?

jonahwilliams · 2019-11-19T00:12:28Z

Followed up offline: the logs were from a version that still had a bug in the app.so include logic. The latest commits build correctly

jonahwilliams · 2019-11-19T19:26:00Z

YOLO

dnfield · 2019-12-18T23:41:53Z

This introduced #46163

initial draft of collapsed gradle tasks

7f85131

fluttergithubbot added tool Affects the "flutter" command-line tool. See also t: labels. work in progress; do not review labels Nov 10, 2019

googlebot added the cla: yes label Nov 10, 2019

jonahwilliams commented Nov 10, 2019

View reviewed changes

packages/flutter_tools/lib/src/build_system/file_hash_store.dart Outdated Show resolved Hide resolved

Jonah Williams added 9 commits November 10, 2019 15:57

Update file_hash_store.dart

038d54e

Update flutter.gradle

6d01f16

Merge branch 'master' of github.com:flutter/flutter into doo_stuff

0161d77

fix ABI copy

ec60aca

Merge branch 'doo_stuff' of github.com:jonahwilliams/flutter into doo…

28bd67c

…_stuff

update framework depfile

58d23ca

fix expected analytics

5444a1d

fix share library test search path

6276a7e

fix analytics name

dfbde92

jonahwilliams changed the title ~~Collapse all Flutter compile tasks into a single instance~~ Improve performance of build APK by running gen_snapshot concurrently Nov 18, 2019

fluttergithubbot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Nov 18, 2019

jonahwilliams marked this pull request as ready for review November 18, 2019 19:41

jonahwilliams removed the work in progress; do not review label Nov 18, 2019

jonahwilliams commented Nov 18, 2019

View reviewed changes

jonahwilliams requested review from blasten and zanderso November 18, 2019 19:43

zanderso reviewed Nov 18, 2019

View reviewed changes

blasten reviewed Nov 18, 2019

View reviewed changes

jonahwilliams added 2 commits November 18, 2019 14:35

Merge branch 'master' of github.com:flutter/flutter into doo_stuff

9367e9a

address review comments

5b975dc

zanderso reviewed Nov 18, 2019

View reviewed changes

jonahwilliams changed the title ~~Improve performance of build APK by running gen_snapshot concurrently~~ Improve performance of build APK (~50%) by running gen_snapshot concurrently Nov 19, 2019

blasten approved these changes Nov 19, 2019

View reviewed changes

Jonah Williams added 3 commits November 18, 2019 23:47

Update flutter.gradle

ebca531

Merge branch 'master' into doo_stuff

020494d

Update module_test.dart

f96660b

jonahwilliams merged commit df3505c into flutter:master Nov 19, 2019

jonahwilliams deleted the doo_stuff branch November 19, 2019 19:26

jonahwilliams mentioned this pull request Dec 18, 2019

The tool builds the same kernel file three times #44187

Closed

dnfield mentioned this pull request Dec 18, 2019

cannot load assets file starting with "#" from the rootBundle after install #46163

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2021

Improve performance of build APK (~50%) by running gen_snapshot concurrently #44534

Improve performance of build APK (~50%) by running gen_snapshot concurrently #44534

Uh oh!

Conversation

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

Uh oh!

Description

Uh oh!

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!

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!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blasten commented Nov 18, 2019

Uh oh!

jonahwilliams commented Nov 18, 2019

Uh oh!

jonahwilliams commented Nov 19, 2019

Uh oh!

jonahwilliams commented Nov 19, 2019

Uh oh!

dnfield commented Dec 18, 2019

Uh oh!

Uh oh!

jonahwilliams commented Nov 10, 2019 •

edited

Loading