feat: use smol as runtime #757

dignifiedquire · 2020-04-26T18:53:08Z

There is a new runtime in town called smol, you will hear more in the coming days from @stjepang.

This PR drops the internal runtime and instead use smol. Based on this, this PR brings us the following features

Speed, I will post more benchmarks soon, but all things are pointing at considerable speed improvements
spawn_local support, as smol already implemented it, it was easy to add
wasm support, compiles now out of the box for wasm. This was much easier to add, as the runtime already was isolated.

src/task/spawn_local.rs

skade · 2020-04-26T21:36:49Z

I'm a bit hesitant on adding spawn_local straight away. We have a slow unstructured creep on the runtime interface without having a written direction where we are going.

Can we hold back on this aspect? I know it is substantial for WASM, but we should have a chat on our ongoing direction first.

dignifiedquire · 2020-04-27T09:09:26Z

@skade how about we mark spawn_local as unstable as compromise? So far #457 hasn't gotten much attention and the way it works right now, it at least mirrors the way spawn works nicely. Not having spawn_local has been stopping me from using async-std in multiple projects already.

dignifiedquire · 2020-04-28T09:44:54Z

@k-nasa do you have any idea why the no-std build is failing. It seems to be that futures-core/alloc doesn't compile anymore, but I didn't change anything in that regard asfaik.

error[E0463]: can't find crate for `std`
##[error]  |
  = note: the `thumbv7m-none-eabi` target may not be installed

error: aborting due to previous error
##[error]aborting due to previous error
For more information about this error, try `rustc --explain E0463`.
error: could not compile `futures-core`.
##[error]could not compile `futures-core`.
To learn more, run the command again with --verbose.
##[error]The process '/usr/share/rust/.cargo/bin/cargo' failed with exit code 101

k-nasa · 2020-04-28T12:46:02Z

@dignifiedquire I researched 🔍

Try the following command.
I think you can probably build it.

cargo build --no-default-features --features alloc --target thumbv7m-none-eabi -Z avoid-dev-deps -Zfeatures=itarget

It seems to me that this is the part that caused the build to fail. By commenting out, you can build without adding -Zfeatures=itarget. Perhaps the problem is that wasm-timer and futures-channel contain futures-core/std.

[target.'cfg(target_arch = "wasm32")'.dependencies]
wasm-timer = "0.2.4"
wasm-bindgen-futures = "0.4.10"
futures-channel = "0.3.4"

rust-lang/cargo#7914

dignifiedquire · 2020-04-28T18:54:08Z

@k-nasa thanks, those dependencies where the issue, managed to fix it

skade · 2020-05-06T15:29:05Z

After chatting with @dignifiedquire, I want to retract my hesitation to add spawn_local.

yoshuawuyts

This is looking fantastic. Amazing work @dignifiedquire!

I did spot a single regression regarding the task::spawn log point which ideally could be fixed before merging. And there's a few formatting nits, but those shouldn't be a reason to hold of merging.

Incredibly excited for this!

yoshuawuyts · 2020-05-07T11:02:32Z

Cargo.toml

+[target.'cfg(target_arch = "wasm32")'.dependencies]
+wasm-timer = { version = "0.2.4", optional = true }
+wasm-bindgen-futures = { version = "0.4.10", optional = true }
+futures-channel = { version = "0.3.4", optional = true }


Ideally we could use our own channels in this context, which already work. But for the purpose of this PR this seems fine.

They didn't quite do what I needed, we can improve on that later on.

yoshuawuyts · 2020-05-07T11:03:53Z

src/io/read/mod.rs

 assert_eq!(&other_buffer, &[5, 6, 7, 8]);
- Ok(())
- })
+ });


Why was ? removed in this file? Is that a stylistic choice, or does this point to a change in behavior?

because block_on can't return a value in wasm, making this blow up, now that I cfged that out I can undo that change probably

yoshuawuyts · 2020-05-07T11:04:40Z

src/net/tcp/listener.rs

