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

chore: speedup docker build time #6787

Draft
wants to merge 7 commits into
base: unstable
Choose a base branch
from

Conversation

jeluard
Copy link
Member

@jeluard jeluard commented May 15, 2024

Motivation

Speedup docker builds

Description

Tweak current Docker build process to improve build speeds.

This is achieved by:

  • using regular node images for build layers; this remove the need to add extra tooling via apk every run
  • document how to use yarn offline mirror to reduce the need to download dependencies

Using regular node images comes at a higher download cost (image is bigger) but is cached. slim images do not ship with the necessary tooling.

Note that the current Docker build creates a multi-platform Docker image (arm64 and amd64). We can probably improve things further by having a dedicated Dockerfile for dev usage.

Results

Before

1st run: 5m 22s
Subsequent runs, after code change: 4m 52s

After

1st run: 3m 16s
Subsequent runs, after code change: 2m 32s

@jeluard jeluard requested a review from a team as a code owner May 15, 2024 16:04
@jeluard jeluard mentioned this pull request May 15, 2024
Copy link

codecov bot commented May 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.88%. Comparing base (8e875c6) to head (557a0b0).
Report is 8 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #6787      +/-   ##
============================================
- Coverage     61.88%   61.88%   -0.01%     
============================================
  Files           562      562              
  Lines         59309    59331      +22     
  Branches       1916     1916              
============================================
+ Hits          36703    36715      +12     
- Misses        22563    22573      +10     
  Partials         43       43              

Dockerfile Outdated Show resolved Hide resolved
@jeluard jeluard marked this pull request as draft May 16, 2024 07:02
Copy link
Member

@nflaig nflaig left a comment

Choose a reason for hiding this comment

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

need to confirm this does not increase the image size

Dockerfile Outdated Show resolved Hide resolved
@jeluard
Copy link
Member Author

jeluard commented May 17, 2024

I added some experiment using a lodestar base builder image. This reduce the need to either use a big image or update via apk every build.
This image will have to be pushed to ChainSafe dockerhub beforehand.

@@ -0,0 +1,2 @@
FROM node:20-alpine
RUN apk update && apk add --no-cache g++ make python3 && rm -rf /var/cache/apk/*
Copy link
Member

Choose a reason for hiding this comment

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

What was the argument against using the node:20 as builder image?

Copy link
Member Author

Choose a reason for hiding this comment

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

For some unclear reason it fails to build native libs in the second stage. Might be the original reason for using alpine.
Also this one would be smaller and offers more opportunity for further tweaks down the road.

Copy link
Member

Choose a reason for hiding this comment

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

I am really not sure it's worth to have a separate builder image just for these dependencies as the layer will always be retrieved from local cache after the first local build, and even if it is executed it just took 11 seconds on my machine.

The issues I see with using a separate builder image is that right now we automatically use the latest 20.x version which is good to get security updates in. There is also extra maintenance to keep this up-to-date if it's not part of the CI. Although we could build this image before running the main docker build.


```bash
yarn config set yarn-offline-mirror .yarn/yarn-offline-mirror
mv ~/.yarnrc ./ # `yarn config` commands are always global, make `.yarnrc` specific to this project
Copy link
Member

@nflaig nflaig May 21, 2024

Choose a reason for hiding this comment

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

What if someone has settings in their global yarn config? can't we just commit a .yarnrc to the project with this setting in it?

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.

None yet

3 participants