Investigate adding __attribute__((noinline)) to std::string constructors (1MB) |
||||||||||
Issue descriptionI tried adding __attribute__((noinline)) to every basic_string constructors (including destructor) in: third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string This reduced libmonochrome.so by 400kb (for is_official_build) I then also added it to operator=(), and it shrank by another 100kb No idea what the perf impact of this is, but it's a large enough reduction to do some perf tests for! We may also want to consider using ((noinline)) for targets compiled with optimize_for_size, and allow inlining for optimize_for_speed.
,
Jun 29 2017
Somewhat related is bug 733470. Uploaded ndk patch (apply to third_party/android_tools/ndk)
,
Jul 17 2017
Related: bug 738469 : 150kb saved by making v8_inspector::String16 constructors out-of-line (this class is just a std::string wrapper). bug 741831 : 100kb regression from std::string operator+ being inlined more aggressively for -O2 compilation units.
,
Jul 17 2017
,
Jul 19 2017
Side experiment: I used objdump to see which functions are inlined the most. Methodology: third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-objdump --line-numbers --source --disassemble -j .text out-gn/Release/lib.unstripped/libmonochrome.so| grep -E ':[0-9]+$' > decompile-paths2.txt # Wait ~2 weeks sed -E -i s:^$PWD/out-gn/Release/:: decompile-paths2.txt sort < decompile-paths2.txt > sorted.txt uniq --count < sorted.txt > counted.txt sort -n --reverse < counted.txt > final.txt Results: * I've attached final.txt, but with counts < 6 removed. * Note that objdump gives some pre-context for source, so each line number is generally 5 less than the relevant function name. The top results are: 36334 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2628 35334 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2629 32155 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2627 25475 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:1712 23509 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2608 18226 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/type_traits:3313 17629 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2138 16141 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2144 15926 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:657 15665 ../../v8/include/v8.h:9441 15428 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2609 15165 ../../v8/src/handles.h:56 14070 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2620 13532 ../../third_party/WebKit/Source/platform/wtf/RefPtr.h:80 13016 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2621 12941 ../../base/bind_internal.h:434 12832 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:1813 12693 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:1774 12374 ../../v8/src/base/atomicops_internals_portable.h:105 12276 ../../base/memory/ref_counted.h:512 11854 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/type_traits:3312 11522 ../../base/bind.h:65 11423 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:1636 11284 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:1479 11222 ../../third_party/WebKit/Source/platform/wtf/Vector.h:396 11121 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/type_traits:3314 10093 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:1806 9558 ../../third_party/WebKit/Source/platform/heap/Member.h:116 8354 ../../third_party/WebKit/Source/platform/heap/Member.h:80 7705 ../../third_party/WebKit/Source/platform/wtf/PassRefPtr.h:51 7629 ../../base/bind_internal.h:329 7564 ../../third_party/WebKit/Source/platform/heap/Member.h:82 7392 ../../third_party/WebKit/Source/platform/wtf/Vector.h:991 7372 ../../base/callback_internal.h:149 7356 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:1791 7317 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/tuple:216 7089 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:1634 6975 ../../third_party/WebKit/Source/platform/heap/Member.h:83 6760 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:1794 6738 ../../base/bind_internal.h:351 6667 ../../third_party/WebKit/Source/platform/bindings/WrapperTypeInfo.h:212 6571 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:651 6515 ../../v8/src/heap/spaces.h:442 6500 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:1645 6321 ../../base/memory/ref_counted.h:503 6291 ../../third_party/WebKit/Source/platform/wtf/RefPtr.h:55 6270 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2616 6185 ../../base/memory/ref_counted.h:472 6095 ../../base/memory/ref_counted.h:473 5905 ../../v8/include/v8.h:9039 5853 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/iterator:1185 5741 ../../base/bind.h:69 5720 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:455 5705 ../../third_party/WebKit/Source/platform/wtf/Vector.h:1021 5674 ../../base/bind.h:52 5516 ../../v8/src/objects-inl.h:3392 5303 ../../v8/src/utils.h:309 5052 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:375 5028 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:448 4905 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:1756 4904 ../../base/bind_internal.h:330 4901 ../../third_party/protobuf/src/google/protobuf/generated_message_util.h:87 4894 ../../v8/src/heap/spaces.h:394 4773 ../../base/memory/ptr_util.h:56 4697 ../../third_party/WebKit/Source/bindings/core/v8/ExceptionState.h:120 4648 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:1497 4457 ../../third_party/WebKit/Source/platform/wtf/Vector.h:398 4411 ../../third_party/protobuf/src/google/protobuf/arenastring.h:220 4395 ../../base/bind_internal.h:467 4285 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:662 4280 ../../third_party/WebKit/Source/platform/wtf/RefPtr.h:51 4270 ../../third_party/WebKit/Source/bindings/core/v8/ExceptionState.h:81 4263 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:951 4261 ../../v8/src/heap/heap-inl.h:122 4240 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:1764 4197 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/__hash_table:77 4135 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/__tree:552 4097 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:2122 4022 ../../v8/src/handles-inl.h:67 3998 ../../v8/src/heap/spaces.h:757 3993 ../../base/callback.h:37 3965 ../../base/memory/ref_counted.h:153 3943 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:2324 3930 ../../v8/src/handles-inl.h:68 3911 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:639 3899 ../../base/bind_internal.h:466 3760 ../../third_party/WebKit/Source/platform/heap/Member.h:78 3735 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2799 3719 ../../base/callback_forward.h:29 3699 ../../v8/src/objects.h:1735 3660 ../../third_party/WebKit/Source/platform/wtf/PassRefPtr.h:59 3617 ../../v8/include/v8.h:9424 3616 ../../third_party/protobuf/src/google/protobuf/io/coded_stream.h:1194 3524 ../../third_party/WebKit/Source/platform/wtf/PassRefPtr.h:60 3509 ../../third_party/WebKit/Source/platform/wtf/text/StringImpl.h:193 3456 ../../mojo/public/cpp/system/handle.h:80 3425 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:1464 3402 ../../third_party/skia/include/core/SkRefCnt.h:330 3391 ../../v8/include/v8.h:8849 3376 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:1771 3332 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:1601 3317 ../../third_party/skia/src/core/../opts/SkNx_neon.h:140 3316 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/tuple:291 3266 ../../third_party/WebKit/Source/platform/heap/ThreadState.h:733 3250 ../../v8/src/handles-inl.h:103 3228 ../../v8/src/handles-inl.h:107 3225 ../../v8/src/handles-inl.h:109 3218 ../../base/bind_internal.h:470 3204 ../../third_party/WebKit/Source/platform/bindings/ScopedPersistent.h:53 3194 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:953 3190 ../../third_party/WebKit/Source/platform/wtf/RefPtr.h:85 3175 ../../v8/src/handles-inl.h:117 3159 ../../base/bind_internal.h:468 3144 ../../v8/src/handles-inl.h:118 3130 ../../third_party/WebKit/Source/bindings/core/v8/ExceptionState.h:78 3128 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/__split_buffer:327 3120 ../../base/bind_internal.h:475 3119 ../../base/bind_internal.h:476 3085 ../../third_party/WebKit/Source/platform/wtf/text/StringView.h:215 3072 ../../mojo/public/cpp/bindings/lib/serialization_forward.h:58 3068 ../../v8/src/handles-inl.h:102 3067 ../../v8/src/base/atomicops_internals_portable.h:93 3060 ../../third_party/skia/include/core/SkRefCnt.h:192 3021 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/atomic:922 2988 ../../third_party/WebKit/Source/platform/wtf/text/StringImpl.h:264 2962 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/algorithm:700 2958 ../../v8/include/v8.h:9046 2951 ../../v8/src/counters.h:981 2927 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2607 2889 ../../third_party/WebKit/Source/platform/wtf/RefPtr.h:87 2880 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:1607 2865 ../../third_party/WebKit/Source/bindings/core/v8/ExceptionState.h:80 2834 ../../v8/src/handles-inl.h:70 2809 ../../v8/src/handles-inl.h:32 2798 ../../v8/src/heap/heap-inl.h:650 2797 ../../base/memory/ref_counted.h:466 2790 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/__split_buffer:326 2777 ../../base/trace_event/trace_event.h:1042 2766 ../../base/memory/ref_counted.h:488 2765 ../../v8/src/utils.h:496 2751 ../../v8/include/v8.h:9404 2733 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:1472 2718 ../../base/memory/ref_counted.h:467 2709 ../../third_party/WebKit/Source/platform/wtf/ConditionalDestructor.h:20 2703 ../../base/trace_event/trace_event.h:1045 2702 ../../base/trace_event/trace_event.h:1043 2695 ../../third_party/WebKit/Source/bindings/core/v8/ExceptionState.h:77 2684 ../../base/trace_event/trace_event.h:1030 2677 ../../base/trace_event/trace_event.h:1044 2671 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:1583 2662 ../../third_party/skia/include/core/SkRefCnt.h:282 2661 ../../third_party/WebKit/Source/platform/wtf/Vector.h:397 2594 ../../third_party/skia/src/core/../opts/SkNx_neon.h:138 2554 ../../v8/src/inspector/string-16.h:112 2554 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:1781 2550 ../../third_party/WebKit/Source/platform/wtf/Vector.h:433 2525 ../../v8/src/objects-inl.h:159 2518 ../../base/bind_internal.h:340 2513 ../../base/callback_internal.h:127 2471 ../../third_party/WebKit/Source/platform/wtf/text/AtomicString.h:50 2469 ../../base/logging.h:975 2467 ../../base/threading/thread_local.h:65 2464 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:3833 2464 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:3185 2450 ../../third_party/WebKit/Source/platform/heap/Member.h:36 2441 ../../base/bind.h:43 2438 ../../third_party/WebKit/Source/platform/heap/TraceTraits.h:225 2422 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/tuple:180 2421 ../../v8/src/compiler/graph.h:70 2392 ../../base/bind_internal.h:453 2391 ../../v8/include/v8.h:9447 2364 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:473 2342 ../../v8/src/compiler/js-graph.h:156 2334 ../../third_party/skia/src/core/../opts/SkNx_neon.h:176 2333 ../../third_party/WebKit/Source/bindings/core/v8/ExceptionState.h:79 2282 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:1588 2278 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:3221 2275 ../../third_party/skia/src/core/../opts/SkNx_neon.h:175 2272 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/utility:306 2267 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2198 2265 ../../base/bind_internal.h:459 2259 ../../third_party/WebKit/Source/platform/heap/Heap.h:509 2253 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:1571 2237 ../../base/memory/ref_counted.h:558 2216 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/__tree:1130 2205 ../../third_party/skia/include/core/SkRefCnt.h:87 2205 ../../third_party/skia/include/core/SkRefCnt.h:84 2194 ../../base/bind_internal.h:457 2172 ../../base/memory/weak_ptr.h:242 2117 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:2486 2098 ../../mojo/public/cpp/system/handle.h:157 2089 ../../v8/src/counters.h:1386 2065 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:456 2063 ../../base/time/time.h:367 2061 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:1959 2045 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:1700 2045 ../../base/synchronization/lock_impl.h:70 2044 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:663 2023 ../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/string:1490 2023 ../../base/bind_internal.h:456 2015 ../../ipc/ipc_message_utils.h:106 2011 ../../v8/src/base/atomic-utils.h:116 2009 ../../base/bind_internal.h:275 2003 ../../third_party/protobuf/src/google/protobuf/io/coded_stream.h:1001 Note: use a version built at afa076bcf93368cfd37ca5fa972ce2537eaa1cc7 Takeaway: * This experiment doesn't show at all whether any of these inlines would result in a space saving if out-lined. * Might be worth compiling with -fno-inline and dumping the size of these functions to compute saving estimates (although apparently __attribute__((always_inline)) functions will still be inlined) For reference, here's a snippet of what disassembly output looks like: _ZNVSt6__ndk113__atomic_baseIiLb0EE23compare_exchange_strongERiiNS_12memory_orderES3_(): /usr/local/google/home/agrieve/ssd/git/clankium1/src/out-gn/Release/../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/atomic:862 (discriminator 24) memory_order __s, memory_order __f) _NOEXCEPT {return __c11_atomic_compare_exchange_weak(&__a_, &__e, __d, __s, __f);} _LIBCPP_INLINE_VISIBILITY bool compare_exchange_strong(_Tp& __e, _Tp __d, memory_order __s, memory_order __f) volatile _NOEXCEPT {return __c11_atomic_compare_exchange_strong(&__a_, &__e, __d, __s, __f);} 2dee0e: e854 0f00 ldrex r0, [r4] 2dee12: b920 cbnz r0, 2dee1e <_Znaj+0x4a> 2dee14: e84a 5000 strex r0, r5, [sl] 2dee18: 2800 cmp r0, #0 2dee1a: d1f8 bne.n 2dee0e <_Znaj+0x3a> 2dee1c: e00d b.n 2dee3a <_Znaj+0x66> 2dee1e: f3bf 8f2f clrex 2dee22: f3bf 8f5b dmb ish _ZN12_GLOBAL__N_114CallNewHandlerEj(): /usr/local/google/home/agrieve/ssd/git/clankium1/src/out-gn/Release/../../base/allocator/allocator_shim.cc:70 // 4.9 and newer, but it will be a few more years before a newer sysroot // becomes available. std::new_handler nh; { while (subtle::Acquire_CompareAndSwap(&g_new_handler_lock, 0, 1)) PlatformThread::YieldCurrentThread(); 2dee26: f2bf f511 bl 99e84c <_ZN4base14PlatformThread18YieldCurrentThreadEv> _ZNVSt6__ndk113__atomic_baseIiLb0EE23compare_exchange_strongERiiNS_12memory_orderES3_(): /usr/local/google/home/agrieve/ssd/git/clankium1/src/out-gn/Release/../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/atomic:862 (discriminator 24) 2dee2a: e85b 0f00 ldrex r0, [fp] 2dee2e: 2800 cmp r0, #0 2dee30: d1f5 bne.n 2dee1e <_Znaj+0x4a> 2dee32: e848 5000 strex r0, r5, [r8] 2dee36: 2800 cmp r0, #0 2dee38: d1f7 bne.n 2dee2a <_Znaj+0x56> _ZN12_GLOBAL__N_114CallNewHandlerEj(): /usr/local/google/home/agrieve/ssd/git/clankium1/src/out-gn/Release/../../base/allocator/allocator_shim.cc:71 nh = std::set_new_handler(0); 2dee3a: 2000 movs r0, #0 _ZNVSt6__ndk113__atomic_baseIiLb0EE23compare_exchange_strongERiiNS_12memory_orderES3_(): /usr/local/google/home/agrieve/ssd/git/clankium1/src/out-gn/Release/../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/atomic:862 (discriminator 24) 2dee3c: f3bf 8f5b dmb ish 2dee40: 2600 movs r6, #0 _ZN12_GLOBAL__N_114CallNewHandlerEj():
,
Jul 19 2017
Also - nice observation about _LIBCPP_INLINE_VISIBILITY! The definition is: #ifndef _LIBCPP_INLINE_VISIBILITY # ifdef _LIBCPP_MSVC # define _LIBCPP_INLINE_VISIBILITY __forceinline # else // MinGW GCC and Clang # define _LIBCPP_INLINE_VISIBILITY __attribute__ ((__always_inline__)) # endif #endif So, an easy next step is to just pass -D_LIBCPP_INLINE_VISIBILITY when compiling so that it's the compiler that gets to make inlining decisions.
,
Aug 16 2017
,
Sep 8 2017
Just tested #6 and found that it increased binary size:
Original libchrome.so: 50557464
libchrome.so with patch: 50807320
Here's my patch:
diff --git a/build/config/android/BUILD.gn b/build/config/android/BUILD.gn
index 35ecf928db96..e09e89cdb30a 100644
--- a/build/config/android/BUILD.gn
+++ b/build/config/android/BUILD.gn
@@ -122,6 +122,10 @@ config("runtime_library") {
]
defines = [ "__GNU_SOURCE=1" ] # Necessary for clone().
+ defines += [ "_LIBCPP_INLINE_VISIBILITY=" ]
+ defines += [ "_LIBCPP_INLINE_VISIBILITY_EXCEPT_GCC49=" ]
ldflags = [ "-nostdlib" ]
lib_dirs = [ android_libcpp_lib_dir ]
,
Sep 8 2017
Another interesting commit just came up: https://chromium-review.googlesource.com/c/v8/v8/+/654898 Changing from v8::List -> std::vector caused 10kb in code size increase for a definition. Perhaps vector::push_back would also benefit from not being forced inline
,
Sep 8 2017
Original libchrome.so (arm32) 50569752 Removing _LIBCPP_INLINE_VISIBILITY from vector::push_back 50532888 (37kb smaller) Adding __attribute__((noinline)) to push_back: 50397720 (172kb smaller)
,
Sep 22 2017
Finally have a CL up: https://chromium-review.googlesource.com/c/android_ndk/+/676666 Some explanation of methodology: - Make a change, then use: tools/binary_size/diagnose_bloat.py --subrepo third_party/android_tools/ndk HEAD --clean - Some methods reduce code size by force-inlining them, some reduce by disallowing inlining - Methods take take parameters require more machine instructions to call - Use supersize console command printed by diagnose_bloat.py, then see if any noinline functions are listed with a small size (esp. ones that take parameters): - Example query: >>> Print(size_info2.symbols.WhereNameMatches('::basic_string').Sorted(key=lambda s: s.size_without_padding), verbose=1) Why is this required at all? Why does the compiler not do the right thing? - Inlining is hard because the decision is made for each .o file independent of others. - It might make sense to inline within a single .o file - But across the entire codebase it might be much better to not inline.
,
Sep 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/android_ndk/+/81aa20a99d189418dc3639a4026a61bf158098ad commit 81aa20a99d189418dc3639a4026a61bf158098ad Author: Andrew Grieve <agrieve@chromium.org> Date: Mon Sep 25 18:56:03 2017 Add CHROMIUM_CXX_TWEAK_INLINES to reduce binary size Experimented with std::vector & std::string, and ended up shrinking libchrome.so by 728kb (2% of .text) Used system health trybots and found no difference in the (fairly noisy) metrics: https://docs.google.com/document/d/11v5NDuk_CgG7b3rxWdxt-LZu8QDWgfTa3NuWg3i7mmc/edit Bug: 738155 Change-Id: Ib3962e3a9d3ca11c51f4815fa8e26182300a9905 Reviewed-on: https://chromium-review.googlesource.com/676666 Reviewed-by: Andrew Grieve <agrieve@chromium.org> Tested-by: Andrew Grieve <agrieve@chromium.org> [modify] https://crrev.com/81aa20a99d189418dc3639a4026a61bf158098ad/sources/cxx-stl/llvm-libc++/libcxx/include/__config [modify] https://crrev.com/81aa20a99d189418dc3639a4026a61bf158098ad/sources/cxx-stl/llvm-libc++/libcxx/include/vector [modify] https://crrev.com/81aa20a99d189418dc3639a4026a61bf158098ad/README.chromium [add] https://crrev.com/81aa20a99d189418dc3639a4026a61bf158098ad/chromium-patches/Lower-std-deque-block-size.patch [add] https://crrev.com/81aa20a99d189418dc3639a4026a61bf158098ad/chromium-patches/CHROMIUM_CXX_TWEAK_INLINES.patch [modify] https://crrev.com/81aa20a99d189418dc3639a4026a61bf158098ad/sources/cxx-stl/llvm-libc++/libcxx/include/string
,
Sep 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/android_tools/+/3dd12d2ebe9ca670f637238ad86b3cba9da8c289 commit 3dd12d2ebe9ca670f637238ad86b3cba9da8c289 Author: Andrew Grieve <agrieve@chromium.org> Date: Mon Sep 25 19:09:28 2017 Roll ndk to include CHROMIUM_CXX_TWEAK_INLINES change. Bug: 738155 Change-Id: I94a883bfcbaeb6781eec3e605d7face859ee22ba Reviewed-on: https://chromium-review.googlesource.com/682637 Reviewed-by: John Budorick <jbudorick@chromium.org> [modify] https://crrev.com/3dd12d2ebe9ca670f637238ad86b3cba9da8c289/DEPS
,
Sep 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/android_ndk/+/19c16015e3c888062e57dda6aeb83de39f9ba080 commit 19c16015e3c888062e57dda6aeb83de39f9ba080 Author: Andrew Grieve <agrieve@chromium.org> Date: Tue Sep 26 01:12:50 2017 Revert "Add CHROMIUM_CXX_TWEAK_INLINES to reduce binary size" This reverts commit 81aa20a99d189418dc3639a4026a61bf158098ad. Reason for revert: Doesn't work in component builds Original change's description: > Add CHROMIUM_CXX_TWEAK_INLINES to reduce binary size > > Experimented with std::vector & std::string, and ended up shrinking > libchrome.so by 728kb (2% of .text) > > Used system health trybots and found no difference in the > (fairly noisy) metrics: > https://docs.google.com/document/d/11v5NDuk_CgG7b3rxWdxt-LZu8QDWgfTa3NuWg3i7mmc/edit > > Bug: 738155 > Change-Id: Ib3962e3a9d3ca11c51f4815fa8e26182300a9905 > Reviewed-on: https://chromium-review.googlesource.com/676666 > Reviewed-by: Andrew Grieve <agrieve@chromium.org> > Tested-by: Andrew Grieve <agrieve@chromium.org> TBR=agrieve@chromium.org,dskiba@chromium.org,jbudorick@chromium.org Change-Id: Id3068444193ad148ef9c40dca6c0579430964157 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 738155 Reviewed-on: https://chromium-review.googlesource.com/683554 Reviewed-by: Andrew Grieve <agrieve@chromium.org> Tested-by: Andrew Grieve <agrieve@chromium.org> [modify] https://crrev.com/19c16015e3c888062e57dda6aeb83de39f9ba080/sources/cxx-stl/llvm-libc++/libcxx/include/__config [modify] https://crrev.com/19c16015e3c888062e57dda6aeb83de39f9ba080/sources/cxx-stl/llvm-libc++/libcxx/include/vector [modify] https://crrev.com/19c16015e3c888062e57dda6aeb83de39f9ba080/README.chromium [delete] https://crrev.com/81aa20a99d189418dc3639a4026a61bf158098ad/chromium-patches/Lower-std-deque-block-size.patch [delete] https://crrev.com/81aa20a99d189418dc3639a4026a61bf158098ad/chromium-patches/CHROMIUM_CXX_TWEAK_INLINES.patch [modify] https://crrev.com/19c16015e3c888062e57dda6aeb83de39f9ba080/sources/cxx-stl/llvm-libc++/libcxx/include/string
,
Sep 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/android_ndk/+/d57523210239b867fa4fb9d05c2aacc3f1802fe0 commit d57523210239b867fa4fb9d05c2aacc3f1802fe0 Author: Andrew Grieve <agrieve@chromium.org> Date: Tue Sep 26 01:16:28 2017 Reland: Add CHROMIUM_CXX_TWEAK_INLINES to reduce binary size This reverts commit 19c16015e3c888062e57dda6aeb83de39f9ba080. Reason for revert: Fixed component builds Experimented with std::vector & std::string, and ended up shrinking libchrome.so by 728kb (2% of .text) Used system health trybots and found no difference in the (fairly noisy) metrics: https://docs.google.com/document/d/11v5NDuk_CgG7b3rxWdxt-LZu8QDWgfTa3NuWg3i7mmc/edit Bug: 738155 TBR=agrieve@chromium.org,dskiba@chromium.org,jbudorick@chromium.org Change-Id: I0198c344945209a3c5f64ab7fa43884b7bd26c8a Reviewed-on: https://chromium-review.googlesource.com/683555 Reviewed-by: Andrew Grieve <agrieve@chromium.org> Tested-by: Andrew Grieve <agrieve@chromium.org> [modify] https://crrev.com/d57523210239b867fa4fb9d05c2aacc3f1802fe0/sources/cxx-stl/llvm-libc++/libcxx/include/__config [modify] https://crrev.com/d57523210239b867fa4fb9d05c2aacc3f1802fe0/sources/cxx-stl/llvm-libc++/libcxx/include/vector [modify] https://crrev.com/d57523210239b867fa4fb9d05c2aacc3f1802fe0/README.chromium [add] https://crrev.com/d57523210239b867fa4fb9d05c2aacc3f1802fe0/chromium-patches/Lower-std-deque-block-size.patch [add] https://crrev.com/d57523210239b867fa4fb9d05c2aacc3f1802fe0/chromium-patches/CHROMIUM_CXX_TWEAK_INLINES.patch [modify] https://crrev.com/d57523210239b867fa4fb9d05c2aacc3f1802fe0/sources/cxx-stl/llvm-libc++/libcxx/include/string
,
Sep 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/android_tools/+/ca9dc7245b888c75307f0619e4a39fb46a82de66 commit ca9dc7245b888c75307f0619e4a39fb46a82de66 Author: Andrew Grieve <agrieve@chromium.org> Date: Tue Sep 26 01:18:33 2017 Roll ndk to include CHROMIUM_CXX_TWEAK_INLINES reland. TBR=jbudorick Bug: 738155 Change-Id: Ib2ce27d8de7e2c24da8e46a411c4e479f4329ddf Reviewed-on: https://chromium-review.googlesource.com/683465 Reviewed-by: Andrew Grieve <agrieve@chromium.org> [modify] https://crrev.com/ca9dc7245b888c75307f0619e4a39fb46a82de66/DEPS
,
Sep 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a218af5c68598645675d76980984777c8429bb3 commit 8a218af5c68598645675d76980984777c8429bb3 Author: Andrew Grieve <agrieve@chromium.org> Date: Tue Sep 26 04:31:21 2017 Android: Roll ndk to add CHROMIUM_CXX_TWEAK_INLINES Expected to save ~700kb of .text on Android Bug: 738155 Change-Id: I58886003c259a65cd2e66baa71e38bfa962a2b3d Reviewed-on: https://chromium-review.googlesource.com/682647 Commit-Queue: Andrew Grieve <agrieve@chromium.org> Reviewed-by: John Budorick <jbudorick@chromium.org> Cr-Commit-Position: refs/heads/master@{#504283} [modify] https://crrev.com/8a218af5c68598645675d76980984777c8429bb3/DEPS [modify] https://crrev.com/8a218af5c68598645675d76980984777c8429bb3/build/config/android/BUILD.gn
,
Sep 26 2017
Filed https://bugs.chromium.org/p/chromium/issues/detail?id=768865 to analyze the objdump output from comment#5. Ended up saving 767kb with this! https://chromeperf.appspot.com/report?sid=f88d0ca3d1d8326db3535f8a240f59b0fd7240dd103f18ce5848ac85a4ff881a&rev=504283
,
Sep 28 2017
Note: this did end up triggering a (single) perf regression. Tracking in bug 769548
,
Nov 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/android_ndk/+/50f4963776c431db65f21e92e4a5e67a4b715920 commit 50f4963776c431db65f21e92e4a5e67a4b715920 Author: David 'Digit' Turner <digit@google.com> Date: Thu Nov 30 02:29:47 2017 Add CHROMIUM_CXX_TWEAK_INLINES to reduce binary size (NDK r16) This patch is a port of Andrew´s original patch reviewed at: https://chromium-review.googlesource.com/c/android_ndk/+/683555 This should reduce the size of libchrome.so by about 700 KiB with little impact on performance. R=agrieve Bug: 738155 Change-Id: If3d442ae37379585c516dc8acda54a88adc7f6da Reviewed-on: https://chromium-review.googlesource.com/797450 Reviewed-by: agrieve <agrieve@chromium.org> Tested-by: agrieve <agrieve@chromium.org> [modify] https://crrev.com/50f4963776c431db65f21e92e4a5e67a4b715920/sources/cxx-stl/llvm-libc++/include/__config [modify] https://crrev.com/50f4963776c431db65f21e92e4a5e67a4b715920/sources/cxx-stl/llvm-libc++/include/string [modify] https://crrev.com/50f4963776c431db65f21e92e4a5e67a4b715920/chromium-patches/CHROMIUM_CXX_TWEAK_INLINES.patch [modify] https://crrev.com/50f4963776c431db65f21e92e4a5e67a4b715920/sources/cxx-stl/llvm-libc++/include/vector
,
Dec 7 2017
Note that with the r12->r16 ndk roll, this patch is even more necessary. Rolling without the patch: +1,011,836 bytes Native code size Rolling with the patch: +36,988 bytes Native code size
,
Dec 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/android_ndk/+/5fda77d1e724be951783a8449c6167c7db90b197 commit 5fda77d1e724be951783a8449c6167c7db90b197 Author: Brian Sheedy <bsheedy@chromium.org> Date: Mon Dec 18 18:06:35 2017 Revert "Add CHROMIUM_CXX_TWEAK_INLINES to reduce binary size (NDK r16)" This reverts commit 50f4963776c431db65f21e92e4a5e67a4b715920. Reason for revert: Causes linking issues on component builds. Original change's description: > Add CHROMIUM_CXX_TWEAK_INLINES to reduce binary size (NDK r16) > > This patch is a port of Andrew´s original patch reviewed at: > https://chromium-review.googlesource.com/c/android_ndk/+/683555 > > This should reduce the size of libchrome.so by about 700 KiB > with little impact on performance. > > R=agrieve > Bug: 738155 > Change-Id: If3d442ae37379585c516dc8acda54a88adc7f6da > Reviewed-on: https://chromium-review.googlesource.com/797450 > Reviewed-by: agrieve <agrieve@chromium.org> > Tested-by: agrieve <agrieve@chromium.org> TBR=digit@chromium.org,agrieve@chromium.org,bsheedy@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 738155 Change-Id: Ia39beadaf61d974886443507d12c78dd343a9264 Reviewed-on: https://chromium-review.googlesource.com/832847 Reviewed-by: Brian Sheedy <bsheedy@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> Tested-by: agrieve <agrieve@chromium.org> [modify] https://crrev.com/5fda77d1e724be951783a8449c6167c7db90b197/sources/cxx-stl/llvm-libc++/include/__config [modify] https://crrev.com/5fda77d1e724be951783a8449c6167c7db90b197/sources/cxx-stl/llvm-libc++/include/string [modify] https://crrev.com/5fda77d1e724be951783a8449c6167c7db90b197/chromium-patches/CHROMIUM_CXX_TWEAK_INLINES.patch [modify] https://crrev.com/5fda77d1e724be951783a8449c6167c7db90b197/sources/cxx-stl/llvm-libc++/include/vector
,
Dec 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/android_ndk/+/e951c37287c7d8cd915bf8d4149fd4a06d808b55 commit e951c37287c7d8cd915bf8d4149fd4a06d808b55 Author: Andrew Grieve <agrieve@chromium.org> Date: Tue Dec 19 19:06:01 2017 Fix component build link errors Same issue that was fixed in: d57523210239b867fa4fb9d05c2aacc3f1802fe0 Bug: 738155 Change-Id: I6abb9c690eb1862cbe87c9eb14f5aa36d7391ce9 Reviewed-on: https://chromium-review.googlesource.com/833071 Reviewed-by: John Budorick <jbudorick@chromium.org> Tested-by: agrieve <agrieve@chromium.org> [modify] https://crrev.com/e951c37287c7d8cd915bf8d4149fd4a06d808b55/sources/cxx-stl/llvm-libc++/include/__config [modify] https://crrev.com/e951c37287c7d8cd915bf8d4149fd4a06d808b55/chromium-patches/CHROMIUM_CXX_TWEAK_INLINES.patch |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by dskiba@chromium.org
, Jun 29 2017