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

Draft: Futures module #1033

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Draft: Futures module #1033

wants to merge 1 commit into from

Conversation

ranfdev
Copy link
Member

@ranfdev ranfdev commented Feb 26, 2023

Related issue: #935

I think there are currently too many timeout functions, with too many combinations of scheduling priority and scheduling precision (second, millisecond).

Instead of just moving every timeout function to a futures module, I've decided to redesign the API taking advantage of the Builder pattern and the IntoFuture trait.

API example:

c.block_on(async {
    sleep(Duration::from_millis(10)).await;
    sleep(Duration::from_secs(1))
        .priority(crate::PRIORITY_LOW)
        .precision(SchedulingPrecision::Seconds)
        .await;
});

I'm still looking if it's possible to redesign spawn and idle with the same mindset. I don't particularly like having spawn, spawn_local, spawn_local_with_priority, spawn_with_priority, spawn_within, spawn_within_with_priority, ...

@ranfdev ranfdev added the enhancement New feature or request label Feb 26, 2023
@ranfdev ranfdev marked this pull request as draft February 26, 2023 17:05
@sdroege
Copy link
Member

sdroege commented Feb 26, 2023

I'm still looking if it's possible to redesign spawn and idle with the same mindset. I don't particularly like having spawn, spawn_local, spawn_local_with_priority, spawn_with_priority, spawn_within, spawn_within_with_priority, ...

That sounds like a nice idea. You probably won't get around distinguishing spawn() and spawn_local() though because of the different trait bounds.

glib/src/futures/mod.rs Outdated Show resolved Hide resolved
self.priority = priority;
self
}
pub fn precision(mut self, precision: SchedulingPrecision) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe the precision should be defined based on whether the Duration has fractional seconds or not? Or is that too implicit?

Otherwise maybe having a sleep(Duration) and sleep_seconds(u32) would be simpler API.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can't make it implicit based on the choosen Duration, because the user could want to actually wait 2000 milliseconds precisely, and not 2 seconds roughly.

What about having the methods precision_seconds, precision_milliseconds, so that we can remove the public enum SchedulingPrecision?

Copy link
Member

Choose a reason for hiding this comment

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

Good point. Yes that would also work

Copy link
Member Author

Choose a reason for hiding this comment

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

Thinking about it... In gtk adding a new enum for a specific function seems to be pretty normal. I think it fits adding an enum here as well

}
}

pub struct Timeout<F: Future> {
Copy link
Member

Choose a reason for hiding this comment

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

This all needs docs

@ranfdev ranfdev force-pushed the futures branch 2 times, most recently from e8c34fd to 5b2608a Compare March 4, 2023 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request glib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants