Issue metadata
Sign in to add a comment
|
A 132.5 KiB regression in resource_sizes (MonochromePublic.apk) at 582871:582871 |
||||||||||||||||||||
Issue descriptionCaused 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.
,
Aug 20
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}
,
Aug 20
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??
,
Aug 20
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.
,
Aug 20
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.
,
Aug 20
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 |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Aug 20