@@ -230,29 +223,31 @@ cfg_unix! {

 impl IntoRawFd for TcpListener {
 fn into_raw_fd(self) -> RawFd {
- self.watcher.into_inner().into_raw_fd()
+ self.watcher.into_raw_fd()


💯 -- I'm excited about the simplifications here.

src/net/mod.rs

yoshuawuyts · 2020-05-07T11:30:51Z

Cargo.toml

+[target.'cfg(target_arch = "wasm32")'.dev-dependencies]
+wasm-bindgen-test = "0.3.10"


This is so exciting!

yoshuawuyts · 2020-05-07T11:44:02Z

src/task/builder.rs

- // Log this `spawn` operation.
- trace!("spawn", {
- task_id: task.id().0,
- parent_task_id: Task::get_current(|t| t.id().0).unwrap_or(0),


We should keep this functionality. If performance is a concern we can make it a no-op unless enabled:

if log_enabled!(Trace) { trace!("spawn", { task_id: wrapped.tag.id().0, parent_task_id: TaskLocalsWrapper::get_current(|t| t.id().0).unwrap_or(0), }); }

Though not documented this is meant for public consumption. If it's at all possible to prevent regressing here I think it'd be worthwhile.

Not necessarily in scope for this PR: but having log points on Drop and cancel would be useful as well. Being able to trace task lifecycles is a useful capability to expose.

Now I remember why I didn't add it, because it is a pain in the current setup :( Will give it some more thought

note that tracing-futures gives you good possibilities as well to track task lifetime if desired.

src/task/join_handle.rs

src/task/yield_now.rs

ordian · 2020-09-24T15:42:24Z

src/task/block_on.rs

+/// Spawns a task and waits for it to finish.
+#[cfg(target_os = "unknown")]
+pub fn block_on<F, T>(future: F)
 where
- F: Future<Output = T>,
+ F: Future<Output = T> + 'static,
+ T: 'static,
 {
- thread_local! {
- // May hold a pre-allocated parker that can be reused for efficiency.
- //
- // Note that each invocation of `block` needs its own parker. In particular, if `block`
- // recursively calls itself, we must make sure that each recursive call uses a distinct
- // parker instance.
- static CACHE: Cell<Option<Arc<Parker>>> = Cell::new(None);
- }
-
- // Virtual table for wakers based on `Arc<Parker>`.
- static VTABLE: RawWakerVTable = {
- unsafe fn clone_raw(ptr: *const ()) -> RawWaker {
- let arc = ManuallyDrop::new(Arc::from_raw(ptr as *const Parker));
- #[allow(clippy::redundant_clone)]
- mem::forget(arc.clone());
- RawWaker::new(ptr, &VTABLE)
- }
-
- unsafe fn wake_raw(ptr: *const ()) {
- let arc = Arc::from_raw(ptr as *const Parker);
- arc.unparker().unpark();
- }
-
- unsafe fn wake_by_ref_raw(ptr: *const ()) {
- let arc = ManuallyDrop::new(Arc::from_raw(ptr as *const Parker));
- arc.unparker().unpark();
- }
-
- unsafe fn drop_raw(ptr: *const ()) {
- drop(Arc::from_raw(ptr as *const Parker))
- }
-
- RawWakerVTable::new(clone_raw, wake_raw, wake_by_ref_raw, drop_raw)
- };
-
- // Pin the future on the stack.
- pin_utils::pin_mut!(future);
-
- CACHE.with(|cache| {
- // Reuse a cached parker or create a new one for this invocation of `block`.
- let arc_parker: Arc<Parker> = cache.take().unwrap_or_else(|| Arc::new(Parker::new()));
- let ptr = (&*arc_parker as *const Parker) as *const ();
-
- // Create a waker and task context.
- let waker = unsafe { ManuallyDrop::new(Waker::from_raw(RawWaker::new(ptr, &VTABLE))) };
- let cx = &mut Context::from_waker(&waker);
-
- loop {
- if let Poll::Ready(t) = future.as_mut().poll(cx) {
- // Save the parker for the next invocation of `block`.
- cache.set(Some(arc_parker));
- return t;
- }
-
- arc_parker.park();
- }
- })
+ Builder::new().local(future).unwrap();


it doesn't seem to wait for the task to finish

skade reviewed Apr 26, 2020

View reviewed changes

src/task/spawn_local.rs Outdated Show resolved Hide resolved

luover mentioned this pull request Apr 27, 2020

support soml runtime ntex-rs/ntex#15

Closed

ghost mentioned this pull request Apr 27, 2020

Task-locals? smol-rs/smol#15

Closed

dignifiedquire marked this pull request as ready for review May 2, 2020 18:27

dignifiedquire added 21 commits May 2, 2020 20:27

add dependency

690ab16

switch to smol instead of an internal runtime

1308fbd

keep std::sync::Arc

fc9ee0d

fix spawning

e082634

bring back random

75ab721

implement task locals

b96afc4

avoid boxing futures

f5fa0d7

switch to smol::Timer

ab9d655

add timeout stress test

fd6ae40

silence must use

10c8b9a

update to work on smol/master

0a7a52a

feat: add spawn_local

228cc59

add some missing windows imports

3161a4e

remove unused dependencies

2cd2ba3

feat: add basic wasm support

e4df140

use published smol

804a52b

fix feature settings

48dd683

remove invalid doc comment

280b1a4

update smol

7a9afbd

fix windows trait declarations for rawsocket

1a6d4f6

mark spawn_local unstable

9253261

dignifiedquire added 4 commits May 2, 2020 20:27

fix windows traits

e092846

make wasm deps part of std

26f62aa

increase timeouts

1214bc2

fix: use run instead of block_on

faea222

dignifiedquire force-pushed the feat/smol branch from f0bcd5c to faea222 Compare May 2, 2020 18:27

dignifiedquire mentioned this pull request May 2, 2020

Not joining all threads = memory leaked on process exit #759

Open

yoshuawuyts approved these changes May 7, 2020

View reviewed changes

ghost mentioned this pull request May 7, 2020

Wasm support smol-rs/smol#87

Open

cr: bring back trace call

27c605b

dignifiedquire mentioned this pull request May 7, 2020

Upgrade to mio 0.7 #742

Closed

dignifiedquire merged commit 10f7abb into async-rs:master May 7, 2020

dignifiedquire deleted the feat/smol branch May 7, 2020 19:21

This was referenced May 7, 2020

Make async-std compile for wasm32-unknown-unknown #225

Closed

[tracking] browser WASM support #220

Open

prepare v1.6.0-beta.1 #765

Merged

leo60228 mentioned this pull request May 7, 2020

Expose net::driver::Watcher mechanism #293

Closed

svyatonik mentioned this pull request Jun 3, 2020

Problems with new async-std paritytech/parity-bridges-common#118

Closed

ordian reviewed Sep 24, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use smol as runtime #757

feat: use smol as runtime #757

dignifiedquire commented Apr 26, 2020 •

edited

Loading

skade commented Apr 26, 2020

dignifiedquire commented Apr 27, 2020

dignifiedquire commented Apr 28, 2020

k-nasa commented Apr 28, 2020

dignifiedquire commented Apr 28, 2020

skade commented May 6, 2020

yoshuawuyts left a comment

yoshuawuyts May 7, 2020

dignifiedquire May 7, 2020

yoshuawuyts May 7, 2020

dignifiedquire May 7, 2020

yoshuawuyts May 7, 2020

yoshuawuyts May 7, 2020

yoshuawuyts May 7, 2020

yoshuawuyts May 7, 2020

dignifiedquire May 7, 2020

najamelan May 9, 2020

ordian Sep 24, 2020

		[target.'cfg(target_arch = "wasm32")'.dev-dependencies]
		wasm-bindgen-test = "0.3.10"

feat: use smol as runtime #757

feat: use smol as runtime #757

Conversation

dignifiedquire commented Apr 26, 2020 • edited Loading

skade commented Apr 26, 2020

dignifiedquire commented Apr 27, 2020

dignifiedquire commented Apr 28, 2020

k-nasa commented Apr 28, 2020

dignifiedquire commented Apr 28, 2020

skade commented May 6, 2020

yoshuawuyts left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dignifiedquire commented Apr 26, 2020 •

edited

Loading