Skip to content

Improve cargo check times for incremental changes to godot-core #266

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

Merged

Conversation

lilizoey
Copy link
Member

@lilizoey lilizoey commented May 8, 2023

On my machine, this change makes the time cargo check takes to run go from 48 seconds to 11 seconds when making an incremental change to godot-core. in this case tested by adding/removing a single method to/from Basis.

I've seen it be as low as 2 seconds sometimes but im not entirely sure why that happens.

lilizoey added 2 commits May 7, 2023 00:38
Incremental rebuild of `cargo check` after change in godot-core timings:

before: 48.015s average
after:  32.063s average
diff:   -33.22%
Incremental rebuild of `cargo check` after change in godot-core timings:

before: 32.63s average
after:  11.068s average
diff:   -65.48%
@lilizoey lilizoey added quality-of-life No new functionality, but improves ergonomics/internals c: tooling CI, automation, tools labels May 8, 2023
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-266

@Bromeon
Copy link
Member

Bromeon commented May 8, 2023

Good catch! I definitely wouldn't have expected such an impact.
There might be other places where we can use fully-qualified paths to improve compile times.

bors r+

@bors
Copy link
Contributor

bors bot commented May 8, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

  • full-ci

@bors bors bot merged commit 97ec8f1 into godot-rust:master May 8, 2023
@ttencate
Copy link
Contributor

ttencate commented May 9, 2023

Today I learned from here that glob imports are resolved differently from named imports. For ergonomics reasons, name collisions when importing multiple globs are ignored until you actually try to use an ambiguous name.

So my guess is that these imports are resolved lazily, and the compiler has to work harder to resolve each identifier if there are any glob imports in scope. I can't find any mention on the rustc issue tracker about this though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: tooling CI, automation, tools quality-of-life No new functionality, but improves ergonomics/internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants