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

Greatly increased compile times when upgrading from grpcio:0.9 to grpcio:10.2 #578

Open
purew opened this issue Jun 17, 2022 · 19 comments
Open

Comments

@purew
Copy link

purew commented Jun 17, 2022

The rest of the build utilizes sccache https://github.com/mozilla/sccache/ for caching of build-artifacts between builds.

However, it does not seem like the grpc-C++ library makes use of sccache. Is there any way of setting the Cmake option -DCMAKE_CXX_COMPILER_LAUNCHER=sccache which another part of the project uses for the C++ dependency?

@BusyJay
Copy link
Member

BusyJay commented Jun 17, 2022

You can enable it by environment variables env CMAKE_CXX_COMPILER_LAUNCHER=sccache cargo build.

@purew
Copy link
Author

purew commented Jun 21, 2022

Are you sure CMAKE_CXX_COMPILER_LAUNCHER makes it through the grpcio build.rs script to the underlying Cmake invocation?

I can't seem to find that this makes a difference in build time or sccache --show-stats.

@BusyJay
Copy link
Member

BusyJay commented Jun 21, 2022

You can check the logs in target directory to see what launcher it's using. For example, grep sccache -R target/debug/build/grpcio-sys-{hash}/out/. Or just check the process list during compilation, it should show sccache is used.

@BusyJay
Copy link
Member

BusyJay commented Jun 21, 2022

To reduce compile time (and the size of target directory), TiKV disable debug info from grpcio in debug mode. The side affect is you won't be able to set a breakpoint or step into the grpcio.

@purew
Copy link
Author

purew commented Jun 21, 2022

Are you using sccache yourself successfully to cache the C++ library?

@BusyJay
Copy link
Member

BusyJay commented Jun 21, 2022

No, we don't use sccache. We do use cache in CI.

@purew
Copy link
Author

purew commented Feb 17, 2023

I'm just coming back to this again. Have you considered instructing cc to build the c++ codebase in parallel like so:

cc = { version = "1.0", features = ["parallel"] }

from https://docs.rs/cc/1.0.79/cc/#parallelism

Building the codebase with a single worker takes a looooong time.

@BusyJay
Copy link
Member

BusyJay commented Feb 20, 2023

Interesting, never used this feature, but I think it worth a try. Note you can also enabling the feature without changing grpcio by including cc as a dependency of your project and enabling the feature. The feature will then also be used by grpc-rs. If there is any improvement, please let me know.

@Ten0
Copy link
Contributor

Ten0 commented Feb 20, 2023

The issue appears to come from the way we build gRPC Core. It is also not reproducible on all platforms (happens on Arch but not in an Ubuntu docker for instance). It's been a bother for us since ~ mid January.
When I build it on Arch with verbose (cargo check -vv), I get the following message:

...
[grpcio-sys 0.12.1+1.46.5-patched] -- Found OpenSSL: /usr/lib/libcrypto.so (found version "3.0.8")  
[grpcio-sys 0.12.1+1.46.5-patched] -- Found ZLIB: /path/to/target/debug/build/libz-sys-1bf928a372e0ff96/out/lib/libz.a (found version "1.2.11") 
[grpcio-sys 0.12.1+1.46.5-patched] -- Configuring done
[grpcio-sys 0.12.1+1.46.5-patched] -- Generating done
[grpcio-sys 0.12.1+1.46.5-patched] CMake Warning:
[grpcio-sys 0.12.1+1.46.5-patched]   Manually-specified variables were not used by the project:
[grpcio-sys 0.12.1+1.46.5-patched] 
[grpcio-sys 0.12.1+1.46.5-patched]     CMAKE_ASM_COMPILER
[grpcio-sys 0.12.1+1.46.5-patched]     CMAKE_ASM_FLAGS
[grpcio-sys 0.12.1+1.46.5-patched] 
[grpcio-sys 0.12.1+1.46.5-patched] 
[grpcio-sys 0.12.1+1.46.5-patched] -- Build files have been written to: /path/to/target/debug/build/grpcio-sys-5fa980c5c66c1f57/out/build
[grpcio-sys 0.12.1+1.46.5-patched] running: "cmake" "--build" "." "--target" "grpc" "--config" "RelWithDebInfo"
[grpcio-sys 0.12.1+1.46.5-patched] make[2]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
[grpcio-sys 0.12.1+1.46.5-patched] [  0%] Building CXX object third_party/re2/CMakeFiles/re2.dir/re2/bitstate.cc.o
[grpcio-sys 0.12.1+1.46.5-patched] [  0%] Building CXX object third_party/re2/CMakeFiles/re2.dir/re2/compile.cc.o
...

Notice: jobserver unavailable: using -j1. Add '+' to parent make rule. This seems to be what blocks parallel compilation.

takes a looooong time

It takes ~10 min for us to build grpcio-sys due to this.

Setting the CMAKE_BUILD_PARALLEL_LEVEL env var before running cargo build seems to work around the issue:

[grpcio-sys 0.12.1+1.46.5-patched] running: "cmake" "--build" "." "--target" "grpc" "--config" "RelWithDebInfo"
[grpcio-sys 0.12.1+1.46.5-patched] make: warning: -j64 forced in submake: resetting jobserver mode.
[grpcio-sys 0.12.1+1.46.5-patched] [  0%] Building C object CMakeFiles/upb.dir/third_party/upb/upb/reflection.c.o

The message doesn't show in either case when compiling manually https://github.com/pingcap/grpc/tree/996605a5e62f3f00043ac8d3ebca84523bc2dd76

@purew
Copy link
Author

purew commented Feb 20, 2023

That's exactly right. I'm on Archlinux as well and I see [grpcio-sys 0.12.1+1.46.5-patched] make[2]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule. in the output, and it's all single-threaded building.

@BusyJay
Copy link
Member

BusyJay commented Feb 21, 2023

This project doesn't control the parallelism of build. Instead, it's controlled by cargo. Cargo has its own jobserver, and cmake-rs will bridge the jobserver from cargo with cmake/make. However, make will disable jobserver if it thinks child command is not a make.

I think @Ten0 has given a good workaround. And for those who accepts build system other than make, you can also use ninja by env CMAKE_GENERATOR=Ninja cargo build.

@BusyJay
Copy link
Member

BusyJay commented Feb 21, 2023

Probably related to rust-lang/jobserver-rs#47.

@purew
Copy link
Author

purew commented Feb 27, 2023

I'm going to close this ticket as parallel and cached builds have gotten me back to the previous baseline.

@purew purew closed this as completed Feb 27, 2023
@Ten0
Copy link
Contributor

Ten0 commented Feb 27, 2023

@purew I don't understand - it seems the issue is still present: parallel build should enable and it doesn't.
The workaround is what it is: a workaround. Until people don't have to set that environment variable themselves before they build, I don't think we can consider this issue fixed.

@purew
Copy link
Author

purew commented Feb 27, 2023

👍 feel free to keep the ticket open. I agree with the workaround classification.

@Ten0
Copy link
Contributor

Ten0 commented Feb 27, 2023

I don't have the necessary access to reopen it.

@purew purew reopened this Feb 27, 2023
@anacrolix
Copy link

I'm experiencing this issue too. Build times are crazy, and single threaded.

@anacrolix
Copy link

CMAKE_GENERATOR=Ninja cargo build works, but it wasn't clear above how to pass CMAKE_BUILD_PARALLEL_LEVEL.

@Ten0
Copy link
Contributor

Ten0 commented Jan 24, 2024

Same way as you pass CMAKE_GENERATOR.

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

No branches or pull requests

4 participants