API discoverability; align ourselves with the wider ecosystem #412
Conversation
|
On your first point, I agree about the second point about some of those functions naturally belonging to the executor. However is probably best to do I'll take a deeper look soon |
|
I just wanna mention that I am gonna make some more extensive comments later today after work, but that I do have some comments. |
|
@bryandmc Are you still planning to comment on this PR? |
HippoBaro
force-pushed
the
task_refactor
branch
3 times, most recently
from
ba73dd6 to
2ffbfac
Compare
|
@bryandmc for some reason the |
examples/cooperative_preempt.rs
Outdated
| // * glommio::executor().yield_task_queue_now(), works like yield_if_needed() | ||
| // but yields unconditionally. | ||
| // | ||
| // * glommio::executor().yield_now(), which unconditional yield the current |
There was a problem hiding this comment.
isn't this the same as the one in futures_lite ? Why go through the executor ?
There was a problem hiding this comment.
Just to be consistent with the other ones really
There was a problem hiding this comment.
A reader will automatically ask themselves "but what is the difference between that and futures::yield_now()`? So it's good to mention that it is the same, just under a different interface.
And while we are at it, let's add doc links
There was a problem hiding this comment.
Alright, I did just that and made lots of corrections to our documentation throughout the crate while at it. We now have better, more uniform comments and documentation with more links as appropriate.
Spawning tasks via a member function of `Task<T>` is not great from an API discovery point of view but also mean we often need to use the starfish syntax such as `Task::<()>::local(...)`. A free function makes it simpler for the type system to infer the return type of the `Task` to create. On a side node, it aligns us with tokio.
Rename them to `scoped_local` and `scoped_local_into`.
Functions such as `create_task_queue` intuitively relate to the executor and not to a `Task`. To make things more easy, we define a free function `executor()` that is now included in the prelude. It returns a proxy struct to the `LocalExecutor`. This is where all executor functions accessible to the user will be located moving forward. Once Rust gets its act together wrt thread-local-storage we will remove this proxy entirely and return a direct reference to the `LocalExecutor` itself. Alas, we can't do that yet.
Also add a free function `yield_if_needed` since it's one of the most used in Glommio apps.
HippoBaro
force-pushed
the
task_refactor
branch
from
ce2cc1e to
ac22f55
Compare
Fixed a bunch of typos, grammar errors, capitalisation issues, etc. Also promoted some comments to actual documentation where appropriate. Added links to actual symbols too when it makes sense.
HippoBaro
force-pushed
the
task_refactor
branch
from
ac22f55 to
6b46730
Compare
|
Glad to see you merged because I agree that it looks good! |
What does this PR do?
Task<T>is not great from anAPI discovery point of view but also means we often need to use the
starfish syntax such as
Task::<()>::local(...). A free function makesit simpler for the type system to infer the return type of the
Tasktocreate. This makes our spawning API similar to that of Tokio for
instance.
create_task_queueintuitively relate to the executorand not to a
Task. To make things easier, we define a free functionexecutor()that is now included in the prelude. It returns a proxystruct to the
LocalExecutor. This is where all executor functionsaccessible to the user will be located moving forward.
Once Rust gets better thread-local-storage we will remove this proxy
entirely and return a direct reference to the
LocalExecutoritself. Alas,we can't do that yet.
Motivation
What inspired you to submit this pull request?
Related issues
A list of issues either fixed, containing architectural discussions, otherwise relevant
for this Pull Request.
Additional Notes
Anything else we should know when reviewing?
Checklist
[] I have added unit tests to the code I am submitting
[] My unit tests cover both failure and success scenarios
[] If applicable, I have discussed my architecture