Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow shared linkage for V8 #809

Merged
merged 1 commit into from
Jul 15, 2023
Merged

Allow shared linkage for V8 #809

merged 1 commit into from
Jul 15, 2023

Conversation

fhanau
Copy link
Collaborator

@fhanau fhanau commented Jun 26, 2023

For linking tests, we currently use dynamic linkage, except for V8, which has dynamic linkage disabled. The provided V8 patch shifts a snapshot related file around to make dynamic linkage possible. This dramatically reduces test binary sizes – //src/workerd/api:crypto-impl-test shrinks from ~100MB to 5.8MB.
Note that the mac toolchain does not enable dynamic linkage, so the change is having no effect there.

There is no noticeable speedup in CI, Perhaps this will improve when the remote cache is primed and runners can fetch the (significantly smaller) test binaries from there. For the Linux runner at least, disk usage compared to a build on the main branch goes from 11G => 7.9G for the disk cache and 7.4G => 4.5G for the output directory.

This should be more beneficial for the local development on Linux when running and recompiling tests: Dynamic linking all workerd tests currently takes 8s on devspace, using the patch reduces this to 1.3s.

@fhanau fhanau force-pushed the felix/bazel-v8-shared branch from fe7d985 to 144a0f2 Compare June 26, 2023 16:16
@fhanau fhanau force-pushed the felix/bazel-v8-shared branch 4 times, most recently from 0c0a6c9 to 90dea13 Compare July 11, 2023 15:38
@fhanau fhanau changed the title [WIP] Allow shared linkage for most of V8 [WIP] Allow shared linkage for V8 Jul 11, 2023
@fhanau fhanau changed the title [WIP] Allow shared linkage for V8 Allow shared linkage for V8 Jul 11, 2023
@fhanau fhanau marked this pull request as ready for review July 11, 2023 23:14
@fhanau fhanau requested review from mikea and ohodson July 11, 2023 23:14
Copy link
Contributor

@ohodson ohodson left a comment

Choose a reason for hiding this comment

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

Nice going!

@fhanau fhanau force-pushed the felix/bazel-v8-shared branch 5 times, most recently from 0e0b62f to 69b9d21 Compare July 15, 2023 17:50
@fhanau fhanau force-pushed the felix/bazel-v8-shared branch from 69b9d21 to 729868f Compare July 15, 2023 19:01
@fhanau fhanau merged commit 056517f into main Jul 15, 2023
@fhanau fhanau deleted the felix/bazel-v8-shared branch July 15, 2023 19:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants