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

compat: add a compat runtime #1663

Merged
merged 44 commits into from
Nov 1, 2019
Merged

compat: add a compat runtime #1663

merged 44 commits into from
Nov 1, 2019

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Oct 15, 2019

Motivation

The futures crate's compat module provides
interoperability between futures 0.1 and std::future future types
(e.g. implementing std::future::Future for a type that implements the
futures 0.1 Future trait). However, this on its own is insufficient
to run code written against tokio 0.1 on a tokio 0.2 runtime, if
that code also relies on tokio's runtime services. If legacy tasks are
executed that rely on tokio::timer, perform IO using tokio's
reactor, or call tokio::spawn, those API calls will fail unless there
is also a runtime compatibility layer.

Solution

As proposed in #1549, this branch introduces a new tokio-compat crate,
with implementations of the thread pool and current-thread runtimes that
are capable of running both tokio 0.1 and tokio 0.2 tasks. The compat
runtime creates a background thread that runs a tokio 0.1 timer and
reactor, and sets itself as the tokio 0.1 executor as well as the
default 0.2 executor. This allows 0.1 futures that use 0.1 timer,
reactor, and executor APIs may run alongside std::future tasks on the
0.2 runtime.

Examples

Spawning both tokio 0.1 and tokio 0.2 futures:

use futures_01::future::lazy;

tokio_compat::run(lazy(|| {
    // spawn a `futures` 0.1 future using the `spawn` function from the
    // `tokio` 0.1 crate:
    tokio_01::spawn(lazy(|| {
        println!("hello from tokio 0.1!");
        Ok(())
    }));

    // spawn an `async` block future on the same runtime using `tokio`
    // 0.2's `spawn`:
    tokio_02::spawn(async {
        println!("hello from tokio 0.2!");
    });

    Ok(())
}))

Futures on the compat runtime can use timer APIs from both 0.1 and 0.2
versions of tokio:

use std::time::{Duration, Instant};
use futures_01::future::lazy;
use tokio_compat::prelude::*;

tokio_compat::run_03(async {
    // Wait for a `tokio` 0.1 `Delay`...
    let when = Instant::now() + Duration::from_millis(10);
    tokio_01::timer::Delay::new(when)
        // convert the delay future into a `std::future` that we can `await`.
        .compat()
        .await
        .expect("tokio 0.1 timer should work!");
    println!("10 ms have elapsed");

    // Wait for a `tokio` 0.2 `Delay`...
    let when = Instant::now() + Duration::from_millis(20);
    tokio_02::timer::delay(when).await;
    println!("20 ms have elapsed");
});

Future Work

This is just an initial implementation of a tokio-compat crate; there
are more compatibility layers we'll want to provide before that crate is
complete. For example, we should also provide compatibility between
tokio 0.2's AsyncRead and AsyncWrite traits and the futures 0.1
and futures 0.3 versions of those traits. In #1549, @carllerche also
suggests that the compat crate provide reimplementations of APIs that
were removed from tokio 0.2 proper, such as the tcp::Incoming
future.

Additionally, there is likely extra work required to get the
tokio-threadpool 0.1 blocking APIs to work on the compat runtime.
This will be addressed in a follow-up PR.

Fixes: #1605
Fixes: #1552
Refs: #1549

@hawkw hawkw requested review from carllerche and a team October 15, 2019 23:28
@hawkw hawkw self-assigned this Oct 15, 2019
@hawkw hawkw added the C-enhancement Category: A PR with an enhancement or bugfix. label Oct 15, 2019
@hawkw
Copy link
Member Author

hawkw commented Oct 15, 2019

AFAICT the FreeBSD CI failures are spurious and appear to be occuring on master as well (https://github.com/tokio-rs/tokio/runs/260482009)

Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

This looks great, nothing blocking on my end. I'd really like to ship this ASAP and get some sort of blog post up about how to use it. Thoughts @carllerche?

@hawkw
Copy link
Member Author

hawkw commented Oct 17, 2019

@LucioFranco

I'd really like to ship this ASAP and get some sort of blog post up about how to use it.

I'm definitely planning on doing a blog post! There are a couple things missing from the compat crate (CurrentThread compat runtime, AsyncRead/AsyncWrite adapters...) that I'd like to get in before we release it, though.

hawkw and others added 11 commits October 17, 2019 10:11
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Thanks @LucioFranco!

Co-Authored-By: Lucio Franco <luciofranco14@gmail.com>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@LucioFranco
Copy link
Member

@hawkw those sound like a good plan to me

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw
Copy link
Member Author

hawkw commented Oct 31, 2019

I don't think it's possible for tokio 0.1's blocking to work on the compat runtime. This is because (unlike the timer, executor, and reactor), there's no way to override the global blocking functionality. I think the best solution is for tokio-compat to provide its own blocking for 0.1 futures.

Unless there are any objections, I think we might want to do that in a subsequent PR. (cc @carllerche)

@carllerche
Copy link
Member

@hawkw ah, I see. That would mean stuff like tokio-fs doesn't work...

I have another thought... hear me out, and feel free to shoot it down. Have we considered updating tokio 0.1 to include a tokio-next-compat feature flag? If it were enabled, it would swap the impl details w/ 0.2's impl. I don't know if this would make things easier or harder...

@carllerche
Copy link
Member

We can also add the necessary hooks to 0.1 to support blocking compat.

@hawkw
Copy link
Member Author

hawkw commented Nov 1, 2019

Yeah, I think in order to get the ideal compat story for blocking, we'll need to make changes to the 0.1 runtime as well. I'm open to doing that, but I think it would be better to roll forward with these changes, if possible, and then move on to getting 0.1 blocking to work.

@carllerche
Copy link
Member

👍 I'll leave the decisions to you & @LucioFranco. I'm just throwing out ideas.

Cargo.toml Show resolved Hide resolved
@LucioFranco
Copy link
Member

Things left are to fix block_on futures not getting a timer.

I think @taiki-e is right we need to add tokio back to the workspace.

Otherwise, I am happy with this change, the blocking stuff is somewhat not as important imo. Most users probably don't use that as much? For my use case I can easily swap out our blocking code.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
these pass, but it's good to have more tests

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw
Copy link
Member Author

hawkw commented Nov 1, 2019

Things left are to fix block_on futures not getting a timer.

Yup, looks like the tokio 0.1 timer was not being set in calls to Runtime::block_on (with the thread pool runtime), thanks for catching that! I've added tests for this in 8505ee0 and cd2d646, and fixed it in 477a417!

I think @taiki-e is right we need to add tokio back to the workspace.

Fixed that one too, my bad!

Otherwise, I am happy with this change, the blocking stuff is somewhat not as important imo. Most users probably don't use that as much? For my use case I can easily swap out our blocking code.

I've added a note documenting that the compat runtime doesn't currently support legacy blocking. I think adding support for blocking is best done in a follow-up.

Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

Lets get this merged, I will contiue to look for issues in trying to convert my codebase over.

@hawkw hawkw merged commit e699d46 into master Nov 1, 2019
carllerche added a commit to tokio-rs/tokio-compat that referenced this pull request Nov 1, 2019
@carllerche carllerche deleted the eliza/compat branch November 1, 2019 21:06
hawkw added a commit that referenced this pull request Nov 12, 2019
## Motivation

The initial version of `tokio-compat`'s compatibility runtime added in
#1663 doesn't support the calls to `tokio_threadpool` 0.1's `blocking`.
This is because (unlike the timer, executor, and reactor), there's no
way to override the global `blocking` functionality in
`tokio-threadpool`.

## Solution

As discussed [here][1], this branch adds APIs to the v0.1.x version of
`tokio-threadpool` that allow overriding the behavior used by calls to
`blocking`. The threadpool crate now exposes `blocking::set_default` and
`blocking::with_default` functions, like `executor`, `timer`, and
`reactor`. This will allow `tokio-compat` to override calls to 0.1's
`blocking` to use the new `tokio` 0.2 blocking APIs.

Unlike the similar APIs in `executor`, `timer`, and `reactor`, the hooks
for overriding blocking behaviour are `#[doc(hidden)]` and have comments
warning against their use outside of `tokio-compat`. In general, there 
probably won't be a compelling reason to override these outside of the 
compatibility layer.

Refs: #1722

[1]: #1663 (comment)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: A PR with an enhancement or bugfix.
Projects
None yet
6 participants