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

Always store state in an Arc #1269

Closed
davidpdrsn opened this issue Aug 17, 2022 · 1 comment · Fixed by #1270
Closed

Always store state in an Arc #1269

davidpdrsn opened this issue Aug 17, 2022 · 1 comment · Fixed by #1270
Assignees
Labels
A-axum C-feature-request Category: A feature request, i.e: not implemented / a PR.
Milestone

Comments

@davidpdrsn
Copy link
Member

Feature Request

Motivation

The way the state is passed around in #1155 leads to it being cloned quite a bit so its important that the state is cheap to clone. We would like to always store the state in an Arc so users don't have to worry about that.

This should be possible because we only give references to the state and because FromRef<T> for T requires T: Clone.

Additionally this means we can remove all the S: Clone bounds we have today.

Proposal

Always store an Arc<S> inside Router, MethodRouter, and whatever else might store the state.

Something like this for constructing Router:

struct Router<S> {
    state: Arc<S>,
    ...
}

impl<S> Router<S> {
    fn with_state(state: S) -> Self {
        Self::with_state_arc(Arc::new(state))
    }

    // this way users can avoid a double `Arc` if they already have an `Arc<S>`
    fn with_state_arc(state: Arc<S>) -> Self {
        Self {
            state,
            ...
        }
    }
}

impl Router<()> {
    fn new() -> Self {
        // making an `Arc<()>` does allocate but this is a single allocation
        // made during setup, so probably fine
        Self::with_state(Arc::new(()))
    }
}

Alternatives

Do nothing but document that users should make sure their state is cheaply cloneable.

@davidpdrsn davidpdrsn added C-feature-request Category: A feature request, i.e: not implemented / a PR. A-axum labels Aug 17, 2022
@davidpdrsn davidpdrsn added this to the 0.6 milestone Aug 17, 2022
@davidpdrsn davidpdrsn self-assigned this Aug 17, 2022
@davidpdrsn
Copy link
Member Author

Note to self: First add benchmarks for Extension and State, to compare, and to see how much an Arc<S> helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-axum C-feature-request Category: A feature request, i.e: not implemented / a PR.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant