Disable V8/ICU whole-archive flag, improving file size

[fhanau]

This patches V8 to not use the alwayslink parameter (corresponding to the --whole-archive linker flag on Linux) for several V8 and ICU-related libraries. This reduces binary sizes by ~1MB on Linux and ~19MB on macOS for opt binaries. macOS sees a much larger improvement since bazel uses --ffunction-sections and section garbage collection by default for opt Linux binaries it while we do not yet use the equivalent -dead_strip linker flag on macOS.
I will take a deeper look at V8's code base to confirm this is safe before converting to a non-draft PR.

[ohodson]

The title of the change suggests removing a patch, but that's not in the current draft.

[ohodson]

It would be good to have some data in the commit message, e.g. what is the saving?

Please sync the title with the PR (e.g. remove a patch or fix the title), thanks.

[fhanau]

@ohodson Sorry, I didn't realize GitHub notified reviewers right away on draft PRs. I got some numbers and will update the description later on, it can probably wait until #478 is merged though.

[kentonv]

Do we know why these were marked alwayslink in the first place? Removing alwayslink can break things e.g. if the code depends on startup-time initializers to populate global registries. However, I thought the Chrome project was adverse to startup-time initializers in the first place so it'd be slightly surprising to me if V8 relies on them.

[fhanau]

Was going to look into that a bit more before turning it into a non-draft PR – tcmalloc uses alwayslink and probably depends on it, I believe in V8's case it's be more about preventing linking issues with shared libraries (which we don't use).

[kentonv]

We do use shared libraries in debug builds FWIW.

[fhanau]

Took a deep dive in V8's code base and this should be safe:

Within its primary build system GN, V8 only uses --Wl,-whole-archive when creating shared libraries (https://github.com/chromium/chromium/blob/main/build/toolchain/gcc_toolchain.gni#L507) – this is likely used so that all symbols from a static library added to a shared library are included, preventing possible compile errors due to missing symbols. It is not used when compiling binaries or static libraries, meaning that V8 does not rely on the flag being present to compile its binaries and static libraries. Bazel's alwayslink flag does more than that – if it is added to a cc_library, every target depending on it will import the library with whole-archive. V8 adds alwayslink to all the libraries it compiles, which results in whole-archive being present in more places than needed.
As far as I can see we currently link V8 statically, thus alwayslink won't be needed – if we were to link V8 dynamically in some capacity where we do need whole-archive to not omit symbols this would cause link errors.

Looking at the code itself, there are some instances where dlopen, dlsym, atexit, __attribute__((constructor)) and __attribute__((destructor)) are used, functions which can require whole-archive in some situations. However, these uses are harmless (i.e. using dlsym to load libc symbols, atexit referencing functions in the same file they are called in, destructors in ICU code not compiled in V8). V8 does use and depend on whole-archive when compiling some GTest tests, where test initializers would be omitted otherwise. Note that this change is V8 and ICU specific – tcmalloc depends on whole-archive and is unaffected.

[ohodson]

Thanks Felix, nice write up.

It looks like for workerd we link all the v8 object files directly into the workerd binary (including debug), but this is not the same for ew (which I think Kenton was alluding to when he wrote "We do use shared libraries in debug builds FWIW.")

[fhanau]

Thank you!
I'll look at the internal build too, I focused on workerd first since binary size matters more within wrangler, although the size should be acceptable now.