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

Excessive rebuilding when nothing changed #3346

Closed
imotov opened this issue May 16, 2023 · 12 comments · Fixed by #3379
Closed

Excessive rebuilding when nothing changed #3346

imotov opened this issue May 16, 2023 · 12 comments · Fixed by #3379
Assignees
@imotov
Labels
bug Something isn't working

Comments

@imotov
Copy link
Member

imotov commented May 16, 2023

Describe the bug
In the last 3 weeks (between 36051bd and the current main branch) something changed that causes cargo to do partial rebuild when it is not really necessary. This is especially noticeable in VSCode on slow machines or while working with containers and trying to execute something on command line.

Steps to reproduce (if applicable)
Steps to reproduce the behavior:

  1. Run the following 3 commands:
cargo clean
cargo build --features=postgres
cargo run --features=postgres -- run --service searcher --service metastore --config ../config/quickwit.yaml

Expected behavior
3 weeks ago the run command started the service immediately. Now it goes through a partial rebuild process that takes minutes on some machine.
@imotov imotov added the bug Something isn't working label May 16, 2023
@imotov imotov self-assigned this May 16, 2023
@trinity-1686a
Copy link
Contributor

It might somehow be a compiler regression. I've bisected it to #3244, and the problematic part seems to be going from rust 1.68 to 1.69. On 9c43680 (main right now), I don't have the issue if I force compiling with 1.68

@imotov
Copy link
Member Author

imotov commented May 16, 2023

That's unfortunate. I suspected it might be the case, but was hoping it's just some easy to fix bug in a build script.

@imotov
Copy link
Member Author

imotov commented May 17, 2023

@guilload suggested running rustup override set 1.68.0 as a work-around. It worked for me.

@imotov imotov mentioned this issue May 18, 2023
Merged
@imotov
Copy link
Member Author

imotov commented May 19, 2023

I think we might be also affected by bytecodealliance/rustix#562

I see this all the time while running tests in VSCode on both 1.68 and 1.69

 *  Executing task: cargo test --package quickwit-indexing --lib --all-features -- actors::indexing_service::tests --nocapture 

warning: /home/igor/Projects/quickwit-oss/quickwit/quickwit/Cargo.toml: unused manifest key: workspace.rust-version
    Blocking waiting for file lock on build directory
   Compiling rustix v0.36.13
   Compiling procfs v0.14.2
   Compiling prometheus v0.13.3
   Compiling quickwit-common v0.6.0 (/home/igor/Projects/quickwit-oss/quickwit/quickwit/quickwit-common)

@sunfishcode
Copy link

Could you try adding --verbose? With this flag, cargo adds "Dirty" messages which indicate what prompted it to perform rebuilds.

@imotov
Copy link
Member Author

imotov commented May 19, 2023 •
edited

@sunfishcode with --verbose I get this:

       Dirty rustix v0.36.13: the dependency build_script_build was rebuilt (1684521189.122302115s, 13h 21m 42s after last build at 1684473087.941573652s)
   Compiling rustix v0.36.13
.. skipped ..
     Running `rustc --crate-name rustix --edition=2018 /home/igor/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.36.13/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=110 --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=0 --cfg 'feature="default"' --cfg 'feature="fs"' --cfg 'feature="io-lifetimes"' --cfg 'feature="libc"' --cfg 'feature="param"' --cfg 'feature="process"' --cfg 'feature="std"' --cfg 'feature="thread"' --cfg 'feature="use-libc-auxv"' -C metadata=b0502dcc57ded481 -C extra-filename=-b0502dcc57ded481 --out-dir /home/igor/Projects/quickwit-oss/quickwit/quickwit/target/debug/deps -L dependency=/home/igor/Projects/quickwit-oss/quickwit/quickwit/target/debug/deps --extern bitflags=/home/igor/Projects/quickwit-oss/quickwit/quickwit/target/debug/deps/libbitflags-bb8c1d003de5b446.rmeta --extern io_lifetimes=/home/igor/Projects/quickwit-oss/quickwit/quickwit/target/debug/deps/libio_lifetimes-41a94e0c35a418b1.rmeta --extern libc=/home/igor/Projects/quickwit-oss/quickwit/quickwit/target/debug/deps/liblibc-f7fb825032d01339.rmeta --extern linux_raw_sys=/home/igor/Projects/quickwit-oss/quickwit/quickwit/target/debug/deps/liblinux_raw_sys-3430e5721cc3c0c7.rmeta --cap-lints allow --cfg linux_raw --cfg asm`
.. skipped ..
       Dirty procfs v0.14.2: the dependency rustix was rebuilt
   Compiling procfs v0.14.2
.. skipped ..
       Dirty prometheus v0.13.3: the dependency procfs was rebuilt
   Compiling prometheus v0.13.3
.. skipped ..
       Dirty quickwit-common v0.6.0 (/home/igor/Projects/quickwit-oss/quickwit/quickwit/quickwit-common): the dependency prometheus was rebuilt

That in turn triggers rebuild of the rest of quickwit codebase that depends on quickwit-common.

@imotov
Copy link
Member Author

imotov commented May 19, 2023

@sunfishcode one additional observation that might be important here. We also pull rustix v0.37.19 as part of another dependency and it doesn't have the same issue.

@sunfishcode
Copy link

Thanks! Looking into it, this is likely to be due to an issue which was fixed in rustix 0.37, so I've now backported the fix to 0.36 and released it in 0.36.14.

@imotov
Copy link
Member Author

imotov commented May 20, 2023

@sunfishcode wow, that was fast! Thanks a lot, that really helps. While it doesn't solve the original issue (it seems that we have a somewhat similar problem with proc-macro2) the new version resolved many issues that I had in VS Code interactions while switching between building and running tests.

@imotov
Copy link
Member Author

imotov commented May 20, 2023

I think I figured it out. It looks like a regression in rust. Unfortunately, I cannot reproduce it using a small project with a manageable size, but I know the mechanism that is causing the issue and know how to fix it.

I suspect the issue is triggered by this change in 1.69.0 that is producing different command lines for building crates with edition = "2018" depending on how you build it. We have quite a few of these in our dependency tree, but it all starts with unicode_ident. When it is built using cargo build command it is compiled like this

Running `CARGO=/home/igor/.rustup/toolchains/1.69-aarch64-unknown-linux-gnu/bin/cargo CARGO_CRATE_NAME=unicode_ident CARGO_MANIFEST_DIR=/home/igor/.cargo/registry/src/github.com-1ecc6299db9ec823/unicode-ident-1.0.8 CARGO_PKG_AUTHORS='David Tolnay <dtolnay@gmail.com>' CARGO_PKG_DESCRIPTION='Determine whether characters have the XID_Start or XID_Continue properties according to Unicode Standard Annex #31' CARGO_PKG_HOMEPAGE='' CARGO_PKG_LICENSE='(MIT OR Apache-2.0) AND Unicode-DFS-2016' CARGO_PKG_LICENSE_FILE='' CARGO_PKG_NAME=unicode-ident CARGO_PKG_REPOSITORY='https://github.com/dtolnay/unicode-ident' CARGO_PKG_RUST_VERSION=1.31 CARGO_PKG_VERSION=1.0.8 CARGO_PKG_VERSION_MAJOR=1 CARGO_PKG_VERSION_MINOR=0 CARGO_PKG_VERSION_PATCH=8 CARGO_PKG_VERSION_PRE='' LD_LIBRARY_PATH='/home/igor/Projects/quickwit-oss/quickwit/quickwit/target/debug/deps:/home/igor/.rustup/toolchains/1.69-aarch64-unknown-linux-gnu/lib:/home/igor/.rustup/toolchains/1.69-aarch64-unknown-linux-gnu/lib' rustc --crate-name unicode_ident --edition=2018 /home/igor/.cargo/registry/src/github.com-1ecc6299db9ec823/unicode-ident-1.0.8/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=181 --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=0 -C metadata=ae9e877358182a13 -C extra-filename=-ae9e877358182a13 --out-dir /home/igor/Projects/quickwit-oss/quickwit/quickwit/target/debug/deps -L dependency=/home/igor/Projects/quickwit-oss/quickwit/quickwit/target/debug/deps --cap-lints warn`

When it is built using cargo run it is compiled like this:

Running `CARGO=/home/igor/.rustup/toolchains/1.69-aarch64-unknown-linux-gnu/bin/cargo CARGO_CRATE_NAME=unicode_ident CARGO_MANIFEST_DIR=/home/igor/.cargo/registry/src/github.com-1ecc6299db9ec823/unicode-ident-1.0.8 CARGO_PKG_AUTHORS='David Tolnay <dtolnay@gmail.com>' CARGO_PKG_DESCRIPTION='Determine whether characters have the XID_Start or XID_Continue properties according to Unicode Standard Annex #31' CARGO_PKG_HOMEPAGE='' CARGO_PKG_LICENSE='(MIT OR Apache-2.0) AND Unicode-DFS-2016' CARGO_PKG_LICENSE_FILE='' CARGO_PKG_NAME=unicode-ident CARGO_PKG_REPOSITORY='https://github.com/dtolnay/unicode-ident' CARGO_PKG_RUST_VERSION=1.31 CARGO_PKG_VERSION=1.0.8 CARGO_PKG_VERSION_MAJOR=1 CARGO_PKG_VERSION_MINOR=0 CARGO_PKG_VERSION_PATCH=8 CARGO_PKG_VERSION_PRE='' LD_LIBRARY_PATH='/home/igor/Projects/quickwit-oss/quickwit/quickwit/target/debug/deps:/home/igor/.rustup/toolchains/1.69-aarch64-unknown-linux-gnu/lib:/home/igor/.rustup/toolchains/1.69-aarch64-unknown-linux-gnu/lib' rustc --crate-name unicode_ident --edition=2018 /home/igor/.cargo/registry/src/github.com-1ecc6299db9ec823/unicode-ident-1.0.8/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=181 --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C metadata=6bb1a5f23e3b8481 -C extra-filename=-6bb1a5f23e3b8481 --out-dir /home/igor/Projects/quickwit-oss/quickwit/quickwit/target/debug/deps -L dependency=/home/igor/Projects/quickwit-oss/quickwit/quickwit/target/debug/deps --cap-lints warn`

The lines are basically identical with a very small inclusion. The first line has -C debuginfo=0 in it, which also changes fingerprint and causes all that rebuilding. The fix is actually quite silly. Replacing

[profile.dev]
debug = 0

with

[profile.dev]
debug = false

fixes the issue.

imotov added a commit to imotov/quickwit that referenced this issue May 20, 2023
@imotov
Upgrade rustix 0.36.13->0.36.14 and fix debug info
61109f3 
This version of rustix no longer causes recompilation during tests runs
while using VSCode. This commit also works around a bug introduced in
rust 1.69.0 that was causing rebuilding between cargo build and cargo
run.

Fixes quickwit-oss#3346
@imotov imotov mentioned this issue May 20, 2023
Merged
imotov added a commit that referenced this issue May 20, 2023
@imotov
Upgrade rustix 0.36.13->0.36.14 and fix debug info (#3379)
6bf0b21 
This version of rustix no longer causes recompilation during tests runs
while using VSCode. This commit also works around a bug introduced in
rust 1.69.0 that was causing rebuilding between cargo build and cargo
run.

Fixes #3346
@imotov
Copy link
Member Author

imotov commented May 20, 2023

I opened rust-lang/cargo#12163

@imotov
Copy link
Member Author

imotov commented May 21, 2023

Based on the description of the issue in the previous comment, we might need to circle back to this. The workaround that I proposed is currently not working in nightly.

@ilammy ilammy mentioned this issue Aug 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@imotov imotov

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Upgrade rustix 0.36.13->0.36.14 and fix debug info imotov/quickwit
3 participants
@imotov @sunfishcode @trinity-1686a