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.

Sign up for GitHub

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

Jump to bottom

Use cross-compilation where possible #98

Merged
merged 15 commits into from
Jun 23, 2023
Merged

Use cross-compilation where possible #98

merged 15 commits into from
Jun 23, 2023

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Oct 18, 2022 •
edited
Loading

Description of proposed changes

As noted in the PR that introduced multi-platform images, cross-compilation can substantially improve multi-platform build time.

Related issue(s)

Notable discussions

  • Naming of Dockerfile stages (ref)
  • NPM/Auspice cross-platform installation (ref)

Testing

  • Checks pass

  • Verify that binaries on the arm64 image variant are native to the platform. Evidence:

    % docker pull --platform linux/arm64 nextstrain/base:branch-victorlin-arm64-cross-compilation
% docker run -it --entrypoint /bin/bash nextstrain/base:branch-victorlin-arm64-cross-compilation

nextstrain:~/build $ apt-get update && apt-get install file -y

nextstrain:~/build $ file /usr/local/bin/raxmlHPC-PTHREADS-AVX
/usr/local/bin/raxmlHPC-PTHREADS-AVX: ELF 64-bit LSB executable, ARM aarch64, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=a06a245ef61d80661f3c3fd584a4a2002a4568f3, for GNU/Linux 3.7.0, not stripped

nextstrain:~/build $ file /usr/local/bin/FastTreeDblMP
/usr/local/bin/FastTreeDblMP: ELF 64-bit LSB pie executable, ARM aarch64, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=4c9df1efc0725502472822181484324f8aa3be59, for GNU/Linux 3.7.0, not stripped

nextstrain:~/build $ file /usr/local/bin/vcftools
/usr/local/bin/vcftools: ELF 64-bit LSB pie executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=e9b3149b8288e62c72d82f420d423076f22cf490, for GNU/Linux 3.7.0, with debug_info, not stripped

nextstrain:~/build $ node -e "console.log(process.arch)"
arm64

@victorlin victorlin self-assigned this Oct 18, 2022
@victorlin victorlin mentioned this pull request Oct 18, 2022
Merged
8 tasks
@victorlin victorlin force-pushed the victorlin/add-arm64-keep-scripts branch from 29e42a4 to a7f9c1c Compare November 22, 2022 00:01
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch from 3e1ebc8 to 55d8831 Compare November 22, 2022 00:19
@victorlin victorlin force-pushed the victorlin/add-arm64-keep-scripts branch from a7f9c1c to 66a46b9 Compare December 1, 2022 18:31
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch 2 times, most recently from b21001d to 83276a8 Compare December 1, 2022 20:48
@victorlin victorlin force-pushed the victorlin/add-arm64-keep-scripts branch from 66a46b9 to c68440b Compare December 2, 2022 00:31
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch from f06b920 to 7aa8beb Compare December 2, 2022 00:32
@victorlin victorlin force-pushed the victorlin/add-arm64-keep-scripts branch from c68440b to ca169cc Compare December 2, 2022 19:24
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch from 7aa8beb to 64d0c5e Compare December 2, 2022 19:24
@victorlin victorlin force-pushed the victorlin/add-arm64-keep-scripts branch from ca169cc to d4f91c4 Compare December 2, 2022 20:06
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch from 64d0c5e to d26e443 Compare December 2, 2022 20:06
@victorlin victorlin force-pushed the victorlin/add-arm64-keep-scripts branch 2 times, most recently from 3181c5f to 77f1bdf Compare December 6, 2022 19:26
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch from d26e443 to c0b825f Compare December 6, 2022 19:35
@victorlin victorlin force-pushed the victorlin/add-arm64-keep-scripts branch from 77f1bdf to 49ebcf9 Compare December 7, 2022 19:32
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch from c0b825f to 8759edf Compare December 7, 2022 19:44
@victorlin victorlin force-pushed the victorlin/add-arm64-keep-scripts branch 3 times, most recently from 258050a to cd87a3a Compare December 15, 2022 20:47
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch from 8759edf to ce032c4 Compare December 16, 2022 14:32
@victorlin victorlin force-pushed the victorlin/add-arm64-keep-scripts branch from 8cebb0d to 3b53f7a Compare December 16, 2022 14:57
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch 3 times, most recently from a5cc2cd to 924019e Compare December 16, 2022 16:25
Base automatically changed from victorlin/add-arm64-keep-scripts to master December 16, 2022 22:12
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch from 924019e to bd7d1e2 Compare December 20, 2022 21:01
@victorlin victorlin mentioned this pull request Dec 20, 2022
Merged
1 task
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch from a6cc04d to 53cd37b Compare March 1, 2023 00:33
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch 3 times, most recently from 0ff5492 to 21f8283 Compare March 21, 2023 21:22
@victorlin victorlin mentioned this pull request Jun 13, 2023
Merged
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch 3 times, most recently from 7ccea6e to 37e62a9 Compare June 13, 2023 23:25
@victorlin victorlin requested a review from tsibley June 14, 2023 17:54
tsibley
tsibley approved these changes Jun 21, 2023
View reviewed changes
Copy link
Member

