Try to upstream CHROMIUM_CXX_TWEAK_INLINES to libc++ |
|||||
Issue descriptionIn bug 738155 , we modified the ndk's libc++ to add __attribute__((noinline)) in several places, saving almost 1MB of binary size with little runtime performance consequence. We should try to upstream this to llvm (presumably with a different macro name). Maybe ASSUME_LARGE_CODEBASE=1 or something (the savings come from more identical-code-folding, so only apply to larger codebases). Perhaps as a first step, we should apply the patch against our own copy of libc++ (within buildtools/third_party/libc++) to see if the savings exist on other platforms as well.
,
May 25 2018
buildtools/third_party/libc++ is an upstream mirror and can't have local diffs (by design).
,
May 25 2018
It would be nice (or perhaps not so nice!) if DEPS supported patches. I'll investigate if there's any binary size savings on Linux, and also upload an llvm patch.
,
May 25 2018
I've rebased and uploaded a WIP patch here: https://reviews.llvm.org/D47399 The binary size reduction on Linux is surprisingly good. On an official build, this reduced my binary size from 147494776 to 142763928 bytes. That's 4.51MiB or 3.31%.
,
Oct 26
If ToT Clang is the only compiler we care about, then https://reviews.llvm.org/D52405 will address the size issues when it lands on Monday. The chrome binary size goes from 810 MB to 756 MB after this patch.
,
Oct 26
ToT clang is the only compiler we care about for chrome/android (and every other chrome except chrome/ios). 810MB to 756MB sounds like a debug build though :-) Put: is_debug = false in your args.gn to get a release build, and additionally is_official_build = true to get the optimization level we use for shipping (this enables lto and makes links pretty slow, which is why it's a separate toggle).
,
Oct 26
Also, that's fantastic!
,
Oct 26
:O Awesome!! I'll roll those changes into Chrome on Monday! This should also unblock us on using in-tree libc++ for Android.
,
Oct 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/buildtools/+/fdce2ad26b26961629cb6bb2fc4757396d37f174 commit fdce2ad26b26961629cb6bb2fc4757396d37f174 Author: Tom Anderson <thomasanderson@chromium.org> Date: Mon Oct 29 17:41:20 2018 Roll libcxx{abi} This is to pick up "[libc++] Use exclude_from_explicit_instantiation instead of always_inline": https://chromium.googlesource.com/external/llvm.org/libcxx/+/640fa255b9c7441eeace90f96e237ca68bd8c7b6 Full changelogs: https://chromium.googlesource.com/chromium/llvm-project/libcxx/+log/2199647acb904b91eea0a5e045f5b227c87d6e85..640fa255b9c7441eeace90f96e237ca68bd8c7b6 https://chromium.googlesource.com/chromium/llvm-project/libcxxabi/+log/c3f4753f7139c73063304235781e4f7788a94c06..794d4c0c33d99585ae99a069e99ff2c06aa98b56 BUG= 846855 R=thakis Change-Id: I9fa54544198d39de738c1d74e7c0859c2693fbda [modify] https://crrev.com/fdce2ad26b26961629cb6bb2fc4757396d37f174/deps_revisions.gni [modify] https://crrev.com/fdce2ad26b26961629cb6bb2fc4757396d37f174/DEPS
,
Oct 29
I applied the patch and I'm not seeing a significant difference. args.gn: use_goma = true is_debug = false is_component_build = false enable_nacl = false Before: 217588784 (208M) After: 217208008 (208M)
,
Oct 31
ericwf@ I'm not seeing a difference with official builds or debug builds either (patch https://chromium-review.googlesource.com/c/chromium/src/+/1305209). I'm using clang version 8.0.0 (trunk 344066). Is there anything else we have to do to get the size reduction?
,
Oct 31
Your Clang version seems new enough. Can you clean the build artifacts and try again? The build system doesn't seem to track changes to libc++ properly, at least to header only changes.
,
Nov 1
I did a full rebuild but that didn't seem to help (217782400 vs 217410352). The build system should rebuild everything on libc++ rolls as long as this gets updated: https://cs.chromium.org/chromium/src/buildtools/deps_revisions.gni?rcl=13a00f110ef910a25763346d6538b60f12845656&l=9
,
Nov 2
OK, Weird. I double checked and I saw the changes, so I'm not sure what's really going on. What I did to double check was I disabled this `#if` block by adding `&& 0`: https://github.com/llvm-mirror/libcxx/blob/master/include/__config#L757 Can you give me any more information about how your building, so I can help debug this? I wasn't updating that file, Thanks for the heads up.
,
Nov 16
thomasanderson: Looks like we didn't roll forward buildtools. Maybe you didn't have that change because of that?
,
Nov 19
I applied the roll locally when testing. Discussed with ericwf@ offline and he's able to repro now in a release build.
,
Nov 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/buildtools/+/da9b2941cbf6d6d67b694e069e8e8dc06910f66a commit da9b2941cbf6d6d67b694e069e8e8dc06910f66a Author: Nico Weber <thakis@chromium.org> Date: Tue Nov 20 18:58:11 2018 Revert "Roll libcxx{abi} This reverts: > commit fdce2ad26b26961629cb6bb2fc4757396d37f174 > Author: Tom Anderson <thomasanderson@chromium.org> > Date: Mon Oct 29 10:40:50 2018 -0700 > > Roll libcxx{abi} > > This is to pick up "[libc++] Use exclude_from_explicit_instantiation instead of > always_inline": > https://chromium.googlesource.com/external/llvm.org/libcxx/+/640fa255b9c7441eeace90f96e237ca68bd8c7b6 > > Full changelogs: > https://chromium.googlesource.com/chromium/llvm-project/libcxx/+log/2199647acb904b91eea0a5e045f5b227c87d6e85..640fa255b9c7441eeace90f96e237ca68bd8c7b6 > https://chromium.googlesource.com/chromium/llvm-project/libcxxabi/+log/c3f4753f7139c73063304235781e4f7788a94c06..794d4c0c33d99585ae99a069e99ff2c06aa98b56 > > BUG= 846855 > R=thakis Since the CQ doesn't like it and keeping it in buildtools blocks buildtoosl rolls. Change-Id: I284664051b077ecdcde1320e441a7bded1652319 [modify] https://crrev.com/da9b2941cbf6d6d67b694e069e8e8dc06910f66a/deps_revisions.gni [modify] https://crrev.com/da9b2941cbf6d6d67b694e069e8e8dc06910f66a/DEPS
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/buildtools/+/b07ce09e6870a620d7a48e7d78b0885d105f1628 commit b07ce09e6870a620d7a48e7d78b0885d105f1628 Author: Tom Anderson <thomasanderson@chromium.org> Date: Thu Dec 13 00:07:19 2018 Roll lib{c++,c++abi,unwind} https://chromium.googlesource.com/chromium/llvm-project/libcxx/+log/2199647acb904b91eea0a5e045f5b227c87d6e85..f77ee9b3c983dd641a3c4a449554cc878d650dc5 https://chromium.googlesource.com/chromium/llvm-project/libcxxabi/+log/c3f4753f7139c73063304235781e4f7788a94c06..307bb62985575b2e3216a8cfd7e122e0574f33a9 https://chromium.googlesource.com/external/llvm.org/libunwind/+log/94edd59b16d2084a62699290f9cfdf120d74eedf..69d9b84cca8354117b9fe9705a4430d789ee599b R=thakis BUG= 846855 Change-Id: Ic2276eb824fef5cc828b2647031fd310af2efc12 [modify] https://crrev.com/b07ce09e6870a620d7a48e7d78b0885d105f1628/deps_revisions.gni [modify] https://crrev.com/b07ce09e6870a620d7a48e7d78b0885d105f1628/DEPS
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/buildtools/+/5cce74c6ae2e0a24751e92b3ed3f92f8e76935ec commit 5cce74c6ae2e0a24751e92b3ed3f92f8e76935ec Author: Tom Anderson <thomasanderson@chromium.org> Date: Thu Dec 13 20:21:36 2018 Roll libc++ This roll includes 2 commits: https://chromium.googlesource.com/chromium/llvm-project/libcxx/+/e713cc0acf1ae8b82f451bf58ebef67a46ceddfb https://chromium.googlesource.com/chromium/llvm-project/libcxx/+/d3f2d5ec4e82dcec69e3887b0847f6969d9b6b3c The first is needed to fix an odr issue to green-up the asan bot so we can roll buildtools. R=thakis BUG= 846855 Change-Id: Ic0ee5a775a9f6b58fef99d966259e6b9daff8ec7 [modify] https://crrev.com/5cce74c6ae2e0a24751e92b3ed3f92f8e76935ec/deps_revisions.gni [modify] https://crrev.com/5cce74c6ae2e0a24751e92b3ed3f92f8e76935ec/DEPS
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9a085ba23156aba9ade0a08efd3d948f9527f01e commit 9a085ba23156aba9ade0a08efd3d948f9527f01e Author: Tom Anderson <thomasanderson@chromium.org> Date: Thu Dec 13 22:56:15 2018 Roll buildtools BUG= 846855 R=thakis Change-Id: I6c1a1325b089b12e86268b14edbffe4b05a5e58f Reviewed-on: https://chromium-review.googlesource.com/c/1374973 Commit-Queue: Thomas Anderson <thomasanderson@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#616465} [modify] https://crrev.com/9a085ba23156aba9ade0a08efd3d948f9527f01e/DEPS
,
Dec 14
Good news, in my test CL of enabling libc++ on Android, there's actually a 60KB APK size reduction https://ci.chromium.org/p/chromium/builders/luci.chromium.try/android-binary-size/118853 So I believe we're good to go on enabling in-tree libc++ for Android!
,
Dec 14
THAT'S AMAZING NEWS!!!! 🎉 Thanks for working on this!
,
Dec 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/buildtools/+/0e1cbc4eab6861b0c84bf2ed9a3c4b7aa2063819 commit 0e1cbc4eab6861b0c84bf2ed9a3c4b7aa2063819 Author: Tom Anderson <thomasanderson@chromium.org> Date: Fri Dec 14 00:43:35 2018 Stop removing hide_all_but_jni_onload config from libc++ This is no longer needed and causes gn analyze to fail on Android component builds since the hide_all_but_jni_onload config is never added. BUG= 846855 R=thakis Change-Id: I0a9961f6fbda74cd5d3c71a3420588d02bfd86f8 [modify] https://crrev.com/0e1cbc4eab6861b0c84bf2ed9a3c4b7aa2063819/third_party/libc++/BUILD.gn
,
Dec 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bdfa3bb74e76d0da628f876469af1d94bd5bad83 commit bdfa3bb74e76d0da628f876469af1d94bd5bad83 Author: Tom Anderson <thomasanderson@chromium.org> Date: Fri Dec 14 19:59:26 2018 Fixes necessary to enable in-tree libc++ on Android All that should be necessary after this change is to flip use_custom_libcxx to true on Android. BUG= 846855 R=thakis Change-Id: I386176970ea7f05a9dbd7f04a2254c3a14d5419b Reviewed-on: https://chromium-review.googlesource.com/c/1377501 Commit-Queue: Thomas Anderson <thomasanderson@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#616799} [modify] https://crrev.com/bdfa3bb74e76d0da628f876469af1d94bd5bad83/DEPS [modify] https://crrev.com/bdfa3bb74e76d0da628f876469af1d94bd5bad83/build/config/android/BUILD.gn
,
Dec 14
,
Dec 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eebbc8a9b5d3574cb05bafb07319336f011f55f4 commit eebbc8a9b5d3574cb05bafb07319336f011f55f4 Author: Max Moroz <mmoroz@chromium.org> Date: Sat Dec 15 00:09:19 2018 Revert "Fixes necessary to enable in-tree libc++ on Android" This reverts commit bdfa3bb74e76d0da628f876469af1d94bd5bad83. Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=910864#c14 Original change's description: > Fixes necessary to enable in-tree libc++ on Android > > All that should be necessary after this change is to flip use_custom_libcxx to > true on Android. > > BUG= 846855 > R=thakis > > Change-Id: I386176970ea7f05a9dbd7f04a2254c3a14d5419b > Reviewed-on: https://chromium-review.googlesource.com/c/1377501 > Commit-Queue: Thomas Anderson <thomasanderson@chromium.org> > Reviewed-by: Nico Weber <thakis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#616799} TBR=thakis@chromium.org,thomasanderson@chromium.org Change-Id: I82ad36ccb2aadc760045644e208091dc4914f154 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 846855 Reviewed-on: https://chromium-review.googlesource.com/c/1378881 Reviewed-by: Max Moroz <mmoroz@chromium.org> Commit-Queue: Max Moroz <mmoroz@chromium.org> Cr-Commit-Position: refs/heads/master@{#616886} [modify] https://crrev.com/eebbc8a9b5d3574cb05bafb07319336f011f55f4/DEPS [modify] https://crrev.com/eebbc8a9b5d3574cb05bafb07319336f011f55f4/build/config/android/BUILD.gn
,
Dec 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0e2335f44024cc59d7c4a8a3fd76212628f1f64c commit 0e2335f44024cc59d7c4a8a3fd76212628f1f64c Author: Tom Anderson <thomasanderson@chromium.org> Date: Sat Dec 15 00:33:24 2018 Disable libc++ on android and rollback buildtools to 7d88270d This CL 1. Reverts 9a085ba23156aba9ade0a08efd3d948f9527f01e 2. Rolls buildtools back to 7d88270de197ebe8b439ab5eb57a4a2a0bb810e0 3. Disables libc++ on Android BUG=910864, 846855 , 767901 TBR=thakis CC=mmoroz NOTRY=true Change-Id: I44e3a504fa6b842099fef78371c7dfcf7921f2f5 Reviewed-on: https://chromium-review.googlesource.com/c/1379245 Commit-Queue: Thomas Anderson <thomasanderson@chromium.org> Reviewed-by: Thomas Anderson <thomasanderson@chromium.org> Cr-Commit-Position: refs/heads/master@{#616895} [modify] https://crrev.com/0e2335f44024cc59d7c4a8a3fd76212628f1f64c/DEPS [modify] https://crrev.com/0e2335f44024cc59d7c4a8a3fd76212628f1f64c/build/config/android/BUILD.gn [modify] https://crrev.com/0e2335f44024cc59d7c4a8a3fd76212628f1f64c/build/config/c++/c++.gni |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by agrieve@chromium.org
, May 25 2018