From 184e6fbff8eeda94c134cce63ee6e278b1a29be2 Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Wed, 1 Aug 2018 17:20:51 -0700 Subject: [PATCH 1/5] Simplify select --- futures-util/src/async_await/mod.rs | 1 + futures-util/src/async_await/select.rs | 16 +++++++++------- futures-util/src/future/mod.rs | 2 +- futures-util/src/sink/mod.rs | 2 +- futures-util/src/stream/mod.rs | 2 +- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/futures-util/src/async_await/mod.rs b/futures-util/src/async_await/mod.rs index 1dcc5f01b5..307e254024 100644 --- a/futures-util/src/async_await/mod.rs +++ b/futures-util/src/async_await/mod.rs @@ -21,4 +21,5 @@ mod join; mod select; #[doc(hidden)] +#[inline(always)] pub fn assert_unpin(_: &T) {} diff --git a/futures-util/src/async_await/select.rs b/futures-util/src/async_await/select.rs index ae86c4a85c..85fb617b87 100644 --- a/futures-util/src/async_await/select.rs +++ b/futures-util/src/async_await/select.rs @@ -47,16 +47,18 @@ macro_rules! select { )* } - let __priv_res = loop { + let __priv_res = await!($crate::future::poll_fn(|cx| { $( - let poll_res = $crate::poll!($crate::core_reexport::mem::PinMut::new( - &mut $name)); - if let $crate::core_reexport::task::Poll::Ready(x) = poll_res { - break __PrivResult::$name(x); + match $crate::future::Future::poll( + $crate::core_reexport::mem::PinMut::new(&mut $name), cx) + { + $crate::core_reexport::task::Poll::Ready(x) => + return $crate::core_reexport::task::Poll::Ready(__PrivResult::$name(x)), + $crate::core_reexport::task::Poll::Pending => {}, } )* - $crate::pending!(); - }; + $crate::core_reexport::task::Poll::Pending + })); match __priv_res { $( __PrivResult::$name($name) => { diff --git a/futures-util/src/future/mod.rs b/futures-util/src/future/mod.rs index 6ef478c9e7..a0afe5968a 100644 --- a/futures-util/src/future/mod.rs +++ b/futures-util/src/future/mod.rs @@ -5,7 +5,7 @@ use core::marker::Unpin; use core::mem::PinMut; -use futures_core::future::Future; +pub use futures_core::future::Future; use futures_core::stream::Stream; use futures_core::task::{self, Poll, Executor}; diff --git a/futures-util/src/sink/mod.rs b/futures-util/src/sink/mod.rs index 965cbf4080..18c7a04dc8 100644 --- a/futures-util/src/sink/mod.rs +++ b/futures-util/src/sink/mod.rs @@ -7,7 +7,7 @@ use core::marker::Unpin; use either::Either; use futures_core::future::Future; use futures_core::stream::Stream; -use futures_sink::Sink; +pub use futures_sink::Sink; mod close; pub use self::close::Close; diff --git a/futures-util/src/stream/mod.rs b/futures-util/src/stream/mod.rs index 83f0c414b9..ebc169c490 100644 --- a/futures-util/src/stream/mod.rs +++ b/futures-util/src/stream/mod.rs @@ -7,7 +7,7 @@ use core::marker::Unpin; use core::mem::PinMut; use either::Either; use futures_core::future::Future; -use futures_core::stream::Stream; +pub use futures_core::stream::Stream; use futures_core::task::{self, Poll}; use futures_sink::Sink; From 4165fb3f12dd6e333331ed9e0eb38932e4b9f1a6 Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Wed, 1 Aug 2018 17:48:17 -0700 Subject: [PATCH 2/5] Use poll_fn in join! implementation --- futures-util/src/async_await/join.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/futures-util/src/async_await/join.rs b/futures-util/src/async_await/join.rs index 3e3f536ff9..0bc2850a5f 100644 --- a/futures-util/src/async_await/join.rs +++ b/futures-util/src/async_await/join.rs @@ -28,19 +28,19 @@ macro_rules! join { let mut $fut = $crate::future::maybe_done($fut); $crate::pin_mut!($fut); )* - loop { + await!($crate::future::poll_fn(|cx| { let mut all_done = true; $( - if $crate::poll!($fut.reborrow()).is_pending() { + if $crate::future::Future::poll($fut.reborrow(), cx).is_pending() { all_done = false; } )* if all_done { - break; + $crate::core_reexport::task::Poll::Ready(()) } else { - $crate::pending!(); + $crate::core_reexport::task::Poll::Pending } - } + })); ($( $fut.reborrow().take_output().unwrap(), @@ -94,25 +94,29 @@ macro_rules! try_join { let mut $fut = $crate::future::maybe_done($fut); $crate::pin_mut!($fut); )* - let res: $crate::core_reexport::result::Result<(), _> = loop { + + let res: $crate::core_reexport::result::Result<(), _> = await!($crate::future::poll_fn(|cx| { let mut all_done = true; $( - if $crate::poll!($fut.reborrow()).is_pending() { + if $crate::future::Future::poll($fut.reborrow(), cx).is_pending() { all_done = false; } else if $fut.reborrow().output_mut().unwrap().is_err() { // `.err().unwrap()` rather than `.unwrap_err()` so that we don't introduce // a `T: Debug` bound. - break $crate::core_reexport::result::Result::Err( - $fut.reborrow().take_output().unwrap().err().unwrap() + return $crate::core_reexport::task::Poll::Ready( + $crate::core_reexport::result::Result::Err( + $fut.reborrow().take_output().unwrap().err().unwrap() + ) ); } )* if all_done { - break $crate::core_reexport::result::Result::Ok(()); + $crate::core_reexport::task::Poll::Ready( + $crate::core_reexport::result::Result::Ok(())) } else { - $crate::pending!(); + $crate::core_reexport::task::Poll::Pending } - }; + })); res.map(|()| ($( // `.ok().unwrap()` rather than `.unwrap()` so that we don't introduce From c74aacc309addefe99425965f8099e1479aa2a7c Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Wed, 1 Aug 2018 17:48:46 -0700 Subject: [PATCH 3/5] Add tests to prevent size regressions in async/await macros --- futures/tests/async_await_macros.rs | 55 ++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/futures/tests/async_await_macros.rs b/futures/tests/async_await_macros.rs index 706d4d23df..5033fd29d4 100644 --- a/futures/tests/async_await_macros.rs +++ b/futures/tests/async_await_macros.rs @@ -1,6 +1,6 @@ #![feature(async_await, await_macro, pin, arbitrary_self_types, futures_api)] -use futures::{Poll, pending, poll, pin_mut, join, select}; +use futures::{Poll, future, pending, poll, pin_mut, join, try_join, select}; use futures::channel::oneshot; use futures::executor::block_on; @@ -75,3 +75,56 @@ fn select_can_move_uncompleted_futures() { }); assert!(ran); } + +#[test] +fn select_size() { + let fut = async { + let mut ready = future::ready(0i32); + select! { + ready => {}, + } + }; + assert_eq!(::std::mem::size_of_val(&fut), 40); + + let fut = async { + let mut ready1 = future::ready(0i32); + let mut ready2 = future::ready(0i32); + select! { + ready1 => {}, + ready2 => {}, + } + }; + assert_eq!(::std::mem::size_of_val(&fut), 56); +} + +#[test] +fn join_size() { + let fut = async { + let ready = future::ready(0i32); + join!(ready) + }; + assert_eq!(::std::mem::size_of_val(&fut), 48); + + let fut = async { + let ready1 = future::ready(0i32); + let ready2 = future::ready(0i32); + join!(ready1, ready2) + }; + assert_eq!(::std::mem::size_of_val(&fut), 72); +} + +#[test] +fn try_join_size() { + let fut = async { + let ready = future::ready(Ok::(0)); + try_join!(ready) + }; + assert_eq!(::std::mem::size_of_val(&fut), 48); + + let fut = async { + let ready1 = future::ready(Ok::(0)); + let ready2 = future::ready(Ok::(0)); + try_join!(ready1, ready2) + }; + assert_eq!(::std::mem::size_of_val(&fut), 80); +} From 4e299633321033597cdf3f0cca6f34246fa242c2 Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Wed, 1 Aug 2018 18:19:29 -0700 Subject: [PATCH 4/5] Use move closures in join! to prevent unnecessary upvar storage --- futures-util/src/async_await/join.rs | 26 ++++++++++++-------------- futures/tests/async_await_macros.rs | 8 ++++---- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/futures-util/src/async_await/join.rs b/futures-util/src/async_await/join.rs index 0bc2850a5f..45c9a7f52f 100644 --- a/futures-util/src/async_await/join.rs +++ b/futures-util/src/async_await/join.rs @@ -28,7 +28,7 @@ macro_rules! join { let mut $fut = $crate::future::maybe_done($fut); $crate::pin_mut!($fut); )* - await!($crate::future::poll_fn(|cx| { + await!($crate::future::poll_fn(move |cx| { let mut all_done = true; $( if $crate::future::Future::poll($fut.reborrow(), cx).is_pending() { @@ -36,15 +36,13 @@ macro_rules! join { } )* if all_done { - $crate::core_reexport::task::Poll::Ready(()) + $crate::core_reexport::task::Poll::Ready(($( + $fut.reborrow().take_output().unwrap(), + )*)) } else { $crate::core_reexport::task::Poll::Pending } - })); - - ($( - $fut.reborrow().take_output().unwrap(), - )*) + })) } } } @@ -95,7 +93,7 @@ macro_rules! try_join { $crate::pin_mut!($fut); )* - let res: $crate::core_reexport::result::Result<(), _> = await!($crate::future::poll_fn(|cx| { + let res: $crate::core_reexport::result::Result<_, _> = await!($crate::future::poll_fn(move |cx| { let mut all_done = true; $( if $crate::future::Future::poll($fut.reborrow(), cx).is_pending() { @@ -112,16 +110,16 @@ macro_rules! try_join { )* if all_done { $crate::core_reexport::task::Poll::Ready( - $crate::core_reexport::result::Result::Ok(())) + $crate::core_reexport::result::Result::Ok(($( + // `.ok().unwrap()` rather than `.unwrap()` so that we don't introduce + // an `E: Debug` bound. + $fut.reborrow().take_output().unwrap().ok().unwrap(), + )*))) } else { $crate::core_reexport::task::Poll::Pending } })); - res.map(|()| ($( - // `.ok().unwrap()` rather than `.unwrap()` so that we don't introduce - // an `E: Debug` bound. - $fut.reborrow().take_output().unwrap().ok().unwrap(), - )*)) + res } } } diff --git a/futures/tests/async_await_macros.rs b/futures/tests/async_await_macros.rs index 5033fd29d4..b74d56a8e0 100644 --- a/futures/tests/async_await_macros.rs +++ b/futures/tests/async_await_macros.rs @@ -103,14 +103,14 @@ fn join_size() { let ready = future::ready(0i32); join!(ready) }; - assert_eq!(::std::mem::size_of_val(&fut), 48); + assert_eq!(::std::mem::size_of_val(&fut), 40); let fut = async { let ready1 = future::ready(0i32); let ready2 = future::ready(0i32); join!(ready1, ready2) }; - assert_eq!(::std::mem::size_of_val(&fut), 72); + assert_eq!(::std::mem::size_of_val(&fut), 64); } #[test] @@ -119,12 +119,12 @@ fn try_join_size() { let ready = future::ready(Ok::(0)); try_join!(ready) }; - assert_eq!(::std::mem::size_of_val(&fut), 48); + assert_eq!(::std::mem::size_of_val(&fut), 40); let fut = async { let ready1 = future::ready(Ok::(0)); let ready2 = future::ready(Ok::(0)); try_join!(ready1, ready2) }; - assert_eq!(::std::mem::size_of_val(&fut), 80); + assert_eq!(::std::mem::size_of_val(&fut), 64); } From 393f733b3105001c6d2edc15fd2908697f70c0b7 Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Thu, 2 Aug 2018 10:11:46 -0700 Subject: [PATCH 5/5] Use $crate::core_reexport::future::Future rather than reexporting separately --- futures-util/src/async_await/join.rs | 13 +++++++------ futures-util/src/async_await/select.rs | 2 +- futures-util/src/future/mod.rs | 2 +- futures-util/src/sink/mod.rs | 2 +- futures-util/src/stream/mod.rs | 2 +- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/futures-util/src/async_await/join.rs b/futures-util/src/async_await/join.rs index 45c9a7f52f..06883d6991 100644 --- a/futures-util/src/async_await/join.rs +++ b/futures-util/src/async_await/join.rs @@ -31,7 +31,7 @@ macro_rules! join { await!($crate::future::poll_fn(move |cx| { let mut all_done = true; $( - if $crate::future::Future::poll($fut.reborrow(), cx).is_pending() { + if $crate::core_reexport::future::Future::poll($fut.reborrow(), cx).is_pending() { all_done = false; } )* @@ -96,7 +96,7 @@ macro_rules! try_join { let res: $crate::core_reexport::result::Result<_, _> = await!($crate::future::poll_fn(move |cx| { let mut all_done = true; $( - if $crate::future::Future::poll($fut.reborrow(), cx).is_pending() { + if $crate::core_reexport::future::Future::poll($fut.reborrow(), cx).is_pending() { all_done = false; } else if $fut.reborrow().output_mut().unwrap().is_err() { // `.err().unwrap()` rather than `.unwrap_err()` so that we don't introduce @@ -111,10 +111,11 @@ macro_rules! try_join { if all_done { $crate::core_reexport::task::Poll::Ready( $crate::core_reexport::result::Result::Ok(($( - // `.ok().unwrap()` rather than `.unwrap()` so that we don't introduce - // an `E: Debug` bound. - $fut.reborrow().take_output().unwrap().ok().unwrap(), - )*))) + // `.ok().unwrap()` rather than `.unwrap()` so that we don't introduce + // an `E: Debug` bound. + $fut.reborrow().take_output().unwrap().ok().unwrap(), + )*)) + ) } else { $crate::core_reexport::task::Poll::Pending } diff --git a/futures-util/src/async_await/select.rs b/futures-util/src/async_await/select.rs index 85fb617b87..8b84d2fd5d 100644 --- a/futures-util/src/async_await/select.rs +++ b/futures-util/src/async_await/select.rs @@ -49,7 +49,7 @@ macro_rules! select { let __priv_res = await!($crate::future::poll_fn(|cx| { $( - match $crate::future::Future::poll( + match $crate::core_reexport::future::Future::poll( $crate::core_reexport::mem::PinMut::new(&mut $name), cx) { $crate::core_reexport::task::Poll::Ready(x) => diff --git a/futures-util/src/future/mod.rs b/futures-util/src/future/mod.rs index a0afe5968a..6ef478c9e7 100644 --- a/futures-util/src/future/mod.rs +++ b/futures-util/src/future/mod.rs @@ -5,7 +5,7 @@ use core::marker::Unpin; use core::mem::PinMut; -pub use futures_core::future::Future; +use futures_core::future::Future; use futures_core::stream::Stream; use futures_core::task::{self, Poll, Executor}; diff --git a/futures-util/src/sink/mod.rs b/futures-util/src/sink/mod.rs index 18c7a04dc8..965cbf4080 100644 --- a/futures-util/src/sink/mod.rs +++ b/futures-util/src/sink/mod.rs @@ -7,7 +7,7 @@ use core::marker::Unpin; use either::Either; use futures_core::future::Future; use futures_core::stream::Stream; -pub use futures_sink::Sink; +use futures_sink::Sink; mod close; pub use self::close::Close; diff --git a/futures-util/src/stream/mod.rs b/futures-util/src/stream/mod.rs index ebc169c490..83f0c414b9 100644 --- a/futures-util/src/stream/mod.rs +++ b/futures-util/src/stream/mod.rs @@ -7,7 +7,7 @@ use core::marker::Unpin; use core::mem::PinMut; use either::Either; use futures_core::future::Future; -pub use futures_core::stream::Stream; +use futures_core::stream::Stream; use futures_core::task::{self, Poll}; use futures_sink::Sink;