New issue
Advanced search Search tips

Issue 875880 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Aug 20
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

A 132.5 KiB regression in resource_sizes (MonochromePublic.apk) at 582871:582871

Project Member Reported by huangs@google.com, Aug 20

Issue description

Caused by “Revert "[build:android] Add a module for AR in Monochrome"”

Commit: 99c7a9db66196aac59a737e8a7a7af83adfac24b

Link to size graph: https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICQ48fgswoM

Debugging size regressions is documented at: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/apk_size_regressions.md#Debugging-Apk-Size-Increase

Based on the graph: 20kb of native code, 8kb of png

It looks like this increase was probably unexpected or might be avoidable (this is a CL does revert, so something is really odd).

Please have a look and either:

1. Close as “Won't Fix” with a short justification, or
2. Land a revert / fix-up.

 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=875880

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=af0544fb88913f7d2158195feeebf96f6262c739aa315866441daa558f7ced12


Bot(s) for this bug's original alert(s):

Android Builder Perf
Assigning to alexilin@chromium.org because this is the only CL in range:
Revert "[build:android] Add a module for AR in Monochrome"

This reverts commit bc740b07a52e8a89612b75454b17a4ad614b1e97.

Reason for revert: this commit causes compile errors on several internal.client.clank builders, particularly on "monochrome_bundle" target. Failed build: https://ci.chromium.org/buildbot/internal.client.clank/arm-builder-rel/20238

Original change's description:
> [build:android] Add a module for AR in Monochrome
> 
> Add an AR Dynamic Feature Module (DFM) and bundle it into the public
> Monochrome bundle.
> 
> + Add loadable modules to the build config so that their paths can be
>   passed to the module create target.
> 
> + Add option to bundle targets to specify the Android SDK target for
>   synchronized proguarding. This was necessary because the Monochrome
>   base module uses a different Android SDK than the AR module, which
>   made proguard sad. Also pass the Android SDK Jar as a dedicated
>   classpath Jar to proguard and don't mix it with the other classpath
>   Jars.
> 
> Bug:  863063 
> Change-Id: I024d05dd99136c069e510995657ac7236f6b6e5e
> Reviewed-on: https://chromium-review.googlesource.com/1165533
> Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Reviewed-by: Ian Vollick <vollick@chromium.org>
> Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#582765}

TBR=vollick@chromium.org,yfriedman@chromium.org,tedchoc@chromium.org,agrieve@chromium.org,tiborg@chromium.org

Change-Id: I87ab63c3153018390f4018527d760bbd10cdcd16
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  863063 
Reviewed-on: https://chromium-review.googlesource.com/1174271
Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582871}
Oops, for comment #1, we should have:

Based on the graph: 132.5 KiB of native lib.

To see details please run:
tools/binary_size/diagnose_bloat.py --cloud 99c7a9db66196aac59a737e8a7a7af83adfac24

Output:

******************************Native Diff******************************
Common Metadata:
    apk_file_name=apks/MonochromePublic.apk
    elf_arch=arm
    elf_file_name=lib.unstripped/libmonochrome.so
    gn_args=ffmpeg_branding="Chrome" goma_dir="/b/build/slave/goma_cache/client" is_chrome_branded=true is_debug=false is_official_build=true proprietary_codecs=true strip_absolute_paths_from_debug_symbols=true symbol_level=1 target_os="android" use_goma=true
    linker_name=lld_v1
    map_file_name=lib.unstripped/libmonochrome.so.map.gz
    tool_prefix=third_party/llvm-build/Release+Asserts/bin/llvm-
Old Metadata:
    apk_size=69102835
    elf_build_id=54183f9d1847cf645e5afb1f8bc1eb287b29d64a
    elf_mtime=2018-08-14 05:16:53
    git_revision=b0198b937f700cc219121ae1e0afc2eb187d0810
New Metadata:
    apk_size=69242272
    elf_build_id=4b8f593cc0b289a36b9ca07cc2f1faac034adfa8
    elf_mtime=2018-08-14 05:33:52
    git_revision=99c7a9db66196aac59a737e8a7a7af83adfac24b

