[FR] remove superfluous LLVM libraries

[ur4t]

Description

I am curious about how LLVM toolchain distributed with NDK are built and find some potential improvements in current versions (r26.2/r487747e and r28/r522817):

1. Plugin support in Clang was disabled to reduce binary size, but...
2. libclang.so, libclang-cpp.so and libLLVM-17.so are built, beacuse LLVM_BUILD_LLVM_DYLIB is ON since the build scripts are refactored, and...
3. Those shared libraries are not linked to any binaries because LLVM_LINK_LLVM_DYLIB is not set (OFF by default since introduced by https://github.com/llvm/llvm-project/commit/9211396d825dc90e1f1c5d33941853a037b7f842).
4. Currently the build scripts perform BOLT themselves, while upstream CMake has added BOLT support (https://github.com/llvm/llvm-project/commit/3dab7fede2019c399d793c43ca9ea5a4f2d5031f and https://github.com/llvm/llvm-project/commit/076240fa062415b6470b79413559aff2bf5bf208).

By the way, I find LLVM_ENABLE_THREADS is explicitly defined as ON, which is default on most platforms supported by LLVM.

Those are not bugs because functionalities are not broken, so this issue is labeled with "enhancement". As for "plugin support in Clang", it would be nice to compile and load out-of-tree passes independently rather than compile the whole modified LLVM toolchain.

[pirama-arumuga-nainar]

Description

1. Plugin support in Clang was disabled to reduce binary size, but...
2. libclang.so, libclang-cpp.so and libLLVM-17.so are built, beacuse LLVM_BUILD_LLVM_DYLIB is ON since the build scripts are refactored, and...

Thanks for flagging this. The libclang and libLLVM libraries are used by clang-based tools used in Android platform build. @DanAlbert They can be removed from the ndk.

4. Currently the build scripts perform BOLT themselves, while upstream CMake has added BOLT support (llvm/llvm-project@3dab7fe and llvm/llvm-project@076240f).

This is by design. AOSP clang collects PGO and BOLT profiles when building Android so all backends get exercised and profiled. The CMake support only builds for the host architecture.

By the way, I find LLVM_ENABLE_THREADS is explicitly defined as ON, which is default on most platforms supported by LLVM.

https://r.android.com/3037873

As for "plugin support in Clang", it would be nice to compile and load out-of-tree passes independently rather than compile the whole modified LLVM toolchain.

For now, we plan to leave plugin support off considering the significant size reduction and build speedup, esp. for users on old hardware.

[DanAlbert]

https://android-review.googlesource.com/c/platform/ndk/+/3038133. Will cherry-pick to r27 since that also shaves about half a GB off the install size. We had some code that stripped those (the toolchain you give me has those unstripped for whatever reason), but they either moved or whoever wrote that code in the first place didn't test it, because it was stripping the wrong location :(

[pirama-arumuga-nainar]

I am not sure why the libraries are not stripped. Filed internal bug b/333762307 to track this down.

[ur4t]

@pirama-arumuga-nainar @DanAlbert Thanks for your kindly response introducing more NDK design perspectives!

FYI, when LLVM_LINK_LLVM_DYLIB is enabled, we have a dependency list:

components	dependencies
clang*	libclang-cpp.so and libLLVM.so
llvm* lld	libLLVM.so
lldb	liblldb.so
libclang.so liblldb.so	libclang-cpp.so and libLLVM.so
libclang-cpp.so	libLLVM.so

That means currently those bloated binaries and libraries, whose size are around 100MB and more, contain multiple copies of (at least part of) libclang-cpp.so and libLLVM.so.

With LLVM_LINK_LLVM_DYLIB enabled, we have:

• Most binaries, libLTO.so and libclang.so below 1MB;
• liblldb.so, clangd and clang-tidy around 20MB;
• libclang-cpp.so around 70MB.

So I believe enabling LLVM_LINK_LLVM_DYLIB is able to introduce more size reduction and build speedup (no more redundant static linking), which makes it negligible to enable plugin support in clang.

The problem is whether optimizers, especially BOLT, work with dynamic libraries. I have noticed that both upstream CMake and our build scripts only perform BOLT over the clang binary itself. Is it possible to perform BOLT over underlying libraries as well?

[DanAlbert]

A change in that policy would be up to @pirama-arumuga-nainar, as I wouldn't be the one paying the maintenance costs for it.

Closing as we've cleaned up the misleading mess, and there's currently no plan to add plugin support. If the toolchain team decides they want to take on the costs of plugin support, we'll get an FR filed to track it (or it'll just show up in the changelog).

[saurik]

I am somewhat concerned and certainly disappointed with this change in the NDK, as someone who remembers being really happy to discover back a long time ago that you were finally shipping libclang. FTR: Apple ships libclang with Xcode, and Microsoft ships it with their Visual Studio distributions of LLVM. It has been great having everyone on board.

To me, having these libraries are less about being able to write plug-ins, and more about being able to have parts of my tooling not just run clang but also parse C code using "the same" (having the same quirks and patches) libclang for analysis and automated code generation, such as to generate bridges to other languages (in my case, JavaScript).

FWIW, after reading this issue yesterday and having that concern, but then just kind of going on with my life and thinking I'd write about this later, without even looking for it I ran into a concrete scenario that might disappoint other people: Rust's bindgen, which uses the clang-sys crate, which attempts to autodetect libclang's path from the toolchain.

[DanAlbert]

FWIW, I completely agree. I'm not thrilled with this state of affairs either, but we're very short handed and have to pick our battles. This isn't something we can afford to support. Hopefully that will change some day.

Rust's bindgen, which uses the clang-sys crate, which attempts to autodetect libclang's path from the toolchain.

That's potentially helpful ammo for us getting this support down the road. Rust isn't supported for apps right now, but it seems inevitable that that'll change sooner or later, and if Rust needs libclang it's not really optional.

[saurik]

@DanAlbert It just feels like you went out of your way to break this just because someone random suggested deleting the libraries... even though that same user then noted that that wasn't even the reasonable way to free up disk space after they thought about it longer (as, in fact, all of your disk space is being wasted due to how you statically link everything, and it is still being wasted now that you deleted the libraries). Like, you say you can't afford to support it, and maybe you could support it later, as if shipping libclang requires a lot of work, but you just went through active work to stop shipping it :/.

[DanAlbert]

We only went "out of our way" to do what a user asked :( Deleting files is pretty easy. Supporting libclang is not (shipping an unsupported libclang isn't hard, no, but that's not what support is, and this bug was someone asking us to remove unsupported stuff to free up space).

The size of the NDK is a common complaint, so we do act on it when people find problems. Yes, there are potentially ways we could improve on it even more. I don't recall the reason for the toolchain being statically linked, but for portability reasons (the NDK only works on Linux distros that are sufficiently like Ubuntu because we depend on glibc from the host, which excludes a lot of popular dockerfile base images) we're likely to go further in that direction rather than reducing the static linking.

If you want libclang support, please file an FR and we'll see what we can do.

[DanAlbert]

(filed https://github.com/android/ndk/issues/2046)