-
Notifications
You must be signed in to change notification settings - Fork 258
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
clang produces larger binaries than gcc for arm7 #133
Comments
measured across the whole platform, the switch was a wash --- some devices were a few MiB smaller, some a few MiB larger. and that's out of ~1GiB+! i think the ChromeOS folks are investigating switching atm, so i'll see if we can out more about their experience --- libwebview_chromium.so is orders of magnitude larger than the next largest library on the system (and we take prebuilts, so they won't have been affected by the rest of the platform switching). out of interest --- do you build for x86/x86-64 too? do you see similar results there or is that better/worse? |
We build for x86 as well, but I haven't measured that. I will do that and report back. |
Which -O flags are you passing? Are you using -Os or just -O2? It would be really useful to know the common set of flags. One other question is are you stripping these binaries? Clang has larger debug info than GCC, but that shouldn't be a problem for shipped builds. |
We use We link with The measurements are done with unstripped binaries, but the measurements only look at things that wouldn't get stripped away; in the below output,
Or, if you prefer:
The first measurement with |
Could you upload the gcc and clang-built binaries somewhere for comparison? |
Sure. Is it a problem if I strip debug information but leave symbols? My upload speed is terrible. |
I think that should be fine. |
I've uploaded them at https://people.mozilla.org/~nfroyd/libxuls.tar.bz2 . That'll unpack into:
Please let me know if you need anything else. |
FYI, it sounds like the ChromeOS folks might have found something. It seems that clang is usually better or equivalent, but there are some pathological cases where it produces much larger binaries. |
The ChromeOS issues are actually invalid, since they were omitting -mthumb, which isn't on by default for the clang target they are using. When they recompiled for Thumb-2, they got a 5.5% reduction vs. GCC on code size. Can you check to make sure that you are indeed using -mthumb throughout your build (and not putting -mno-thumb anywhere)? |
We are using
(that is, show me all the function symbols that have arm addresses; thumb addresses would have the lowest bit set and would match the regex Eyeballing a build build log also shows that we are passing |
In our company, we use exactly the same compile flags for both GCC and Clang, as described in #21, yet binaries produced by Clang are still significantly larger. |
Any news on this matter? If anyone wants to test it, I can provide the steps to compile Qt to reproduce this problem. |
@pirama-arumuga-nainar: You'd looked at some of the other qt issues, could you take a look at the 40% (wat) size increase? Anything that bad probably has some pretty obvious causes... |
@bog-dan-ro Can you post instructions to reproduce? It'd be great if you can isolate it to one or few object files with egregious increase, and post build commands with both gcc and clang. |
@pirama-arumuga-nainar they are almost the same instructions as #34 (comment) Sadly I compared the final stripped libs ...
Final step is to configure & compile Qt, you'll need Android NDK & SDK (with API-16 installed): Build for with clang:
The libs will be placed in libs Build for with g++ (please use NDKr10e):
Same as above, the libs will be placed in libs I'll do the same thing tomorrow, maybe I'll spot the object files that are way too big. Edit: qtbase is enough
For instance libQt5Core.so is just ~20% larger but libQt5Sql.so is over 60% (same libQt5Xml.so) [1] |
@pirama-arumuga-nainar QtXML seems the easiest to check it hs just a few .o files. You can find the .o files in |
@bog-dan-ro: Just looking at build output, you're not using Another thing worth noting, I see you're using C++1z with Clang. I hope you're not using anything higher than C++11 when building with GCC, because GCC 4.9's C++14 ABI is not correct (it's still a pre-final ABI that is no longer supported by GCC or by Clang). |
@DanAlbert
As you can see the percentage is the same ~65% ... To test it locally please apply https://paste.kde.org/pmagqlgvf and |
Sorry, the gcc bins are still the same. clang bins shrinked from 201532 to 197432, but still ~65% larger than gcc. |
@bog-dan-ro I am unable to build with Clang based on your instructions. The ARM triple doesn't get passed to the .c files in harfbuzz. If I do configure with '-target ..' in CFLAGS, some files that get compiled with gcc throw an error. |
@pirama-arumuga-nainar did you used ndkr13 for clang? |
@froydnj Just looking at the libxul shared objects takes me down a rabbit hole. I definitely see differences in inlining in a few places. Can you share build instructions or maybe a capture of the build commands for a single module? @bog-dan-ro Here's the list of commands I run:
I get the following error:
All the .c files in harfbuzz are targeted for x86_64. |
@pirama-arumuga-nainar You don't need to clone all the modules, qtbase is enough. I've updated the instructions a few days ago, please check #133 (comment) again. |
@pirama-arumuga-nainar please use 5.6.2 branch instead, it seems we have some problems in 5.7.1 branch (sorry for not checking it earlier). EDIT: clean & rebuild 5.7.1 branch and everything seems ok. |
Firefox has switched to using Clang by default: https://bugzilla.mozilla.org/show_bug.cgi?id=1163171. Regarding code size: https://bugzilla.mozilla.org/show_bug.cgi?id=1163171#c14:
|
Something else to look at: try fixing/reenabling the |
given that firefox has switched, should we move remaining issues to a separate Qt bug? |
The fundamental issues here are still present. Firefox switched despite this issue, not because the issue is fixed aiui. @rprichard it looks like you still have a handful of things mentioned above that you want to investigate. Up to you if you want to split those out into separate bugs and close this one. |
Oh, never mind. Missed the update showing that |
we should probably update https://android.googlesource.com/platform/ndk/+/master/docs/ClangMigration.md too with some of the other tips. (and make it more findable. i have to search for "Clang Migration Notes" rather than "NDK clang migration" to find it, and i never remember that. i usually end up finding it by browsing the source tree :-/ .) actually, maybe we should just have a more general "Things that affect your binary size" page --- much of this stuff (such as |
-Os doesn't really optimize for size. Test: ./checkbuild.py && ./run_tests.py Bug: android/ndk#133 Change-Id: I8d4eb6f57ca88ea1034c9a3fa02419c0134d5aff
#573 is fixed now, so I think we could switch the NDK build systems' defaults back to |
Yeah, I'll do that. |
https://android-review.googlesource.com/c/platform/ndk/+/708632 switched us back to -Oz, so this is fixed in r18? |
I think there's more work we could do here, but maybe we can close it. It's an open-ended bug report.
Edit: When I tested with r18, I accidentally compiled it in a way that statically linked part of libc++ into the Qt binaries, inflating the Clang binary sizes. I'll post updated figures later. |
I measured Qt5 sizes again using the 5.11.1 tag and this patch, which is needed to work with the per-API linker scripts (#672). It seems that diff --git a/mkspecs/android-clang/qmake.conf b/mkspecs/android-clang/qmake.conf
index b665000d00..b136bb0be4 100644
--- a/mkspecs/android-clang/qmake.conf
+++ b/mkspecs/android-clang/qmake.conf
@@ -40,7 +40,8 @@ QMAKE_CFLAGS += -DANDROID_HAS_WSTRING --sysroot=$$NDK_ROOT/sysroot \
ANDROID_SOURCES_CXX_STL_LIBDIR = $$NDK_ROOT/sources/cxx-stl/llvm-libc++/libs/$$ANDROID_TARGET_ARCH
ANDROID_STDCPP_PATH = $$ANDROID_SOURCES_CXX_STL_LIBDIR/libc++_shared.so
-ANDROID_CXX_STL_LIBS = -lc++
+ANDROID_CXX_STL_LIBS = $$ANDROID_SOURCES_CXX_STL_LIBDIR/libc++.so.$$replace(ANDROID_PLATFORM, "android-", "")
+!exists($$ANDROID_CXX_STL_LIBS): ANDROID_CXX_STL_LIBS = $$ANDROID_SOURCES_CXX_STL_LIBDIR/libc++.so
QMAKE_CFLAGS_OPTIMIZE_SIZE = -Oz
I used GCC from r17b and Clang from r18 build 4855205, and I targeted either android-16 or android-21. For Clang, I added API 16:
API 21:
|
I understand the arm64-v8a binaries are larger than armeabi-v7a, but, is it normal they are all between 20% to 30% larger than the v7a version? This is what I'm getting, same code same compilation, yet 20% to 30% size difference per binary. |
armv7 has the https://en.wikipedia.org/wiki/ARM_architecture#Thumb compressed encoding so many common instructions will be 16-bit. armv8 has no compressed encoding so every instruction is 32-bit. |
20-30% is within the realm of what we see for 32-bit vs. 64-bit as well, so this isn't unexpected. |
The only open question remaining here is whether |
Due to android/ndk#133 (comment) and bug 1163171 comment #14, we should use -Oz instead of -Os on Android/clang MozReview-Commit-ID: 1T6fI87sa33 UltraBlame original commit: 29ef8a111035ecbfd1cc50bd2cab3435384ad0b0
Due to android/ndk#133 (comment) and bug 1163171 comment #14, we should use -Oz instead of -Os on Android/clang MozReview-Commit-ID: 1T6fI87sa33 UltraBlame original commit: 29ef8a111035ecbfd1cc50bd2cab3435384ad0b0
Due to android/ndk#133 (comment) and bug 1163171 comment #14, we should use -Oz instead of -Os on Android/clang MozReview-Commit-ID: 1T6fI87sa33 UltraBlame original commit: 29ef8a111035ecbfd1cc50bd2cab3435384ad0b0
We've been working on compiling Firefox for Android with clang lately and today I finally got everything to build with NDK r12 and clang today. All the necessary patches aren't committed yet, so replicating this would be a bit tricky at the moment, but the end result is that our main library,
libxul.so
, compiled with clang is about 10% bigger than when we use gcc (text size, as measured bysize(1)
). Our mobile team is quite concerned about our APK size already, and telling them that a mandated compiler switch would tack on another 3MB+ of size...well, let's just say they wouldn't be happy. 😉This result was pretty surprising, as we were expecting similar performance and even hoping for a codesize win. I have verified that the build flags were identical in both builds (same
-O
options,-fno-exceptions
turned on,-ffunction-sections
+-Wl,--gc-sections
turned on, both builds usinglibc++
, etc.). It is entirely possible that I've missed some key clang option, though I can verify that using-funique-section-names
doesn't change the result.Judging from a quick skim of diff'd
nm
output, the clang build has many more symbols, especially template methods. I can't tell whether this is an effect of inlining heuristics, or of clang somehow keeping those symbols around when they're not actually used.I apologize for not being able to provide replication instructions; I'll try to get all the necessary patches committed soon. We had a workweek last week, and that's been a bit disruptive for getting code reviewed and committed.
Is this sort of difference something that you've seen in your own testing as well, or does this sound like an outlier?
The text was updated successfully, but these errors were encountered: