Fix unnecessary rebuilds by updating to maturin 1.3.1 #3266

emilk · 2023-09-11T08:36:39Z

Often when I run cargo check, it rebuilds dependencies deep down the dependency chain, e.g. egui. Why? This is slowing me down a lot.

The text was updated successfully, but these errors were encountered:

Wumpf · 2023-09-22T09:18:29Z

I isolated one pattern with deterministic & unnecessary rebuilds

do a change
cargo rerun
just py-build
cargo rerun <- will build winit again

Looking at it with CARGO_LOG=cargo::core::compiler::fingerprint=trace I found

[2023-09-21T11:25:03Z INFO  cargo::core::compiler::fingerprint] dependency on `winit` is newer than we are 1695295464.541226352s > 1695295444.936536278s "/Users/andreas/.cargo/git/checkouts/egui-5e4507fa4153be06/d949eaf/crates/egui-winit"
[2023-09-21T11:25:03Z INFO  cargo::core::compiler::fingerprint] dependency on `winit` is newer than we are 1695295464.541226352s > 1695295445.096291254s "/Users/andreas/.cargo/git/checkouts/egui-5e4507fa4153be06/d949eaf/crates/eframe"

which seems to imply an issue with our current egui patches

emilk · 2023-09-26T08:17:48Z

We have a lot of build.rs files following this pattern:

fn main() {
    re_build_tools::rebuild_if_crate_changed("re_data_source");
    re_build_tools::export_env_vars();
}

I am afraid this is brittle, and causes too many re-builds.

We have this code in order to get correct and up-to-date build info.

I wonder if we really need this though. Sure we want good build info for build from CI, but who really cares about the commit hash when you are building locally?

I added it originally for when working on the web-viewer, since it wasn't always obvious if the web-viewer that e.g. a notebook was serving was the latest built one (either due to bugs in the build.rs script that should trigger a rebuild of the web-viewer, or because of browser-side caching). In any case: this could be solves somehow else, e.g. with something opt-in.

emilk · 2023-09-26T09:46:29Z

Running cargo codegen over and over will rebuild re_types_builder every time. That can't be right.

teh-cmc · 2023-09-26T10:12:51Z

Running cargo codegen over and over will rebuild re_types_builder every time. That can't be right.

It's the same underlying problem as the roundtrip test suite execution time regression:

Fix re_types_builder falsely triggering rebuilds #3474

#3460 removed committed hashes, but `re_types_builder` is still subscribed to changes to `source_hash.txt`. Even though the build script will early exit as soon as it realizes that the hash is `None`, this is enough to invalidate `rustc`'s cache, which then invalidates every crate downstream. `main`: ![image](https://github.com/rerun-io/rerun/assets/2910679/80ff901e-926e-4695-a305-c88a483a9c0f) this branch: ![image](https://github.com/rerun-io/rerun/assets/2910679/acaec63e-660c-43ee-8d9e-4e0ecaea06b9) Fixes: - The roundtrip test suite execution time regression - #3266 ### What ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3474) (if applicable) - [PR Build Summary](https://build.rerun.io/pr/3474) - [Docs preview](https://rerun.io/preview/ea449009416498c258c9672706dc3ebc362f16e2/docs)  - [Examples preview](https://rerun.io/preview/ea449009416498c258c9672706dc3ebc362f16e2/examples)  - [Recent benchmark results](https://ref.rerun.io/dev/bench/) - [Wasm size tracking](https://ref.rerun.io/dev/sizes/)

emilk · 2023-09-26T16:15:21Z

This issue was opened before the PR that caused the bug fixed by #3474. There are more improvements to be had.

emilk · 2023-10-09T15:48:55Z

Right now I'm leaning towards something like this:

opt-in to most of our build.rs-shenanigans using an environment variable
when unset, we don't show things like commit hash in the viewer (since it is unreliable without the environment variable)

This means you'd most often need to run cargo codegen manually, unless you turn on that environment variable.

### What * Part of #3266 This cleans up all our `build.rs` files by introducing better helpers in `re_build_tools`. No functionality is actually changed (except a few tiny improvements). This will make it easier to take take the next steps of opting-in to most of these features. This is just the first step. ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3789) (if applicable) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG - [PR Build Summary](https://build.rerun.io/pr/3789) - [Docs preview](https://rerun.io/preview/e2cd48dcc1b549b9523fb92423e0da7521522afc/docs)  - [Examples preview](https://rerun.io/preview/e2cd48dcc1b549b9523fb92423e0da7521522afc/examples)  - [Recent benchmark results](https://ref.rerun.io/dev/bench/) - [Wasm size tracking](https://ref.rerun.io/dev/sizes/)

* Part of #3266 ### What #### `rust-analyzer` will no longer run codegen …if you configure `rust-analyzer` to use `--all-features`, which you should, and which we do in `.vscode/settings.json` Same is true for stuff like `bacon` too. #### No build-time and git branch/commit for local developers I've removed the build-time and git branch/commit from: * `rerun --version` * Viewer Rerun Menu -> About * analytics for local developers (that means you), who has cloned our repository. This is to save us from annoying re-builds using complex detection code. #### No build-time and git branch/commit for crate users Users of our crates (i.e. users who put `rerun` in their `Cargo.toml` or build `rerun` using `cargo install rerun`) will also no longer see the build-time and git branch/commit in these places. Previously we would try to get the git branch/commit hash for whatever folder they happened to be in (😬 ), which was not a great idea. So that's a bug fix. The build time is a bit sadder to let go, but it was always unreliable: if you ran `cargo update` for instance, the build time would not update even if the rerun crate was being rebuilt. So instead of showing an unreliable built-time, I think it is better to just remove it. ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3814) (if applicable) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG - [PR Build Summary](https://build.rerun.io/pr/3814) - [Docs preview](https://rerun.io/preview/b6822d26484951599b89cd0c06b607cb88d2c4ec/docs)  - [Examples preview](https://rerun.io/preview/b6822d26484951599b89cd0c06b607cb88d2c4ec/examples)  - [Recent benchmark results](https://ref.rerun.io/dev/bench/) - [Wasm size tracking](https://ref.rerun.io/dev/sizes/)

emilk · 2023-10-12T05:24:11Z

The problem that Andreas reported still remain on main:

> cargo rerun --version
…
> just py-build
…
> cargo rerun --version
   Compiling objc-sys v0.2.0-beta.2
   Compiling objc2-encode v2.0.0-pre.2
   Compiling objc2 v0.3.0-beta.3.patch-leaks.3
   Compiling winit v0.28.7
   Compiling accesskit_macos v0.7.1
   Compiling accesskit_winit v0.14.1
   Compiling egui-wgpu v0.23.0
   Compiling egui-winit v0.23.0
   Compiling eframe v0.23.0
   Compiling re_ui v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/re_ui)
   Compiling re_viewer_context v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/re_viewer_context)
   Compiling re_data_ui v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/re_data_ui)
   Compiling re_space_view v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/re_space_view)
   Compiling re_space_view_text_document v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/re_space_view_text_document)
   Compiling re_space_view_time_series v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/re_space_view_time_series)
   Compiling re_space_view_bar_chart v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/re_space_view_bar_chart)
   Compiling re_space_view_spatial v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/re_space_view_spatial)
   Compiling re_time_panel v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/re_time_panel)
   Compiling re_space_view_text_log v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/re_space_view_text_log)
   Compiling re_space_view_tensor v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/re_space_view_tensor)
   Compiling re_viewport v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/re_viewport)
   Compiling re_viewer v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/re_viewer)
   Compiling rerun v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/rerun)
   Compiling rerun-cli v0.10.0-alpha.6+dev (/Users/emilk/code/rerun/rerun/crates/rerun-cli)
    Finished dev [optimized + debuginfo] target(s) in 13.23s
     Running `target/debug/rerun --version`

This made me wonder if the feature flags are changing somehow. eframe has some feature flags in Cargo.toml and some other in crates/re_viewer/Cargo.toml - but if I unify them to always enable all the feature, I still end up with the same result. Curious.

emilk · 2023-10-12T05:31:34Z

objc-sys is the first crate being rebuilt. I think the problem lies in https://github.com/madsmtm/objc2/blob/master/crates/objc-sys/build.rs and the environment variables given to it. I think maturin sets MACOSX_DEPLOYMENT_TARGET when compiling, which objc-sys/build.rs detect and triggers a rebuild:

❯ just py-build        
⚠️ Warning: specify [package.metadata.maturin] name in Cargo.toml is deprecated, use module-name in [tool.maturin] section in pyproject.toml instead
🔗 Found pyo3 bindings with abi3 support for Python ≥ 3.8
🐍 Not using a specific python interpreter
📡 Using build options locked from pyproject.toml
…
💻 Using `MACOSX_DEPLOYMENT_TARGET=11.0` for aarch64-apple-darwin by default
   Compiling objc-sys v0.2.0-beta.2

EDIT: Yes indeed! By always setting the environment variable MACOSX_DEPLOYMENT_TARGET=11.0 we can get rid of the rebuilds.

The problem is now: how do we make sure objc-sys always sees the same MACOSX_DEPLOYMENT_TARGET?
Surprisingly, setting it in .cargo/config.toml does NOT work - it seems to only make the env-var visible for build.rs files inside the workspace.

We can of course tell our developers to just set MACOSX_DEPLOYMENT_TARGET=11.0 in their global ~/.zshrc or whatever, but that's is not great.

Or we patch Maturin in a way to make it not set MACOSX_DEPLOYMENT_TARGET unless we tell it to.

I've opened a Maturin ticket to start a discussion about this: PyO3/maturin#1804

emilk · 2023-11-06T08:40:23Z

Unblocked thanks to PyO3/maturin#1804
if we update to maturin 1.3.1 this should be fixed: https://github.com/PyO3/maturin/releases/tag/v1.3.1

### What This should fix the recompilation issue related to the `MACOSX_DEPLOYMENT_TARGET` env var. * Fixes #3266 ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using newly built examples: [app.rerun.io](https://app.rerun.io/pr/5197/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/5197/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [app.rerun.io](https://app.rerun.io/pr/5197/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! - [PR Build Summary](https://build.rerun.io/pr/5197) - [Docs preview](https://rerun.io/preview/09679948de831e3bc7ec53cd3bd9298f8731d16e/docs)  - [Examples preview](https://rerun.io/preview/09679948de831e3bc7ec53cd3bd9298f8731d16e/examples)  - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

emilk added the 🧑‍💻 dev experience developer experience (excluding CI) label Sep 11, 2023

emilk added this to the 0.10 Polish milestone Sep 26, 2023

teh-cmc mentioned this issue Sep 26, 2023

Fix re_types_builder falsely triggering rebuilds #3474

Merged

3 tasks

teh-cmc linked a pull request Sep 26, 2023 that will close this issue

Fix re_types_builder falsely triggering rebuilds #3474

Merged

3 tasks

teh-cmc closed this as completed in #3474 Sep 26, 2023

emilk reopened this Sep 26, 2023

emilk changed the title ~~Investigate why cargo check is doing double-work~~ Simplify build.rs magic Oct 9, 2023

emilk self-assigned this Oct 10, 2023

emilk mentioned this issue Oct 10, 2023

Refactor our build.rs files #3789

Merged

4 tasks

emilk mentioned this issue Oct 11, 2023

Less automatic build.rs shenanigans #3814

Merged

4 tasks

emilk changed the title ~~Simplify build.rs magic~~ Fix unnecessary rebuilds Oct 12, 2023

emilk removed their assignment Oct 12, 2023

emilk added the blocked can't make progress right now label Oct 18, 2023

emilk removed the blocked can't make progress right now label Nov 6, 2023

emilk changed the title ~~Fix unnecessary rebuilds~~ Fix unnecessary rebuilds by updating to maturin 1.3.1 Nov 6, 2023

emilk added the 🏎️ Quick Issue Can be fixed in a few hours or less label Nov 6, 2023

emilk assigned emilk and unassigned emilk Nov 28, 2023

abey79 mentioned this issue Feb 14, 2024

Bump maturin to 1.14.0 #5197

Merged

5 tasks

emilk closed this as completed in #5197 Feb 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix unnecessary rebuilds by updating to maturin 1.3.1 #3266

Fix unnecessary rebuilds by updating to maturin 1.3.1 #3266

emilk commented Sep 11, 2023 •

edited

Loading

Wumpf commented Sep 22, 2023

emilk commented Sep 26, 2023 •

edited

Loading

emilk commented Sep 26, 2023 •

edited

Loading

teh-cmc commented Sep 26, 2023

emilk commented Sep 26, 2023

emilk commented Oct 9, 2023

emilk commented Oct 12, 2023 •

edited

Loading

emilk commented Oct 12, 2023 •

edited

Loading

emilk commented Nov 6, 2023 •

edited

Loading

Fix unnecessary rebuilds by updating to maturin 1.3.1 #3266

Fix unnecessary rebuilds by updating to maturin 1.3.1 #3266

Comments

emilk commented Sep 11, 2023 • edited Loading

Wumpf commented Sep 22, 2023

emilk commented Sep 26, 2023 • edited Loading

emilk commented Sep 26, 2023 • edited Loading

teh-cmc commented Sep 26, 2023

emilk commented Sep 26, 2023

emilk commented Oct 9, 2023

emilk commented Oct 12, 2023 • edited Loading

emilk commented Oct 12, 2023 • edited Loading

emilk commented Nov 6, 2023 • edited Loading

emilk commented Sep 11, 2023 •

edited

Loading

emilk commented Sep 26, 2023 •

edited

Loading

emilk commented Sep 26, 2023 •

edited

Loading

emilk commented Oct 12, 2023 •

edited

Loading

emilk commented Oct 12, 2023 •

edited

Loading

emilk commented Nov 6, 2023 •

edited

Loading