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

Avoid moves in debug builds with MaybeUninit (once we are ready) #97

Closed
Veetaha opened this issue Sep 1, 2024 · 1 comment
Closed
Labels
feature request A new feature is requested

Comments

@Veetaha
Copy link
Contributor

Veetaha commented Sep 1, 2024

This is based on a reddit comment

The proposal is to implement a builder that uses MaybeUninit<Fields> under the hood and does ptr::write/read to access the underlying fields. It does so safely because the builder has the info about which fields are initialized using the type state.

I'm writing up this issue for the future when bon is more stable and feature complete, at which point the cost of maintaining unsafe code isn't that high.

Motivation

This will allow for better-optimized debug builds. The current approach of using the safe code doesn't allow for that because it moves data around and relies on compiler optimizations. With the MaybeUnint version we don't move anything and just mutate an existing memory.

Note on the important caveat that there must be a generic Drop implementation that makes sure all fields that were initialized are dropped in case when only part of the fields were initialized (e.g. there was a panic or an error in the middle).

@Veetaha Veetaha changed the title Use unsafe code with MaybeUninit to generate a builder that doesn't move anything Use unsafe code with MaybeUninit to generate a builder that doesn't move anything (far future) Sep 1, 2024
@Veetaha Veetaha added the feature request A new feature is requested label Sep 1, 2024
@Veetaha Veetaha changed the title Use unsafe code with MaybeUninit to generate a builder that doesn't move anything (far future) Use unsafe code with MaybeUninit to generate a builder that doesn't move anything (once we are ready) Sep 2, 2024
@Veetaha Veetaha changed the title Use unsafe code with MaybeUninit to generate a builder that doesn't move anything (once we are ready) Avoid moves in debug builds with MaybeUninit (once we are ready) Nov 10, 2024
@Veetaha
Copy link
Contributor Author

Veetaha commented Dec 10, 2024

I've revisited this idea in light of #[builder(field)] and #225 (comment), where I'd like to make some of the builder's private fields accessible to the user. This already goes against the idea of this issue, and I think it's fine, because otherwise it lets bon provide nicer API for the users.

Otherwise I believe the overhead of the builder is already quite low even in debug builds. This is because the builder stores all fields in Options. It then assigns to them in setters and the only thing that requires moving data is the PhantomData field, which the compiler should optimize out even in debug builds.

I'm closing this as not planned because it would conflict with the existing API of #[builder(field)] and #[builder(start_fn)]

@Veetaha Veetaha closed this as completed Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A new feature is requested
Projects
None yet
Development

No branches or pull requests

1 participant