New issue
Advanced search Search tips

Issue 810890 link

Starred by 2 users
Status: Fixed
Owner:
agrieve@chromium.org
Closed: Jun 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Java changes take too long to compile

Project Member Reported by agrieve@chromium.org, Feb 9 2018 
The time it takes between java edit and final apk feels like it has regressed a lot in the past year.

I took a trace when making a base_java edit that didn't touch any public apis via https://github.com/nico/ninjatracing and have attached it here.

Things that stood out to me:
* 14s for gen/chrome/browser/android/chrome_jni_registration.h
* gen/media/base/android/media_java.javac.jar was listed as a job. It shouldn't need to re-run at all. 
* All bytecode-rewritten.jar files recreated. These should be skipped when interface jar doesn't change
* Apk finalization took >20s. Should look to see if this was from signing, and use cached signatures if it's not already leveraging them.

Comment 1 by agrieve@chromium.org, Feb 9 2018 

Actually attaching trace.
base-java-change.json
387 KB View Download

Comment 2 by agrieve@chromium.org, Feb 9 2018

Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 20 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca62ba02f857e4d453a6e5201c3970ff757d5698

commit ca62ba02f857e4d453a6e5201c3970ff757d5698
Author: Andrew Grieve <agrieve@chromium.org>
Date: Tue Feb 20 22:20:59 2018

Android: Use apksigner instead of jarsigner.

Brings the finalize_apk step 19 seconds -> 4 seconds on
my machine (for ChromePublic.apk).

Also enables v2 signing of apks, which makes them install
faster on N+ devices.

Bug:  810890 
Change-Id: I9fd52ed26c56472bd2894ace6f0516fc386143d6
Reviewed-on: https://chromium-review.googlesource.com/920651
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537908}
[modify] https://crrev.com/ca62ba02f857e4d453a6e5201c3970ff757d5698/build/android/gyp/finalize_apk.py
[modify] https://crrev.com/ca62ba02f857e4d453a6e5201c3970ff757d5698/build/config/android/config.gni
[modify] https://crrev.com/ca62ba02f857e4d453a6e5201c3970ff757d5698/build/config/android/internal_rules.gni

Comment 4 by agrieve@chromium.org, Feb 21 2018 

Here's another trace, showing that:
1) android_lint takes longer than java compilation
2) Targets are waiting until lint is done for their deps rather than running in parallel.
base-java-change.json
109 KB View Download
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 21 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f4424029c0d3f43b64dcf5a04488e224229c5ec0

commit f4424029c0d3f43b64dcf5a04488e224229c5ec0
Author: agrieve <agrieve@chromium.org>
Date: Wed Feb 21 16:45:43 2018

Revert "Android: Use apksigner instead of jarsigner."

This reverts commit ca62ba02f857e4d453a6e5201c3970ff757d5698.

Reason for revert: Broke apkmerger.py

Original change's description:
> Android: Use apksigner instead of jarsigner.
> 
> Brings the finalize_apk step 19 seconds -> 4 seconds on
> my machine (for ChromePublic.apk).
> 
> Also enables v2 signing of apks, which makes them install
> faster on N+ devices.
> 
> Bug:  810890 
> Change-Id: I9fd52ed26c56472bd2894ace6f0516fc386143d6
> Reviewed-on: https://chromium-review.googlesource.com/920651
> Reviewed-by: John Budorick <jbudorick@chromium.org>
> Commit-Queue: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#537908}

TBR=agrieve@chromium.org,tikuta@chromium.org,jbudorick@chromium.org

Change-Id: Iac2ea335ba0f7698437ce96a14c50a83d696304f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  810890 , 814350
Reviewed-on: https://chromium-review.googlesource.com/928942
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538133}
[modify] https://crrev.com/f4424029c0d3f43b64dcf5a04488e224229c5ec0/build/android/gyp/finalize_apk.py
[modify] https://crrev.com/f4424029c0d3f43b64dcf5a04488e224229c5ec0/build/config/android/config.gni
[modify] https://crrev.com/f4424029c0d3f43b64dcf5a04488e224229c5ec0/build/config/android/internal_rules.gni
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 22 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bef0ce0b1f29658f9c86d89e6a1fbb33069d5754

commit bef0ce0b1f29658f9c86d89e6a1fbb33069d5754
Author: Andrew Grieve <agrieve@chromium.org>
Date: Thu Feb 22 14:51:45 2018

Reland: Android: Use apksigner instead of jarsigner.

Reverted in: f4424029c0d3f43b64dcf5a04488e224229c5ec0.

Reason for reland:
 * Fixed apkmerger.py

Brings the finalize_apk step 19 seconds -> 4 seconds on
my machine (for ChromePublic.apk).

Also enables v2 signing of apks, which makes them install
faster on N+ devices.

Bug:  810890 , 814350
Change-Id: I8bb010c2da59ace1450da79985e5f2a1111a9330
Reviewed-on: https://chromium-review.googlesource.com/929101
Reviewed-by: Tao Bai <michaelbai@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538433}
[modify] https://crrev.com/bef0ce0b1f29658f9c86d89e6a1fbb33069d5754/android_webview/tools/apk_merger.py
[modify] https://crrev.com/bef0ce0b1f29658f9c86d89e6a1fbb33069d5754/build/android/gyp/apkbuilder.py
[modify] https://crrev.com/bef0ce0b1f29658f9c86d89e6a1fbb33069d5754/build/android/gyp/finalize_apk.py
[delete] https://crrev.com/4a88d90aa73c3c8a2cbbd00e52646e6d44d22520/build/android/gyp/finalize_splits.py
[modify] https://crrev.com/bef0ce0b1f29658f9c86d89e6a1fbb33069d5754/build/config/android/config.gni
[modify] https://crrev.com/bef0ce0b1f29658f9c86d89e6a1fbb33069d5754/build/config/android/internal_rules.gni
Project Member

Comment 7 by bugdroid1@chromium.org, May 16 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a178ca27a2c2cddab78a30b311307ee6e3927f2

commit 6a178ca27a2c2cddab78a30b311307ee6e3927f2
Author: Andrew Grieve <agrieve@chromium.org>
Date: Wed May 16 15:55:07 2018

Andriod: Fix invalid depfile causing apks to always appear dirty

Bug:  810890 
Change-Id: I5230aaf33ea5d90e61a4a20e77a08b46d457a275
Reviewed-on: https://chromium-review.googlesource.com/1059950
Reviewed-by: David Turner <digit@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559124}
[modify] https://crrev.com/6a178ca27a2c2cddab78a30b311307ee6e3927f2/build/android/gyp/merge_jar_info_files.py
Project Member

Comment 8 by bugdroid1@chromium.org, May 23 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cdbe4f702ca908fcfe34f73206887664ef3d9781

commit cdbe4f702ca908fcfe34f73206887664ef3d9781
Author: Andrew Grieve <agrieve@chromium.org>
Date: Wed May 23 18:37:21 2018

jni_registration_generator.py: Make it faster and improve line wrapping

This script depends on every single .java file, and so runs when any
.java file changes. For some incremental compiles, it is the longest step.

On my machine, running the script for chrome_public_apk's sources goes from
13 seconds -> 1 second.

~10 seconds was going to parsing each file. I made this faster by
sharding it with multiprocessing.Pool.

WrapOutput() was also adding 1-2 seconds. I made this faster by moving it
into the per-file shards and pre-creating TextWrapper instances.

This change also increases line wrap width from 80 -> 100 because it
makes most of the poorly wrapped lines at the end better. E.g.:

if
    (!RegisterNative_org_chromium_content_browser_framehost_RenderFrameHostImpl(env))
  return false;

The result of running with "time" on my machine:

real	0m0.961s
user	0m23.312s
sys	0m0.288s

Bug:  810890 
Change-Id: I8f090af2a0312caf9796dccc434c6d12f44909dc
Reviewed-on: https://chromium-review.googlesource.com/1065512
Reviewed-by: Richard Coles <torne@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561172}
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/SampleForTests_jni.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/jni_generator.py
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/jni_generator_tests.py
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/jni_registration_generator.py
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testCalledByNatives.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testConstantsFromJavaP.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testFromJavaP.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testFromJavaPGenerics.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testInnerClassNatives.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testInnerClassNativesBothInnerAndOuter.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testInnerClassNativesBothInnerAndOuterRegistrations.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testInnerClassNativesMultiple.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testMultipleJNIAdditionalImport.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testNativeExportsOnlyOption.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testNatives.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testNativesLong.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testNativesRegistrations.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testSingleJNIAdditionalImport.golden
[modify] https://crrev.com/cdbe4f702ca908fcfe34f73206887664ef3d9781/base/android/jni_generator/testTracing.golden
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 6 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1d29c5f58575e804a8657f33ccbf9f450b39251c

commit 1d29c5f58575e804a8657f33ccbf9f450b39251c
Author: Andrew Grieve <agrieve@chromium.org>
Date: Wed Jun 06 14:34:37 2018

Refactor Java build rules to greatly reduce overbuild

This makes use of ninja's "restat" feature to eliminate rebuilding of
dependent java libraries when a library's interface jar is unchanged.

We previously approximated this behavior by having scripts use md5sum.py
to early-return when input md5s didn't change.

Bug:  810890 
Change-Id: I08456c172f52e581c84b6a0daf9376e38d7b3df3
Reviewed-on: https://chromium-review.googlesource.com/1060697
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564877}
[modify] https://crrev.com/1d29c5f58575e804a8657f33ccbf9f450b39251c/build/android/BUILD.gn
[modify] https://crrev.com/1d29c5f58575e804a8657f33ccbf9f450b39251c/build/android/gyp/bytecode_processor.py
[modify] https://crrev.com/1d29c5f58575e804a8657f33ccbf9f450b39251c/build/android/gyp/dex.py
[add] https://crrev.com/1d29c5f58575e804a8657f33ccbf9f450b39251c/build/android/gyp/ijar.py
[modify] https://crrev.com/1d29c5f58575e804a8657f33ccbf9f450b39251c/build/android/gyp/javac.py
[modify] https://crrev.com/1d29c5f58575e804a8657f33ccbf9f450b39251c/build/android/gyp/util/build_utils.py
[modify] https://crrev.com/1d29c5f58575e804a8657f33ccbf9f450b39251c/build/config/android/internal_rules.gni
[modify] https://crrev.com/1d29c5f58575e804a8657f33ccbf9f450b39251c/third_party/ijar/README.chromium
[delete] https://crrev.com/56f4b7fe5a8498abca1217b306a591db12382f5f/third_party/ijar/ijar.gni

Comment 10 by agrieve@chromium.org, Jun 25 2018

Status: Fixed (was: Assigned)
Fixed all of the bullet points. Last remaining issue is that android_lint is slow. Filed a separate bug for that:
https://bugs.chromium.org/p/chromium/issues/detail?id=856189
►

Sign in to add a comment