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

#[pg_guard] cleanup #526

Merged
merged 8 commits into from
Apr 18, 2022
Merged

#[pg_guard] cleanup #526

merged 8 commits into from
Apr 18, 2022

Conversation

eeeebbbbrrrr
Copy link
Contributor

This centralizes all the code #[pg_guard] otherwise generates into a common function, which takes a closure that is a wrapper around the desired internal Postgres function call.

Add a new function to pgx-pg-sys, `create::submodules::postgres_function_guard()` and teach the `#[pg_guard]` macro to use that function instead of duplicating all the sigsetjmp code for every function.
@eeeebbbbrrrr eeeebbbbrrrr changed the base branch from master to develop April 2, 2022 19:26
@eeeebbbbrrrr eeeebbbbrrrr changed the base branch from develop to master April 2, 2022 19:27
@eeeebbbbrrrr eeeebbbbrrrr changed the base branch from master to develop April 2, 2022 19:27
…writer` from build.rs.

Also eliminate a bit of cloning from build.rs as well.

This drastically improves pgx-pg-sys build times.

On my machine, a clean build of the crate goes from ~36s to ~18s.  And subsequent rebuilds (where build.rs doesn't run) goes from ~8s to ~2.4s
…tions, which decreases subsequent pgx-pg-sys rebuilds (where build.rs needs to run) by ~50%.
Cargo.toml Show resolved Hide resolved
Copy link
Contributor

@Hoverbear Hoverbear left a comment

Choose a reason for hiding this comment

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

Other than the (I think) dead code and sorting behavior question, this looks good!

Does this improve build time at all? If so I'd love to have a sample I could snippet for the release notes.

Cargo.toml Show resolved Hide resolved
pgx-pg-sys/src/submodules/setjmp.rs Outdated Show resolved Hide resolved
@eeeebbbbrrrr
Copy link
Contributor Author

Does this improve build time at all? If so I'd love to have a sample I could snippet for the release notes.

Yes, on my machine a rebuild of pgx-pg-sys where build.rs runs has gone from ~36s to ~10s. When build.rs doesn't run, it's gone from ~8s to ~2.5s.

It's quite significant.

@Hoverbear
Copy link
Contributor

Way to go! I'm happy to see this merge, you seem to have a couple pending todos according to comments though!

@Hoverbear Hoverbear merged commit 2adba15 into develop Apr 18, 2022
@Hoverbear Hoverbear deleted the pg_guard-optimization branch April 18, 2022 16:09
@eeeebbbbrrrr eeeebbbbrrrr mentioned this pull request Apr 19, 2022
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.

2 participants