Section Sizes (Total=136kb (139437 bytes)):
    .bss: 0 bytes (0 bytes) (not included in totals)
    .data: 0 bytes (0 bytes) (0.0%)
    .data.rel.ro: 0 bytes (0 bytes) (0.0%)
    .dex: 0 bytes (0 bytes) (0.0%)
    .other: 136kb (139437 bytes) (100.0%)
    .pak.nontranslated: 0 bytes (0 bytes) (0.0%)
    .pak.translations: 0 bytes (0 bytes) (0.0%)
    .rel.dyn: 0 bytes (0 bytes) (0.0%)
    .rodata: 0 bytes (0 bytes) (0.0%)
    .text: 0 bytes (0 bytes) (0.0%)

1 symbols added (+), 3 changed (~), 0 removed (-), 1011141 unchanged (not shown)
Of changed symbols, 4 grew, 0 shrank
Number of unique symbols 628216 -> 628217 (+1)
1 paths added, 0 removed, 3 changed

Showing 4 symbols (3 -> 4 unique) with total pss: 139437 bytes
Histogram of symbols based on PSS:
    [32,64): 2   [2048,4096): 1   [131072,262144): 1
.text=0 bytes    .rodata=0 bytes    .data.rel.ro=0 bytes    .data=0 bytes    .bss=0 bytes    .dex=0 bytes    .dex.method=0 bytes    .pak.translations=0 bytes    .pak.nontranslated=0 bytes    .other=136kb      total=136kb
Number of unique paths: 1

Section Legend: t=.text, r=.rodata, R=.data.rel.ro, d=.data, b=.bss, x=.dex, m=.dex.method, p=.pak.translations, P=.pak.nontranslated, o=.other
Index | Running Total | Section@Address | Δ PSS (Δ size_without_padding) | Path
------------------------------------------------------------
+ 0)     135728 (97.3%) o@0x0        135728 (0->135728) third_party/arcore-android-sdk
               lib/armeabi-v7a/libarcore_sdk_c_minimal.so
~ 1)     139352 (99.9%) o@0x0        3624 (0->0)        {no path}
               Overhead: APK file
~ 2)     139397 (100.0%) o@0x0        45 (79682->79727)  $APK/META-INF/MANIFEST.MF
               META-INF/MANIFEST.MF
~ 3)     139437 (100.0%) o@0x0        40 (81602->81642)  $APK/META-INF/CHROMIUM.SF
               META-INF/CHROMIUM.SF

******************************Resource Sizes Diff******************************
For an explanation of these metrics, see:
https://chromium.googlesource.com/chromium/src/+/master/docs/speed/binary_size/metrics.md#Metrics-for-Android

Specifics:
      +139,437 bytes normalized apk size
            +1 zip entries file count
      +135,728 bytes other lib size
InstallSize:
      +139,437 bytes APK size
    +139,437.0 bytes Estimated installed size
InstallBreakdown (+135,813 bytes):
      +135,728 bytes Native code size
           +85 bytes Package metadata size
StaticInitializersCount:
            +1 count count

========
So it looks like lib/armeabi-v7a/libarcore_sdk_c_minimal.so somehow appears, and is lingering after the change??

Cc: -tiborg@chromium.org
Owner: tiborg@chromium.org
There is no data about the resource_sizes before the initial commit, unfortunately. The CL was relanded but it didn't decrease the value.

Reassigning to tiborg@ since this was just a sheriff revert from my side.
Status: WontFix (was: Assigned)
The initial submission of my change (bc740b07a52e8a89612b75454b17a4ad614b1e97) caused a binary size improvement because it erroneously removed some native libraries. alexilin@'s revert added them back and my fixed reland didn't remove them again. So, no net change in binary size.
Ah okay. Before I filed the bug I looked at the graph and didn't see a big dip corresponding to your CL...

Sign in to add a comment