@tsibley tsibley left a comment

Choose a reason for hiding this comment

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

On the whole, this looks good to me!

I haven't closely tested the final image produced, but trust your testing and CI here (esp. given recent CI improvements).

Dockerfile Outdated Show resolved Hide resolved
devel/build Outdated Show resolved Hide resolved
victorlin added 12 commits June 21, 2023 15:51
@victorlin
Prepare for cross-compilation
c72fccd 
Split the builder stage into two stages, and install xx¹ which provides
tools to support cross-compilation.

Note that the first stage doesn't need Python so debian:11-slim is used
as the base image.

There is additional work to be done on individual commands to have
proper cross-compilation, but those fixes will come in subsequent
commits to keep them atomic.

¹ https://github.com/tonistiigi/xx
@victorlin
Dockerfile: Add gcc to build datrie
24d025f 
This is required since datrie only provies wheels up through Python
3.8¹.

¹ https://pypi.org/project/datrie/0.8.2/#files
@victorlin
Replace build-essential with make, gcc, libc6-dev
0880c09 
build-essential should not be installed for cross-compilation since it
contains native-only compilers. Instead, target-specific compilers
provided by xx (e.g. aarch64-linux-gnu-gcc) should be used.

Add dependencies of build-essential that are still needed in separate
stages.
@victorlin
Install native clang
e7ad175 
xx-clang internally calls the native clang binary with additional
configuration for correct cross-compilation¹. This means it should be
installed.

¹ https://github.com/tonistiigi/xx/blob/dad71a2d84fa9f1321ad3b91e1f36e228fb31876/README.md#cc
@victorlin
Install zlib for target architecture
91e0544 
This is how it's done in the xx examples¹. Prevents the VCFtools build
from breaking.

¹ https://github.com/tonistiigi/xx/blob/dad71a2d84fa9f1321ad3b91e1f36e228fb31876/README.md#xx-apk-xx-apt-xx-apt-get---installing-packages-for-target-architecture
@victorlin
Fix RAxML build
3b269c3 
RAxML's Makefile, which was patched previously for proper arm64 builds,
needs another patch in order to cross-compile. The compiler is hardcoded
as gcc, but it should be a target-specific compiler. $(xx-info)-gcc can
be used, but xx-clang is recommended¹.

¹ https://github.com/tonistiigi/xx/blob/dad71a2d84fa9f1321ad3b91e1f36e228fb31876/README.md#cc
@victorlin
Fix FastTree build
a22625a 
FastTree's custom Makefile supports cross-compiling using a
target-specific compiler. xx-clang is recommended¹, but it fails with a
linker error:

    xx-clang -O3 -finline-functions -funroll-loops -Wall -DUSE_DOUBLE -DOPENMP -fopenmp -o FastTreeDblMP FastTree.c -lm
    /usr/bin/aarch64-linux-gnu-ld: cannot find -lomp
    clang: error: linker command failed with exit code 1

$(xx-info)-gcc is able to compile the program successfully.

¹ https://github.com/tonistiigi/xx/blob/dad71a2d84fa9f1321ad3b91e1f36e228fb31876/README.md#cc
@victorlin
Fix VCFtools build: Use --build and --host
adf0cb7 
The xx guide says --host should be sufficient¹, but Autotools did not
detect cross-compilation until --build was added.

¹ https://github.com/tonistiigi/xx/blob/dad71a2d84fa9f1321ad3b91e1f36e228fb31876/README.md#autotools
@victorlin
Fix VCFtools build: Install dpkg-dev
04dd229 
Install dpkg-dev to use pkg-config when cross-building. Otherwise, the
configure script output shows:

    checking pkg-config is at least version 0.9.0... Please install dpkg-dev to use pkg-config when cross-building
    no
@victorlin
Fix VCFtools build: Install g++
6ba9476 
Without this or an equivalent, native c++ is used. That is undesirable
when the target platform is different from the build platform.
@victorlin
Fix VCFtools build: Build from an unreleased commit
cc6d7df 
See changes for reasoning.
@victorlin
Update comment on Auspice install
b78680c 
Remove the comment on build time since it may differ nowadays, and there
is another reason for not caching (unpinned status, implied by other
context in the Dockerfile).
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch 2 times, most recently from fcfc6c7 to 43f57d7 Compare June 22, 2023 18:45
tsibley
tsibley approved these changes Jun 22, 2023
View reviewed changes
Copy link
Member

@tsibley tsibley left a comment

Choose a reason for hiding this comment

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

This seems good to go to me once CI passes.

I'll be curious to see build times before/after…

victorlin added 3 commits June 22, 2023 16:08
@victorlin
Add comment on Auspice cross-compilation
deabcc1 
Pure JavaScript packages (including Auspice) are not platform-specific.

Evidence that the arm64 image variant of build-20230621T190343Z (latest
tagged build) does not have any platform-specific Auspice runtime
dependencies:

    $ apt-get update && apt-get install file -y
    $ find /nextstrain/auspice/node_modules -executable -type f -exec file {} + | grep 64 | sed -e 's=^/nextstrain/auspice/node_modules/=='

    node-notifier/vendor/mac.noindex/terminal-notifier.app/Contents/MacOS/terminal-notifier:                        Mach-O 64-bit x86_64 executable, flags:<NOUNDEFS|DYLDLINK|TWOLEVEL|PIE>
    node-notifier/vendor/notifu/notifu64.exe:                                                                       PE32+ executable (GUI) x86-64, for MS Windows
    puppeteer/.local-chromium/linux-722234/chrome-linux/nacl_irt_x86_64.nexe:                                       ELF 64-bit LSB executable, x86-64, version 1 (SYSV), statically linked, BuildID[sha1]=7aeb4f45ea5cec7d8e4184264ad39f0f77bcaee2, stripped
    puppeteer/.local-chromium/linux-722234/chrome-linux/nacl_helper_bootstrap:                                      ELF 64-bit LSB executable, x86-64, version 1 (SYSV), statically linked, BuildID[sha1]=5c5f3935c8f8a15ba325f0d73dfa585fa9390cf9, not stripped
    puppeteer/.local-chromium/linux-722234/chrome-linux/ClearKeyCdm/_platform_specific/linux_x64/libclearkeycdm.so: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, not stripped
    puppeteer/.local-chromium/linux-722234/chrome-linux/libEGL.so:                                                  ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, not stripped
    puppeteer/.local-chromium/linux-722234/chrome-linux/crashpad_handler:                                           ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, not stripped
    puppeteer/.local-chromium/linux-722234/chrome-linux/chrome_sandbox:                                             ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, not stripped
    puppeteer/.local-chromium/linux-722234/chrome-linux/chrome:                                                     ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, not stripped
    puppeteer/.local-chromium/linux-722234/chrome-linux/libGLESv2.so:                                               ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, not stripped
    puppeteer/.local-chromium/linux-722234/chrome-linux/swiftshader/libEGL.so:                                      ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, not stripped
    puppeteer/.local-chromium/linux-722234/chrome-linux/swiftshader/libGLESv2.so:                                   ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, not stripped
    puppeteer/.local-chromium/linux-722234/chrome-linux/nacl_helper:                                                ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, not stripped
@victorlin
Update scripts to reflect 3-stage build
cb55570 
Apart from this commit, it was necessary to create 2 new repositories in
the nextstrain Docker Hub organization.
@victorlin
Update README for cross-compilation
d6f39af
@victorlin victorlin force-pushed the victorlin/arm64-cross-compilation branch from 43f57d7 to d6f39af Compare June 22, 2023 23:23
@victorlin victorlin merged commit 86dea43 into master Jun 23, 2023
@victorlin victorlin deleted the victorlin/arm64-cross-compilation branch June 23, 2023 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsibley tsibley tsibley approved these changes

Assignees

@victorlin victorlin

Labels
None yet
Projects
No open projects
Nextstrain planning (archived)
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@victorlin @tsibley