Issue metadata
Sign in to add a comment
|
GVR Responsible for about 50% of Static Initializers |
||||||||||||||||||||||
Issue descriptionThis was buried within a large number of commits, so took a while to pinpoint: https://chromeperf.appspot.com/report?sid=a097e74b1aa288511afb4cb616efe0f95ba4d347ad61d5e835072f23450938ba&num_points=10&rev=498445 Running: tools/binary_size/diagnose_bloat.py 58bfbe7f9d99998e97157e89fc4be6c0bdfc60ba Shows: StaticInitializersCount +3 count count MonochromePublic.apk_Breakdown (+12,276 bytes) -2,655 bytes Zip Overhead +12,448 bytes Native code size -172 bytes PNG drawables size +17 bytes Package metadata size +2,646 bytes Java code size -8 bytes Compiled Android resources size MonochromePublic.apk_Specifics +30,132.8 bytes normalized apk size +12,448 bytes main lib size +4,420 bytes main dex size Note also that this added 12k to .so and 4k to dex, more than was thought by analysis in code review. Policy is to revert the CL that added static initializers and reland it with the fix. You can verify locally that no static initializers are added by running the following from within an Android checkout: tools/binary_size/diagnose_bloat.py HEAD -v Common fixes include: Add constexpr, Use LazyInstance<>, Use a getter to return a local static variable.
,
Aug 31 2017
../../build/android/resource_sizes.py --dump-static-initializers apks/ChromePublic.apk --chromium-output-directory=out/Release (non-vr entries omitted). # arm_model.cc _GLOBAL__sub_I_arm_model.cc+0x36 # arm_model.cc _GLOBAL__sub_I_arm_model.cc+0x48 # arm_model.cc _GLOBAL__sub_I_arm_model.cc+0x70 # arm_model.cc _GLOBAL__sub_I_arm_model.cc+0x74 # arm_model.cc _GLOBAL__sub_I_arm_model.cc+0x78 # arm_model.cc ion::math::VectorBase<3, float>::VectorBase() # arm_model.cc std::__ndk1::pair<std::__ndk1::__map_iterator<std::__ndk1::__tree_iterator<std::__ndk1::__value_type<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >, std::__ndk1::__tree_node<std::__ndk1::__value_type<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >, void*>*, int> >, bool> std::__ndk1::map<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> >, std::__ndk1::less<unsigned long long>, std::__ndk1::allocator<std::__ndk1::pair<unsigned long long const, std::__ndk1::vector<int, std::__ndk1::allocator<int> > > > >::emplace<unsigned long long&, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >(unsigned long long&, std::__ndk1::vector<int, std::__ndk1::allocator<int> >&&)+0x134 # arm_model.cc std::__ndk1::pair<std::__ndk1::__map_iterator<std::__ndk1::__tree_iterator<std::__ndk1::__value_type<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >, std::__ndk1::__tree_node<std::__ndk1::__value_type<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >, void*>*, int> >, bool> std::__ndk1::map<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> >, std::__ndk1::less<unsigned long long>, std::__ndk1::allocator<std::__ndk1::pair<unsigned long long const, std::__ndk1::vector<int, std::__ndk1::allocator<int> > > > >::emplace<unsigned long long&, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >(unsigned long long&, std::__ndk1::vector<int, std::__ndk1::allocator<int> >&&)+0x144 # controller_api_impl.cc _GLOBAL__sub_I_controller_api_impl.cc+0x10 # controller_api_impl.cc _GLOBAL__sub_I_controller_api_impl.cc+0xc # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x104 # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x108 # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x10c # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x110 # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x114 # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x118 # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x11c # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x120 # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x124 # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x128 # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x12c # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x130 # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x134 # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x138 # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x13c # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0x140 # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0xa2 # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0xde # gltf_asset.cc _GLOBAL__sub_I_gltf_asset.cc+0xec # gltf_asset.cc WTF::HashTable<blink::LayoutObject const*, WTF::KeyValuePair<blink::LayoutObject const*, std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > >, WTF::KeyValuePairKeyExtractor, WTF::PtrHash<blink::LayoutObject const>, WTF::HashMapValueTraits<WTF::HashTraits<blink::LayoutObject const*>, WTF::HashTraits<std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > > >, WTF::HashTraits<blink::LayoutObject const*>, WTF::PartitionAllocator>::erase(WTF::HashTableIterator<blink::LayoutObject const*, WTF::KeyValuePair<blink::LayoutObject const*, std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > >, WTF::KeyValuePairKeyExtractor, WTF::PtrHash<blink::LayoutObject const>, WTF::HashMapValueTraits<WTF::HashTraits<blink::LayoutObject const*>, WTF::HashTraits<std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > > >, WTF::HashTraits<blink::LayoutObject const*>, WTF::PartitionAllocator>)+0x104 # gltf_asset.cc WTF::HashTable<blink::LayoutObject const*, WTF::KeyValuePair<blink::LayoutObject const*, std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > >, WTF::KeyValuePairKeyExtractor, WTF::PtrHash<blink::LayoutObject const>, WTF::HashMapValueTraits<WTF::HashTraits<blink::LayoutObject const*>, WTF::HashTraits<std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > > >, WTF::HashTraits<blink::LayoutObject const*>, WTF::PartitionAllocator>::erase(WTF::HashTableIterator<blink::LayoutObject const*, WTF::KeyValuePair<blink::LayoutObject const*, std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > >, WTF::KeyValuePairKeyExtractor, WTF::PtrHash<blink::LayoutObject const>, WTF::HashMapValueTraits<WTF::HashTraits<blink::LayoutObject const*>, WTF::HashTraits<std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > > >, WTF::HashTraits<blink::LayoutObject const*>, WTF::PartitionAllocator>)+0x4e9c # gltf_asset.cc WTF::HashTable<blink::LayoutObject const*, WTF::KeyValuePair<blink::LayoutObject const*, std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > >, WTF::KeyValuePairKeyExtractor, WTF::PtrHash<blink::LayoutObject const>, WTF::HashMapValueTraits<WTF::HashTraits<blink::LayoutObject const*>, WTF::HashTraits<std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > > >, WTF::HashTraits<blink::LayoutObject const*>, WTF::PartitionAllocator>::erase(WTF::HashTableIterator<blink::LayoutObject const*, WTF::KeyValuePair<blink::LayoutObject const*, std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > >, WTF::KeyValuePairKeyExtractor, WTF::PtrHash<blink::LayoutObject const>, WTF::HashMapValueTraits<WTF::HashTraits<blink::LayoutObject const*>, WTF::HashTraits<std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > > >, WTF::HashTraits<blink::LayoutObject const*>, WTF::PartitionAllocator>)+0x4eac # gltf_asset.cc WTF::HashTable<blink::LayoutObject const*, WTF::KeyValuePair<blink::LayoutObject const*, std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > >, WTF::KeyValuePairKeyExtractor, WTF::PtrHash<blink::LayoutObject const>, WTF::HashMapValueTraits<WTF::HashTraits<blink::LayoutObject const*>, WTF::HashTraits<std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > > >, WTF::HashTraits<blink::LayoutObject const*>, WTF::PartitionAllocator>::erase(WTF::HashTableIterator<blink::LayoutObject const*, WTF::KeyValuePair<blink::LayoutObject const*, std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > >, WTF::KeyValuePairKeyExtractor, WTF::PtrHash<blink::LayoutObject const>, WTF::HashMapValueTraits<WTF::HashTraits<blink::LayoutObject const*>, WTF::HashTraits<std::__ndk1::unique_ptr<blink::SVGResources, std::__ndk1::default_delete<blink::SVGResources> > > >, WTF::HashTraits<blink::LayoutObject const*>, WTF::PartitionAllocator>)+0x4eb8 # gvr_delegate_provider.cc _GLOBAL__sub_I_gvr_delegate_provider.cc+0x20 # gvr_delegate_provider.cc _GLOBAL__sub_I_gvr_delegate_provider.cc+0x24 # gvr_delegate_provider.cc _GLOBAL__sub_I_gvr_delegate_provider.cc+0x28 # gvr_delegate_provider.cc base::OnceCallback<mojo::InterfacePtr<content::mojom::URLLoaderFactory> ()>::OnceCallback() # gvr_delegate_provider.cc v8::StackFrame::IsWasm() const+0x268 # jni_utils.cc _GLOBAL__sub_I_jni_utils.cc+0x18 # jni_utils.cc _GLOBAL__sub_I_jni_utils.cc+0x1c # jni_utils.cc _GLOBAL__sub_I_jni_utils.cc+0x20 # jni_utils.cc std::__ndk1::pair<std::__ndk1::__map_iterator<std::__ndk1::__tree_iterator<std::__ndk1::__value_type<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >, std::__ndk1::__tree_node<std::__ndk1::__value_type<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >, void*>*, int> >, bool> std::__ndk1::map<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> >, std::__ndk1::less<unsigned long long>, std::__ndk1::allocator<std::__ndk1::pair<unsigned long long const, std::__ndk1::vector<int, std::__ndk1::allocator<int> > > > >::emplace<unsigned long long&, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >(unsigned long long&, std::__ndk1::vector<int, std::__ndk1::allocator<int> >&&)+0x10c5c # jniutil.cc _GLOBAL__sub_I_jniutil.cc+0x20 # jniutil.cc _GLOBAL__sub_I_jniutil.cc+0x24 # jniutil.cc _GLOBAL__sub_I_jniutil.cc+0x28 # jniutil.cc std::__ndk1::pair<std::__ndk1::__map_iterator<std::__ndk1::__tree_iterator<std::__ndk1::__value_type<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >, std::__ndk1::__tree_node<std::__ndk1::__value_type<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >, void*>*, int> >, bool> std::__ndk1::map<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> >, std::__ndk1::less<unsigned long long>, std::__ndk1::allocator<std::__ndk1::pair<unsigned long long const, std::__ndk1::vector<int, std::__ndk1::allocator<int> > > > >::emplace<unsigned long long&, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >(unsigned long long&, std::__ndk1::vector<int, std::__ndk1::allocator<int> >&&)+0x10c5c # jniutil.cc std::__ndk1::pair<std::__ndk1::__map_iterator<std::__ndk1::__tree_iterator<std::__ndk1::__value_type<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >, std::__ndk1::__tree_node<std::__ndk1::__value_type<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >, void*>*, int> >, bool> std::__ndk1::map<unsigned long long, std::__ndk1::vector<int, std::__ndk1::allocator<int> >, std::__ndk1::less<unsigned long long>, std::__ndk1::allocator<std::__ndk1::pair<unsigned long long const, std::__ndk1::vector<int, std::__ndk1::allocator<int> > > > >::emplace<unsigned long long&, std::__ndk1::vector<int, std::__ndk1::allocator<int> > >(unsigned long long&, std::__ndk1::vector<int, std::__ndk1::allocator<int> >&&)+0x10c7c
,
Aug 31 2017
:( Code has been added that depends on the roll.
,
Aug 31 2017
How bad is the regression? Do we have stats for that? We'll need to work with the gvr team to get their static initializers removed. The unfortunate thing is that even if they fix them, it won't make in their next release which we need for some pretty important bugfixes. Waiting until like M64 for some of these fixes would be pretty brutal.
,
Aug 31 2017
The gltf_asset.cc static initializer has been around for a long time - not sure how that went undetected. We should certainly fix that one. Same with the gvr_delegate_provider.cc one. I don't think the jniutil.cc one is related to VR though.
,
Aug 31 2017
Spoke with bshe out-of-band. It's fine to try and fix up the chromium-side ones and wait for the next gvr roll to address the gvr-side ones. There's no real urgency to this, although it should be done at some point. We previously had Static-initializer monitoring only for non-Android platforms. We recently added post-commit checks, and plan to add trybot checks in the future.
,
Aug 31 2017
supersize is also able to which files SIs are coming from:
tools/binary_size/supersize console MonochromePublic.apk.size
>>> Print(size_info.symbols.WhereNameMatches('^startup$|^_GLOBAL__'))
Showing 23 symbols (23 unique) with total pss: 36200 bytes
.text=35.4kb total=35.4kb
Number of unique paths: 16
Section Legend: t=.text
Index | Running Total | Section@Address | PSS | Path
------------------------------------------------------------
0) 72 (0.2%) t@0x9916a0 72 third_party/gvr-android-sdk/libgvr_shim_static_arm.a/{shared}/2
_GLOBAL__sub_I_logging.cc
1) 92 (0.3%) t@0x9916e8 20 third_party/gvr-android-sdk/libgvr_shim_static_arm.a/{shared}/2
_GLOBAL__sub_I_controller_api_impl.cc
2) 216 (0.6%) t@0x9916fc 124 third_party/gvr-android-sdk/libgvr_shim_static_arm.a/{shared}/2
_GLOBAL__sub_I_arm_model.cc
3) 252 (0.7%) t@0x991778 36 third_party/gvr-android-sdk/libgvr_shim_static_arm.a/{shared}/2
_GLOBAL__sub_I_jni_utils.cc
4) 552 (1.5%) t@0x99179c 300 third_party/gvr-android-sdk/libgvr_shim_static_arm.a/{shared}/2
_GLOBAL__sub_I_malloc_hook.cc
5) 596 (1.6%) t@0x9918c8 44 third_party/gvr-android-sdk/libgvr_shim_static_arm.a/{shared}/2
_GLOBAL__sub_I_jniutil.cc
6) 624 (1.7%) t@0x9918f4 28 third_party/gvr-android-sdk/libgvr_shim_static_arm.a/{shared}/2
_GLOBAL__sub_I_spinlock.cc
7) 744 (2.1%) t@0x99b5a8 120 android_webview/browser/aw_contents.cc
startup
8) 832 (2.3%) t@0xbc1118 88 third_party/skia/src/effects/SkHighContrastFilter.cpp
startup
9) 1384 (3.8%) t@0xbc7410 552 third_party/skia/src/effects/SkLightingImageFilter.cpp
startup
10) 1848 (5.1%) t@0xbd7ff4 464 third_party/skia/src/shaders/SkPerlinNoiseShader.cpp
startup
11) 1942 (5.4%) t@0xbe8878 94 third_party/skia/src/utils/SkCurveMeasure.cpp
startup
12) 1982 (5.5%) t@0xc05b20 40 third_party/skia/src/gpu/GrDefaultGeoProcFactory.cpp
startup
13) 2022 (5.6%) t@0xc4fc5c 40 third_party/skia/src/gpu/effects/GrNonlinearColorSpaceXformEffect.cpp
startup
14) 2062 (5.7%) t@0xc52750 40 third_party/skia/src/gpu/effects/GrSRGBEffect.cpp
startup
15) 2126 (5.9%) t@0xc73a84 64 third_party/skia/src/gpu/glsl/GrGLSLShaderBuilder.cpp
startup
16) 2170 (6.0%) t@0x126df14 44 device/vr/android/gvr/gvr_delegate_provider.cc
startup
17) 2198 (6.1%) t@0x192e1b4 28 third_party/libaddressinput/src/cpp/src/post_box_matchers.cc
startup
18) 2226 (6.1%) t@0x1c6ff24 28 third_party/ots/src/os2.cc
startup
19) 35630 (98.4%) t@0x24110a0 33404 chrome/browser/about_flags.cc
startup
20) 35956 (99.3%) t@0x267ab30 326 chrome/browser/vr/gltf_asset.cc
startup
21) 36008 (99.5%) t@0x26acf7c 52 third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libs/armeabi-v7a/libc++_static.a/iostream.o
startup
22) 36200 (100.0%) t@0x26bda0c 192 third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libs/armeabi-v7a/libc++_static.a/locale.o
startup
,
Aug 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5a173f1ca896f78110951805c475b3e1c7d20fa6 commit 5a173f1ca896f78110951805c475b3e1c7d20fa6 Author: Michael Thiessen <mthiesse@chromium.org> Date: Thu Aug 31 20:09:37 2017 Fix static initializers in gltf_asset.cc Bug: 761032 Change-Id: I5eb6dce612c920c62bc185d8b113625651f7040f Reviewed-on: https://chromium-review.googlesource.com/646687 Reviewed-by: Biao She <bshe@chromium.org> Commit-Queue: Michael Thiessen <mthiesse@chromium.org> Cr-Commit-Position: refs/heads/master@{#498968} [modify] https://crrev.com/5a173f1ca896f78110951805c475b3e1c7d20fa6/chrome/browser/vr/gltf_asset.cc
,
Sep 1 2017
b/65290885 is created to track GVR side fix. After Micheal's fix, the only remaining file in Chrome is: gvr_delegate_provider.cc I will try to fix it.
,
Sep 5 2017
,
Sep 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/567b76a9c508f2fe387df50cbc7af6f6936b49a0 commit 567b76a9c508f2fe387df50cbc7af6f6936b49a0 Author: Biao She <bshe@chromium.org> Date: Tue Sep 05 17:42:43 2017 Fix static initializer in gvr_delegate_provider This CL also use Factory pattern for creating GvrDelegateProvider. Bug: 761032 Change-Id: I791a78a98b743add2454d112c6958b2a0c097eb7 Reviewed-on: https://chromium-review.googlesource.com/647479 Reviewed-by: Brandon Jones <bajones@chromium.org> Commit-Queue: Biao She <bshe@chromium.org> Cr-Commit-Position: refs/heads/master@{#499667} [modify] https://crrev.com/567b76a9c508f2fe387df50cbc7af6f6936b49a0/chrome/browser/android/vr_shell/vr_shell_delegate.cc [modify] https://crrev.com/567b76a9c508f2fe387df50cbc7af6f6936b49a0/device/vr/BUILD.gn [delete] https://crrev.com/bb279d2a599faa8f28ece17b398cfdbb9f7db098/device/vr/android/gvr/gvr_delegate_provider.cc [modify] https://crrev.com/567b76a9c508f2fe387df50cbc7af6f6936b49a0/device/vr/android/gvr/gvr_delegate_provider.h [add] https://crrev.com/567b76a9c508f2fe387df50cbc7af6f6936b49a0/device/vr/android/gvr/gvr_delegate_provider_factory.cc [add] https://crrev.com/567b76a9c508f2fe387df50cbc7af6f6936b49a0/device/vr/android/gvr/gvr_delegate_provider_factory.h [modify] https://crrev.com/567b76a9c508f2fe387df50cbc7af6f6936b49a0/device/vr/android/gvr/gvr_device.cc
,
Sep 6 2017
FYI - just landed a change to check Android static initializers in the CQ: https://chromium-review.googlesource.com/c/chromium/src/+/647769/7 The check is done by checking in the number of expected initializers, so any further removals will need to update this number as well.
,
Sep 13 2017
GVR has fixed their static initializers in b/65290885, so our next GVR version bump will hopefully include these fixed.
,
Nov 16 2017
The GVR which include static initializer fix is in Chrome now. Mark this one as fixed.
,
Feb 7 2018
,
Jul 4
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by agrieve@chromium.org
, Aug 31 2017