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

task: make inner of Unconstrained accessible #6886

Closed
wants to merge 1 commit into from

Conversation

ruihe774
Copy link

@ruihe774 ruihe774 commented Oct 7, 2024

Motivation

The wrapped future in Unconstrained was not accessible.

Solution

Added four methods:

  • Unconstrained<F>::get_ref()
  • Unconstrained<F>::get_mut()
  • Unconstrained<F>::get_pin_mut()
  • Unconstrained<F>::into_inner()

@nurmohammed840
Copy link
Contributor

Could you please share the specific use cases you have in mind for this ?
Wouldn't it be better to avoid creating Unconstrained in the first place ?

@Darksonn Darksonn added A-tokio Area: The main tokio crate M-task Module: tokio/task labels Oct 9, 2024
@ruihe774
Copy link
Author

ruihe774 commented Oct 9, 2024

Could you please share the specific use cases you have in mind for this ?

Preempting and some sort of realtime task.

I'm aware that tokio is not suitable for realtime tasks. But Unconstrained at least provides a minimal utility.

For example, I want a timer to preempt current task, which requires this timer to be Unconstrained. In fact, this is how tokio::time::timeout is implemented.

Wouldn't it be better to avoid creating Unconstrained in the first place ?

It's the only public API to tweak coop scheduling. There is no alternative.

And giving it already exists as a public API, I don't think prevent users from using it is a good idea.

@nurmohammed840
Copy link
Contributor

nurmohammed840 commented Oct 9, 2024

The only potential use case would be to conditionally disable cooperative scheduling. For example:

use std::future::poll_fn;
use std::pin::Pin;

async fn main() {
    let fut = std::pin::pin!(async {});
    let mut fut = unconstrained(fut);

    let mut i = 0;
    poll_fn(|cx| {
        i += 1;
        if (i % 2) == 0 {
            Pin::new(fut.get_mut()).poll(cx) // poll inner future. 
        } else {
            Pin::new(&mut fut).poll(cx) // Turn off cooperative scheduling
        }
    })
    .await;
}

Could you explain why conditionally turning off cooperative scheduling might be useful in specific scenarios ?

I don't think prevent users from using it is a good idea.

By calling Unconstrained<F>::get_ref() to obtain a reference to a future is useless. Because you can't do anything useful with it.
So does Unconstrained<F>::into_inner() is pointless if you cannot utilize Unconstrained in the first place.

@ruihe774
Copy link
Author

ruihe774 commented Oct 9, 2024

By calling Unconstrained<F>::get_ref() to obtain a reference to a future is useless. Because you can't do anything useful with it.

get_ref() returns a concrete reference &F; it's not &dyn Future. e.g. I can access (or modify if using get_mut()/get_pin_mut()) the field of F. Why is it useless?

@nurmohammed840
Copy link
Contributor

Why is it useless?

Because, the Future trait does not provide any methods that accept &self

@ruihe774
Copy link
Author

ruihe774 commented Oct 9, 2024

Why is it useless?

Because, the Future trait does not provide any methods that accept &self

Yes but get_ref() returns &F not &dyn Future.

e.g.

struct MyFuture {
  some_state: State
}

impl Future for MyFuture {
  type Output = SomeOutput;
  fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
    // do something
  }
}

pin_project! {
  struct MyWrapper {
    #[pin]
    inner: Unconstrained<MyFuture>
  }
}

impl Future for MyWrapper {
  type Output = SomeOutput;
  fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
    self.inner.get_ref().some_state.do_something();
    self.project().inner.poll(cx)
  }
}

The Future trait has no methods that accept &self but we are dealing a concrete object not a trait object.

Comment on lines +50 to +54

/// Gets a pinned mutable reference to the wrapped future.
pub fn get_pin_mut(self: Pin<&mut Self>) -> Pin<&mut F> {
self.project().inner
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Gets a pinned mutable reference to the wrapped future.
pub fn get_pin_mut(self: Pin<&mut Self>) -> Pin<&mut F> {
self.project().inner
}

You can always create a Pin later when needed

Copy link
Author

Choose a reason for hiding this comment

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

No. Pin::new() requires T impl Unpin.

Copy link
Contributor

Choose a reason for hiding this comment

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

That shouldn't be an issue.

Copy link
Author

Choose a reason for hiding this comment

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

Consider this:

struct State(PhantomPinned);

struct MyFuture {
  some_state: State
}

impl MyFuture {
  fn ready_to_poll(&self) -> bool {
    // check the condition
  }
  fn setup_work(self: Pin<&mut Self>, cx: &mut Context<'_>) {
    // do some setup
  }
}

impl Future for MyFuture {
  type Output = SomeOutput;
  fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
    // poll
  }
}

pin_project! {
  struct MyWrapper {
    #[pin]
    inner: Unconstrained<MyFuture>
  }
}

impl Future for MyWrapper {
  type Output = SomeOutput;
  fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
    if self.inner.get_ref().ready_to_poll() {
      self.project().inner.poll(cx)
    } else {
      self.project().inner.get_pin_mut().setup_work(cx);
      Poll::Pending
    }
  }
}

Existing APIs like tokio::io::BufReader also have get_pin_mut()

@Darksonn
Copy link
Contributor

You can use unconstrained(&mut fut).await or unconstrained(&mut fut).poll(cx) to achieve the things mentioned above. It's not clear to me that these methods are necessary.

@ruihe774
Copy link
Author

You can use unconstrained(&mut fut).await or unconstrained(&mut fut).poll(cx) to achieve the things mentioned above. It's not clear to me that these methods are necessary.

Right.

@ruihe774 ruihe774 closed this Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-task Module: tokio/task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants