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

[Bazel] Improve Mobile-Install Incremental Manifest Generating by applying multi-thread #12085

Conversation

ThomasCJY
Copy link
Contributor

Background
While using mobile-Install, we noticed that it constantly takes more to run on incremental build. Take our app for example, the incremental build metrics for a single line kotlin code change looks like this:

Command Time
bazel build + adb install 63s
mobile-install 91s

After digging into it, I found that the bottleneck is the "Incremental Manifest Generating" action which takes a lot of time (35+ sec) for multidex build. The time is spent on the checksum calculation for all dex files. The SHA256 checksum for each dex file takes around 1-2 sec. Currently in our app we have 10 dex shards and each dex zip contains 2-3 dex files, processing them sequentially takes more than 30 seconds.

Change
In this PR, I added multithread support for this script so that the checksum calculation can be done concurrently and it improved the "Incremental Manifest Generating" to be done in 6 second (80%+ improvement).

Result & Test
After applying this change, the total incremental build time has been reduced to 43 seconds, with a 30s+ improvement from Incremental Manifest Generating step.
Before:

After:

You can also easily verify this from command line:

jchen tmp % time python ../build_incremental_dexmanifest.py ../output/outmanifest.txt shard1.dex.zip  shard10.dex.zip shard2.dex.zip  shard3.dex.zip  shard4.dex.zip  shard5.dex.zip  shard6.dex.zip  shard7.dex.zip  shard8.dex.zip  shard9.dex.zip
python ../build_incremental_dexmanifest.py ../output/outmanifest.txt           0.70s user 0.72s system 31% cpu 4.583 total
jchen tmp % time python ../build_incremental_dexmanifest_before.py ../output/outmanifest.txt shard1.dex.zip  shard10.dex.zip shard2.dex.zip  shard3.dex.zip  shard4.dex.zip  shard5.dex.zip  shard6.dex.zip  shard7.dex.zip  shard8.dex.zip  shard9.dex.zip
python ../build_incremental_dexmanifest_before.py ../output/outmanifest.txt    0.65s user 0.64s system 3% cpu 37.908 total

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@ThomasCJY
Copy link
Contributor Author

@googlebot I consent

@ThomasCJY ThomasCJY force-pushed the jchen-upstream-fixIncrementalManifestGeneration branch from b0910fb to 7da6509 Compare September 10, 2020 22:21
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@gregestren gregestren added the team-Android Issues for Android team label Sep 18, 2020
@ThomasCJY
Copy link
Contributor Author

@ahumesky mind taking a look when available?

@laurentlb
Copy link
Contributor

ping @ahumesky?

@ThomasCJY
Copy link
Contributor Author

ping @ahumesky again can you take a look?

@ThomasCJY
Copy link
Contributor Author

Seems like @ahumesky is not available? @laurentlb @gregestren is there anyone else who can help review this PR?

@gregestren
Copy link
Contributor

I'll poke around to see...

@ahumesky
Copy link
Contributor

ahumesky commented Nov 3, 2020

Thank you for the thorough analysis, and sorry for the delay here!
I'll import this now, should be able to have it merged tomorrow or so.

@bazel-io bazel-io closed this in 1049fe8 Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-Android Issues for Android team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants