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

Optimize docker build, pin rust version, use debian for runtime #23

Merged
merged 3 commits into from Mar 26, 2024

Conversation

yomguy
Copy link
Contributor

@yomguy yomguy commented Mar 25, 2024

This is a short PR based on master to :

  • pin rust v1.76 (always better for sustainability)
  • use last debian version (buster LTS will end on july 24)
  • use debian for both builder and runtime (symetrical env is even better)
  • install debian deps packages before copying app (speedup build time)

@yomguy yomguy changed the title Optimize docker build, pin rust version, use debian as for builder Optimize docker build, pin rust version, use debian for builder Mar 25, 2024
@yomguy yomguy changed the title Optimize docker build, pin rust version, use debian for builder Optimize docker build, pin rust version, use debian for runtime Mar 25, 2024
@SailorSnoW
Copy link
Contributor

Thanks for the PR,

pin rust v1.76 (always better for sustainability)

Since the polkadot-v1.9.0, we are using nightly-2024-01-21 Rust channel because the new construct runtime V2 is using experimental features only available on a nightly channel. Can you adapt the docker rust version to it then ?

install debian deps packages before copying app (speedup build time)

Do you know why or have any source about this ?

@SailorSnoW SailorSnoW added optimization Issues to make the performances and code better P2 Normal priority, kinda important but not required immediately docker labels Mar 26, 2024
@yomguy
Copy link
Contributor Author

yomguy commented Mar 26, 2024

Thanks for the PR,

Welcome

pin rust v1.76 (always better for sustainability)

Since the polkadot-v1.9.0, we are using nightly-2024-01-21 Rust channel because the new construct runtime V2 is using experimental features only available on a nightly channel. Can you adapt the docker rust version to it then ?

nightly-2014-01-21 seems not available directly through a docker image but the latest nightly or stable version like 1.77 released on 21/03. Do you think this 1.77 could suits your needs, embedding what nightly-2014-01-21 has?

If not, we could compile nightly-2014-01-21 during the build...

install debian deps packages before copying app (speedup build time)

Do you know why or have any source about this ?

My long experience on docker packaging is that each task in Dockefile is automatically cached as long as the running command does not modifying anything or if any previous command is already cached. So, the command copying the code base ("ADD . ." or "COPY . .") on a development branch should be done at the latest possible stage so that everything before can be (ideally) cached. This is explained in the official doc.

In the case of the Allfeat package, I have pretty long building time (around 10' on 16 CPU cloud server) because it embeds a lot of dependencies. IMHO a lot of dependency installations could be cached. This is why I'm trying to find a way to have more stages to be sure the code base is compiled after all deps. It seems using cargo-chef is the better approach for now, waiting the feature can be used natively using something like "cargo build --dependencies-only". ATM, from my tests, cargo-chef does not give nothing better, doubling the build time (!). Indeed, it seems the builder does not see the previous deps compiled by chef.

So continuing my experiments to drastically reduce the build time... ;)

@SailorSnoW
Copy link
Contributor

nightly-2014-01-21 seems not available directly through a docker image but the latest nightly or stable version like 1.77 released on 21/03. Do you think this 1.77 could suits your needs, embedding what nightly-2014-01-21 has?

If we could have a 1.79 nightly docker image I think that would work fine, you can test it by just changing the rust version in the rust-toolchain.toml to adapt to the rust docker image also.

Reducing the build time is challenging for a Substrate blockchain yeah.
Thanks for the explanation that make sense.

I have a bad news about this:
We should try to build Allfeat in production profile now by passing to cargo --profile=production.
Production profile make the build being longer and slower but it produce the most optimized binary so that mean the fastest node and runtime execution, and this is kinda important for a blockchain.

It will be important for users to provide a docker image with Allfeat built directly from a CI also in the future.

@SailorSnoW
Copy link
Contributor

LGTM

@SailorSnoW SailorSnoW merged commit d5afb5d into Allfeat:develop Mar 26, 2024
SailorSnoW pushed a commit that referenced this pull request Mar 26, 2024
* use pinned debian bookworm based image, install debian deps before copying

* add before defining workdir

* use nightly-bookworm-slim for now
@yomguy
Copy link
Contributor Author

yomguy commented Mar 27, 2024

--profile=production

OK of course

It will be important for users to provide a docker image with Allfeat built directly from a CI also in the future.

Indeed, we could use hub.docker to build the image automatically. But even on this platform, a shorter build time is always better ;)

Thanks for the merge.

@yomguy
Copy link
Contributor Author

yomguy commented Mar 27, 2024

FYI @SailorSnoW using --production returns an error here:

29.22 error: unexpected argument '--production' found                                                   
29.22                                                                                                   
29.22 Usage: cargo build --locked --release                                                             
29.22                                                                                                   
29.22 For more information, try '--help'.                                                               
------                                                                                                  
Dockerfile:56                                                                                           
--------------------                                                                                    
  54 |     FROM cacher AS builder                                                                       
  55 |     COPY . .                                                                                     
  56 | >>> RUN --mount=type=cache,mode=0755,target=/app/target cargo build --locked --release --production                                                                                                        
  57 |     RUN --mount=type=cache,mode=0755,target=/app/target cp /app/target/release/allfeat /usr/local/bin                                                                                                      
  58 |                                                                                                                                                                                                            
--------------------                                                                                                                                                                                              
ERROR: failed to solve: process "/bin/sh -c cargo build --locked --release --production" did not complete successfully: exit code: 1  

Would it be exclusive to another argument?

@SailorSnoW
Copy link
Contributor

FYI @SailorSnoW using --production returns an error here:

29.22 error: unexpected argument '--production' found                                                   
29.22                                                                                                   
29.22 Usage: cargo build --locked --release                                                             
29.22                                                                                                   
29.22 For more information, try '--help'.                                                               
------                                                                                                  
Dockerfile:56                                                                                           
--------------------                                                                                    
  54 |     FROM cacher AS builder                                                                       
  55 |     COPY . .                                                                                     
  56 | >>> RUN --mount=type=cache,mode=0755,target=/app/target cargo build --locked --release --production                                                                                                        
  57 |     RUN --mount=type=cache,mode=0755,target=/app/target cp /app/target/release/allfeat /usr/local/bin                                                                                                      
  58 |                                                                                                                                                                                                            
--------------------                                                                                                                                                                                              
ERROR: failed to solve: process "/bin/sh -c cargo build --locked --release --production" did not complete successfully: exit code: 1  

Would it be exclusive to another argument?

The correct syntax to pass the production profile to cargo is this one normally:
cargo build --profile=production --locked as this is a profile name created in the Cargo.toml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker optimization Issues to make the performances and code better P2 Normal priority, kinda important but not required immediately
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants