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

Disable thin lto for dev builds by default in template #1035

Merged
merged 1 commit into from
Feb 16, 2023

Conversation

JelteF
Copy link
Contributor

@JelteF JelteF commented Feb 6, 2023

When changing something trivial in the hello world project created by
cargo pgx new, it would take ~20 seconds to compile a dev build of the
generated crate on my machine. This seemed way slower than it should be.
It turns out that the thin LTO setting greatly inrceases compilation
times. By disabling it, compilation time of the crate is only 1 second.

As far as I can tell this setting was enabled initially as a workaround
for this bug in Rust: rust-lang/rust#50007
Since this bug has been fixed since Rust 1.62, and current stable Rust
is version 1.67, this PR removes the workaround.

@eeeebbbbrrrr
Copy link
Contributor

We like PRs against the “develop” branch. Would you mind retargeting this?

otherwise this is perfectly fine. I guess I’m not surprised thin LTO takes so long and we all forgot this was there. Thanks!

@JelteF JelteF changed the base branch from master to develop February 6, 2023 15:15
@JelteF
Copy link
Contributor Author

JelteF commented Feb 6, 2023

We like PRs against the “develop” branch. Would you mind retargeting this?

Done

@eeeebbbbrrrr
Copy link
Contributor

CI is angry at the universe today, it seems. It's not this PR. We'll give it one more chance...

Copy link
Contributor

@thomcc thomcc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to change this in the cargo-pgx template too, here. The one you've changed is part of some nix-specific stuff that most people won't use.

@JelteF
Copy link
Contributor Author

JelteF commented Feb 6, 2023

You need to change this in the cargo-pgx template too, here.

Done, I also removed the same line from all the examples in the repo. (Even though it was already commented out there, but the comment above it suggests to uncomment it)

@JelteF JelteF requested a review from thomcc February 6, 2023 16:41
When changing something trivial in the hello world project created by
`cargo pgx new`, it would take ~20 seconds to compile a dev build of the
generated crate on my machine. This seemed way slower than it should be.
It turns out that the thin LTO setting greatly inrceases compilation
times. By disabling it, compilation time of the crate is only 1 second.

As far as I can tell this setting was enabled initially as a workaround
for this bug in Rust: rust-lang/rust#50007
Since this bug has been fixed since Rust 1.62, and current stable Rust
is version 1.67, this PR removes the workaround.
@eeeebbbbrrrr eeeebbbbrrrr merged commit dc1d399 into pgcentralfoundation:develop Feb 16, 2023
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

Successfully merging this pull request may close these issues.

3 participants