Implements the waker changes for task API stabilization. #1445

Matthias247 · 2019-01-30T09:43:13Z

This removes LocalWaker, and adds ArcWake to the library.

Matthias247 · 2019-01-30T09:45:41Z

That's my current development snapshot. Requires rust-lang/rust#57992 to get merged.
It doesn't build for me locally due to some issue with the rand crate. Not sure whether that is futures related, or whether I have a bad commit of the nightly rustc version.

There's a ton of churn in this change, but most things are renaming. The important parts will be moving the Waker-related things over, e.g. inside LocalPool and the Compat adapter.

Nemo157 · 2019-01-30T09:59:09Z

@Matthias247 try running cargo update -p rand -p rand_core (and then again with all the actual versions of rand_core detected, or just do a full cargo update), rand used a semver trick to do a non-breaking breaking change which causes issues with cargo's normal minimal update on build.

Matthias247 · 2019-01-31T08:53:09Z

Thanks @Nemo157. cargo update helped and I could compile things now. I fixed all compilation issues and the normal unit tests are running. doctests are not running, likely because my locally compiled rustc doesn't have rustdoc included.

If you want to review it, the main focus should be on components which involved Waker: LocalPool, futures-unordered, ArcWake, waker_ref and the few test wakers.

ebkalderon · 2019-02-15T13:47:28Z

@Matthias247 Any updates on this? Several other pending PRs can't pass CI and are blocked until this one gets merged.

Matthias247 · 2019-02-15T16:24:05Z

@ebkalderon From mv POV that's implemented. The checks were failing because it was never built by CI against the changes in std.
So that needs to happen. And a review.

Nemo157

Mostly looks good to me (the Task code reductions look great).

futures-test/src/task/mod.rs

Nemo157 · 2019-02-15T16:35:36Z

futures-util/src/compat/compat03as01.rs

+        // For simplicity reasons wrap the Waker into an Arc.
+        // We can optmize this again later on and reintroduce WakerLt<'a> which
+        // derefs to Waker, and where cloning it through RawWakerVTable returns
+        // an Arc version


I think you meant "optimize" here and not "optmize?"

This is now cloning the current task twice on every single poll.

Yeah, we'll want to follow up with this optimization pretty quickly since this will cause an allocation every time as_waker is called :(

futures-util/src/task/arc_wake.rs

futures-util/src/task/waker_ref.rs

Nemo157 · 2019-02-15T16:48:28Z

The CI failures seem to be related to the conflicting files after merging this with master.

Matthias247 · 2019-02-15T17:27:51Z

@Nemo157 I tried to resolve the conflicts in the web UI. If that won't work, I will only get to it tonight (PST). If you want to look into it before in order to unblock the library, feel free to merge and resolve it. Shouldn't be large issues.

Matthias247 · 2019-02-16T05:15:39Z

@cramertj , @Nemo157 Everything builds now. The remaining failing test is due to CI testing against an older version of rustc which obviously doesn't have the required changes.

taiki-e · 2019-02-16T07:54:26Z

@Matthias247 Could you change the minimum required version on .travis.yml and README.md from nightly-2019-01-11 to nightly-2019-02-15?

Matthias247 · 2019-02-16T08:13:06Z

@taiki-e Thanks! That helped

futures-intrusive/src/channel/mpmc.rs

Nemo157 · 2019-02-16T13:34:29Z

(Just a note, there are some minor no_std issues with the current changes here, but I have a branch fixing them as I work through updating embrio as well, if this is merged before I finish that update I'll open a separate PR, otherwise I can open a PR to this PR to get those in as well).

EDIT: In order to make sure it's tested properly I'll do the fixes in a separate PR that also fixes #1396.

…is merged

cramertj · 2019-02-19T19:18:45Z

futures-channel/benches/sync_mpsc.rs

@@ -27,20 +27,20 @@ fn unbounded_1_tx(b: &mut Bencher) {
        for i in 0..1000 {

            // Poll, not ready, park
-            assert_eq!(Poll::Pending, rx.poll_next_unpin(lw));
+            assert_eq!(Poll::Pending, rx.poll_next_unpin(waker));


nit: here and elsewhere, I think it'd be fine to abbreviate waker as e.g. wk since it's used so often. I can open this as a separate followup change, though, since it seems potentially controversial.

futures-util/src/task/waker_ref.rs

This removes LocalWaker, and adds ArcWake to the library.

cramertj · 2019-02-19T20:53:50Z

I've gone ahead and rebased this and will merge when tests pass, and plan to follow up with some changes to cleanup a few issues that were brought up here before releasing.

jsancio · 2019-02-23T03:53:00Z

futures-util/src/macros/poll.rs

-/// This macro bakes in propagation of `Err` signals by returning early.
-/// This macro bakes in propagation of `Pending` and `Err` signals by returning early.
-#[macro_export]
-macro_rules! try_poll {


@Matthias247 Why was this macro removed?

@jsancio I am not aware having actively removed this in my change. Maybe it was part of @cramertj's cleanup effort, or a rebase/merge artifact. @cramertj Do you know? Otherwise it might have been a mistake.

In 1339c87#diff-01958fc79efbf83fe4d83d5f9cb5b5ec, @cramertj replaced uses of try_poll! with ?

Great. Thanks for the response.

Yeah, try_poll is kinda useless now that ? works on Poll.

josephg mentioned this pull request Jan 31, 2019

Support stablization track futures API Clikengo/foundationdb-rs#111

Closed

ebkalderon mentioned this pull request Jan 31, 2019

Allow !Unpin Sinks in Forward combinator #1441

Merged

leaxoy mentioned this pull request Feb 15, 2019

Future-core-preview-0.3.0-alpha.12 fails to build on new nightly #1453

Closed

Thomasdezeeuw mentioned this pull request Feb 15, 2019

Update to latest nightly Thomasdezeeuw/heph#163

Closed

Nemo157 approved these changes Feb 15, 2019

View reviewed changes

taiki-e reviewed Feb 16, 2019

View reviewed changes

futures-intrusive/src/channel/mpmc.rs Outdated Show resolved Hide resolved

Matthias247 force-pushed the waker_changes3 branch from ea98f49 to 87b85e2 Compare February 16, 2019 08:55

scampi added a commit to scampi/rustfmt that referenced this pull request Feb 18, 2019

silence build failures of futures-rs until rust-lang/futures-rs#1445 …

00c1259

…is merged

yoshuawuyts mentioned this pull request Feb 19, 2019

nightly std library change breaks romio withoutboats/romio#65

Closed

cramertj reviewed Feb 19, 2019

View reviewed changes

futures-util/src/task/waker_ref.rs Show resolved Hide resolved

Matthias247 and others added 8 commits February 19, 2019 12:37

Implements the waker changes for task API stabilization.

4fc3271

This removes LocalWaker, and adds ArcWake to the library.

Fix remaining compilation errors

af2cc71

Rename drop_fn to drop

b8de4fe

Use function to initialize RawWaker

27a3037

Add some items to gitignore

a7e79c8

fix missing colon

e1aec48

Rename noop/panic waker and fix other compilation issue

f10266f

Fix warnings and rebase artifacts

131c13f

cramertj force-pushed the waker_changes3 branch from 87b85e2 to 131c13f Compare February 19, 2019 20:53

Nemo157 mentioned this pull request Feb 19, 2019

Fix no_std build #1456

Merged

cramertj merged commit 1339c87 into rust-lang:master Feb 19, 2019

Matthias247 deleted the waker_changes3 branch February 20, 2019 02:55

cramertj mentioned this pull request Feb 20, 2019

Cleanup #1457

Merged

taiki-e mentioned this pull request Feb 22, 2019

async-await: update to new future/task API tokio-rs/tokio#919

Merged

jsancio reviewed Feb 23, 2019

View reviewed changes

taiki-e mentioned this pull request Jun 30, 2019

Move ready! macro to futures-core from futures-util #1698

Closed

thedavidmeister mentioned this pull request Jul 13, 2019

bump rust nightly holochain/holochain-rust#1555

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implements the waker changes for task API stabilization. #1445

Implements the waker changes for task API stabilization. #1445

Matthias247 commented Jan 30, 2019

Matthias247 commented Jan 30, 2019

Nemo157 commented Jan 30, 2019

Matthias247 commented Jan 31, 2019

ebkalderon commented Feb 15, 2019

Matthias247 commented Feb 15, 2019

Nemo157 left a comment

Nemo157 Feb 15, 2019

ebkalderon Feb 15, 2019

seanmonstar Feb 16, 2019

cramertj Feb 19, 2019

Nemo157 commented Feb 15, 2019

Matthias247 commented Feb 15, 2019

Matthias247 commented Feb 16, 2019

taiki-e commented Feb 16, 2019

Matthias247 commented Feb 16, 2019

Nemo157 commented Feb 16, 2019 •

edited

Loading

cramertj Feb 19, 2019

cramertj commented Feb 19, 2019

jsancio Feb 23, 2019

Matthias247 Feb 24, 2019

jebrosen Feb 24, 2019

jsancio Feb 25, 2019

cramertj Feb 25, 2019

Implements the waker changes for task API stabilization. #1445

Implements the waker changes for task API stabilization. #1445

Conversation

Matthias247 commented Jan 30, 2019

Matthias247 commented Jan 30, 2019

Nemo157 commented Jan 30, 2019

Matthias247 commented Jan 31, 2019

ebkalderon commented Feb 15, 2019

Matthias247 commented Feb 15, 2019

Nemo157 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

Nemo157 commented Feb 15, 2019

Matthias247 commented Feb 15, 2019

Matthias247 commented Feb 16, 2019

taiki-e commented Feb 16, 2019

Matthias247 commented Feb 16, 2019

Nemo157 commented Feb 16, 2019 • edited Loading

Choose a reason for hiding this comment

cramertj commented Feb 19, 2019

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

Nemo157 commented Feb 16, 2019 •

edited

Loading