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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]: Enhanced State Mutation for Effortless Handling of Shared Resources #2744

Closed
2 tasks done
wiseaidev opened this issue Mar 3, 2024 · 5 comments
Closed
2 tasks done
Labels
request Request for new functionality

Comments

@wiseaidev
Copy link

wiseaidev commented Mar 3, 2024

What's missing?

Hey Rocket enthusiasts! 馃憢 I've been cruising with the Rocket framework a little before version 0.5 and, and kudos to the recent Rocket foundation formation, it's been quite the ride! While Rocket is a solid backend framework, I've stumbled upon an interesting feature that could elevate it even further.

Overview:

Currently, Rocket lacks built-in support for efficiently handling mutable shared state across multiple handlers. The absence of this functionality results in the need to create redundant mutable copies of the state within each handler, potentially causing increased memory usage as request volumes rise.

Problem(s) to Solve:

The primary issue is the cumbersome management of mutable state, impacting performance and introducing unnecessary complexity in the code. Seamless support for mutable shared state would enhance the overall developer experience and resource efficiency.

Ideal Solution

The Need for Mutability

Let's talk shared state. Currently, handling mutable variables across multiple handlers is a bit, well, clunky. Take a look at this scenario:

// ...snip...
let client = Client::new(api_key, model); // same behavior as wrapping it with Arc/Mutex, this is not the point of this feature
let routes = all_routes();
rocket::build()
    .mount("/", routes)
    .attach(cors)
    .manage(client)
    .launch()
    .await
    .expect("Launch Error");
// ...snip...

Here, Client is our shared state across endpoints. Now, when we want to mutate this state within a handler, we end up creating a new variable, causing potential memory overhead as requests pile up.

The Rocket Approach

Consider this Rocket endpoint:

#[post(
    "/gems/generate-content",
    format = "application/json",
    data = "<input_text>"
)]
async fn generate_content(client: &State<Client>, input_text: String) -> String {
    let mut client = Client::new(&client.api_key, &client.model);
    match client.generate_content(&input_text).await {
        Ok(response) => response,
        Err(error) => error.to_string(),
    }
}

Here, we're forced to create a mutable copy of client just because the generate_content function mutates the variable.

Axum's Elegance

Now, let's take a peek at its Axum counterpart, where mutability is handled with finesse:

async fn generate_content(
    Extension(mut client): Extension<Client>,
    Json(request): Json<GenerateContentRequest>,
) -> impl IntoResponse {
    match client.generate_content(&request.input_text).await {
        Ok(response) => response,
        Err(error) => error.to_string(),
    }
}

Axum's Extension struct gracefully allows mutation within the endpoint, effortlessly supporting mutable state.

Why can't this be implemented outside of Rocket?

Strong Case

Implementing this feature externally might involve extensive workarounds, including complex abstractions or custom middleware, which can be error-prone and hinder the simplicity that Rocket aims to provide. Internal support for mutability would integrate seamlessly with Rocket's design philosophy and potentially offer better performance optimizations.

Are there workarounds usable today?

Current Workarounds

As of now, the primary workaround involves creating mutable copies of the shared state within each handler, as shown in the initial code snippet. While functional, this approach can lead to increased memory usage and is not as elegant or efficient as an integrated solution.

Alternative Solutions

Probably, consider extending Rocket's middleware capabilities to allow for more dynamic handling of mutable state.

Additional Context

I first encountered this limitation in November 18, 2023, while developing an SDK. The inspiration for this feature request came from recently observing how other frameworks, like Axum, seamlessly handle mutable state. Integrating such functionality into Rocket would undoubtedly contribute to its ongoing evolution and user satisfaction. Let's work together to keep Rocket at the forefront of web frameworks!

Best

System Checks

  • I do not believe that this feature can or should be implemented outside of Rocket.
  • I was unable to find a previous request for this feature.
@wiseaidev wiseaidev added the request Request for new functionality label Mar 3, 2024
@SergioBenitez
Copy link
Member

SergioBenitez commented Mar 4, 2024

Instead of creating a brand new client in the handler, you could simply clone the existing reference you get from State. This would be equivalent to what Axum does, except doing it, explicitly, instead of having it be hidden from you. Rocket does not hide potentially expensive operations from you.

Were you aware aware of this behavior from Axum? If so, what aside from the above, would you propose? And if not, does my response above quell the request?

@wiseaidev
Copy link
Author

wiseaidev commented Mar 4, 2024

Rocket does not hide potentially expensive operations from you.

Nice! I wasn't aware of that until now; I just learned it the hard way.

Now, cloning works, but beginners to Rust and/or Rocket may not be aware that they need to dereference the state first before cloning, as demonstrated below:

async fn generate_content(client: &State<Client>, input_text: String) -> String {
    match (*client).clone().generate_content(&input_text).await {
        Ok(response) => response,
        Err(error) => error.to_string(),
    }
}

While explicitly cloning the state might seem like a good theoretical solution, in practice, the state could originate from a third-party library beyond the developer's scope, and the utilized state may lack implementation for the Clone and/or Copy trait. In such cases, cloning is not feasible.

Following the approach adopted by Axum, implicitly cloning the state would be preferable if the state is inherently cloneable to begin with. Therefore, I would like to propose the following solution:

// Auto implement the `FromRequest` trait at compile time?
async fn generate_content(client: &mut State<Client>, input_text: String) -> String {
    //...snip...
}

// or

async fn generate_content(State(mut client): &State<Client>, input_text: String) -> String {
    //...snip...
}

If the user indicates that client is mutable in the function argument, then allow borrowing as mutable or implicitly cloning the state.

Best

@SergioBenitez
Copy link
Member

While explicitly cloning the state might seem like a good theoretical solution, in practice, the state could originate from a third-party library beyond the developer's scope, and the utilized state may lack implementation for the Clone and/or Copy trait. In such cases, cloning is not feasible.

This would mean that the axum example wouldn't work at all. It would simply fail to compile. This seems antithetic to the proposition of finesse.

Now, cloning works, but beginners to Rust and/or Rocket may not be aware that they need to dereference the state first before cloning, as demonstrated below:

You can also do client.inner().clone(), which is arguably simpler and more explicit.

Following the approach adopted by Axum, implicitly cloning the state would be preferable if the state is inherently cloneable to begin with. Therefore, I would like to propose the following solution:

The approach you're suggesting would mean that only cloneable state can be shared, and that every time you try to access said state you must pay the price to clone. Axum likely cannot expose direct references to share state due to internal design constraints, but Rocket can and doesn't need to fallback to cloning. Rocket gives you a direct reference - this is superior in every way except perhaps the need to call .clone() when that's what you actually want. Though even that can be refuted as running counter to the notion of zero-cost abstractions.

If you really want this abstraction, it's fairly straightforward to implement it yourself, actually:

use rocket::{Rocket, Ignite, State};
use rocket::outcome::{try_outcome, Outcome};
use rocket::request::{self, Request, FromRequest};

#[rocket::async_trait]
impl<'r, T: Send + Sync + Clone + 'static> FromRequest<'r> for Cloned<T> {
    type Error = ();

    async fn from_request(req: &'r Request<'_>) -> request::Outcome<Self, Self::Error> {
        let state = try_outcome!(req.guard::<&State<T>>().await);
        Outcome::Success(Cloned(state.inner().clone()))
    }
}

You can even make it a Sentinel:

impl<T: Send + Sync + 'static> Sentinel for Cloned<T> {
    fn abort(rocket: &Rocket<Ignite>) -> bool {
        <&State<T> as Sentinel>::abort(rocket)
    }
}

Then you can do just as you wished:

async fn generate_content(Cloned(mut client): Cloned<Client>) -> String {
    //...snip...
}

@SergioBenitez
Copy link
Member

All that being said, none of this has much to do with mutability. Axum (or anything else in Rust) doesn't (and can't) magically let you share mutable state across threads. It's simply forbidden to share mutable state. You can, however, share state immutably that offers interior mutability, which effectively means using unsafe to work-around this restriction. This is what types like Mutex<T> do and libraries like dashmap offer. But it's not what axum does.

The reason your axum example "works" is because it's cloning Client, and you're getting an owned handle to the cloned object which you bind mutably. It's not doing anything interesting to allow you to share state mutably.

@wiseaidev
Copy link
Author

Rocket gives you a direct reference - this is superior in every way except perhaps the need to call .clone() when that's what you actually want. Though even that can be refuted as running counter to the notion of zero-cost abstractions.

That's actually a good point, and it aligns with the idea of keeping things efficient and giving the developer the ultimate power over things. Additionally, the example you provided about implementing the FromRequest trait, it's kind of what I mentioned about with the "Auto implement the FromRequest trait at compile time?" comment in the code, but automatically generate this implementation. I'm not sure how hard it would be to implement it, but it's definitely something we could improve on.

Now, despite my skill issues and even though I'm still figuring things out with Rocket, there's something that a lot of backend developers might be wondering about. Maybe we could update the docs about state to clear things up a bit. In any case, I just wanted to bring this up to your attention that sates mutability is something that a lot might be wondering about.

That's pretty much it, feel free to close the issue if you think it is already sorted out.

Best

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Request for new functionality
Projects
None yet
Development

No branches or pull requests

2 participants