Skip to content
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

llvm.org: reduce installed size #2562

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

jonchang
Copy link
Contributor

This may help shrink binary sizes by avoiding static linking.

@what-the-diff
Copy link

what-the-diff bot commented Jul 11, 2023

PR Summary

  • Enhancement to the Build Process
    A new flag, -DLLVM_LINK_LLVM_DYLIB=ON, has been added to the build section in our configuration file (package.yml). This modification will allow the building process to incorporate LLVM's dynamic libraries, ensuring our application is always on par with latest features and improvements in LLVM.

@jonchang
Copy link
Contributor Author

On darwin+aarch64 it looks like this significantly decreases the size of the unpacked artifact:

before: 3.1G
after: 766M

Will dig into the Linux failures, which I believe can be addressed by tweaking some options.

@mxcl
Copy link
Member

mxcl commented Jul 18, 2023

I tested locally and linux passes if we remove the other linux specific ARGS.

$ du -hd1 prefixes/linux/llvm.org/v16/
169M	prefixes/linux/llvm.org/v16//bin
 44K	prefixes/linux/llvm.org/v16//libexec
 46M	prefixes/linux/llvm.org/v16//include
683M	prefixes/linux/llvm.org/v16//lib
180K	prefixes/linux/llvm.org/v16//share
899M	prefixes/linux/llvm.org/v16/

@jonchang
Copy link
Contributor Author

Nice work! (I still haven't gotten tea set up on my Linux box yet.) The only thing left to do is to consider whether this change is desirable. My understanding is that this will lead to a small performance penalty for compiling with LLVM, which is maybe not such a big deal but at the margins (e.g., compiling Firefox or Chrome) would be an issue.

@mxcl
Copy link
Member

mxcl commented Jul 19, 2023

(I still haven't gotten tea set up on my Linux box yet.

pkg -L build llvm.org in your pantry clone runs it via docker in case you weren't aware of this feature.

@mxcl
Copy link
Member

mxcl commented Jul 19, 2023

if we remove the other linux specific ARGS.

this was ofc just a test to see if it helped, those ARGS were added by @jhheider for a reason presumably.

@mxcl
Copy link
Member

mxcl commented Jul 19, 2023

-- Linker detection: unknown
CMake Error at /tmp/eb4c19d9/llvm/cmake/modules/CheckProblematicConfigurations.cmake:14 (if):
  if given arguments:

    "STREQUAL" "MSVC"

  Unknown arguments specified
Call Stack (most recent call first):
  /tmp/eb4c19d9/llvm/cmake/modules/HandleLLVMOptions.cmake:10 (include)
-- Configuring incomplete, errors occurred!
  CMakeLists.txt:157 (include)

I got this once too, but only once. Worked a second time after cleaning. So… that's weird.

edit: no I'm wrong it only builds if we remove the if from the top of the build script thus removing compiler-rt from the runtime list. And I assume we need that for some reason.

@jonchang
Copy link
Contributor Author

jonchang commented Jul 19, 2023

I suspect that some of the build opts conflict between building llvm/clang/lldb and compiler-rt. I'm testing something out now to spin off compiler-rt into compiler-rt.llvm.org (like we already do with openmp.llvm.org).

My preference would actually be to try to split these packages up fully (such that we have an lldb.llvm.org and clang.llvm.org, also should help reduce build sizes) but since there's a fair number of things that already depend on llvm.org I'll hold off on that.

@jhheider
Copy link
Contributor

  1. complier-rt was asked for here
  2. splitting it all up seems like a great idea
  3. you can easily keep things that depend on llvm.org working by making llvm.org depend on the things you split out (i'm sure you know this; this is the meta-package idea to help ease transition)

@jonchang
Copy link
Contributor Author

all good points! forgot about meta-packages / package groups, will give it a shot

@jonchang jonchang changed the title llvm.org: prefer dylibs llvm.org: reduce installed size Jul 19, 2023
@jonchang
Copy link
Contributor Author

jonchang commented Jul 26, 2023

Currently building/testing clang and lld but here's the current plan:

  • llvm.org/core will contain the "core" LLVM utilities and libraries
  • clang.llvm.org has clang, which depends on compiler-rt.llvm.org and llvm.org/core
  • lld.llvm.org has lld
  • The llvm.org metapackage will point to clang, lld, compiler-rt, and core
  • Additional LLVM subprojects and fixing old dependencies on llvm.org to the appropriate subprojects

@jhheider
Copy link
Contributor

Hm. That sounds amazing. We might have a version-locking issue. @mxcl we could handle clang.llvm.org: ${{version}} moustaches in dependencies: but that'd just ruin the resolution algo. Thoughts there?

@jonchang
Copy link
Contributor Author

We could just have everything that currently depends on llvm.org to instead depend on llvm.org/core plus clang.llvm.org (I doubt anything actually used lld). That way there would be nothing in pantry that directly depended upon llvm.org and thus avoiding problems with dependency resolution.

@jhheider
Copy link
Contributor

jhheider commented Jul 26, 2023

That's likely the right move. We could, in tealib make llvm.org an alias for the other two to ease adoption. and not trigger a rebuild of 98% of the pantry which I need to remember to cancel before i hate everything.

which might mean renaming the old package and keeping it on hand, in case it's needed. @ABevier would hate us for that, and rightly. this change is going to require precision.

@mxcl
Copy link
Member

mxcl commented Jul 26, 2023

seems like a very well thought through organization! great work. Will this pkg become llvm.org/core?

@jonchang
Copy link
Contributor Author

Yes, currently testing to see if all the pieces work well together on darwin+linux. Didn't want to push yet to avoid tying up CI for something that might not work

@jonchang
Copy link
Contributor Author

jonchang commented Jul 27, 2023

Okay, figured clang out, it was a bug in llvm's build system. Testing now...

@jonchang
Copy link
Contributor Author

jonchang commented Jul 31, 2023

This is getting pretty close to done, the package yamls are close to their final state. I have tested this on macOS arm64, and am in the process of doing the same for linux intel and arm. I need to come up with a better test for compiler-rt and package some other llvm subprojects (and take a look at openmp since that's already packaged separately)

env:
ARGS:
- -Wl,-rpath,$TEA_PREFIX
build: mkdir -p {{prefix}} && touch {{prefix}}/.tea-empty
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

time to support no src no build pkgs probs! what you think @jhheider

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meta packages are a thing. We could probably use them just in resolution. If they don't have a build: key we can assume they're just dependency bundles.

The pantry workflow just needs to yeet them through.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants