Skip to content
This repository has been archived by the owner on Mar 25, 2023. It is now read-only.

actix-gone but stuff not compiling #532

Closed
wants to merge 5 commits into from
Closed

Conversation

amitu
Copy link
Contributor

@amitu amitu commented Sep 27, 2022

This PR brings down total dependencies from 328 to 275. But fails to compile:

    Checking fpm v0.1.11 (/Users/amitu/Projects/fpm)
error: future cannot be sent between threads safely
   --> src/commands/serve.rs:353:75
    |
353 |     let server = hyper::Server::bind(&tcp_listener.local_addr().unwrap()).serve(make_svc);
    |                                                                           ^^^^^ future returned by `route` is not `Send`
    |
    = help: within `config::Config`, the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<std::collections::BTreeMap<std::string::String, config::Package>>`
    = help: the trait `hyper::common::exec::ConnStreamExec<F, B>` is implemented for `hyper::common::exec::Exec`
note: future is not `Send` as this value is used across an await
   --> src/commands/serve.rs:144:32
    |
144 |         serve_fpm_file(&config).await
    |                        ------- ^^^^^^ await occurs here, with `&config` maybe used later
    |                        |
    |                        has type `&config::Config` which is not `Send`
145 |     } else if path.eq(&camino::Utf8PathBuf::new().join("")) {
    |     - `&config` is later dropped here
note: required by a bound in `hyper::server::Builder::<I, E>::serve`
   --> /Users/amitu/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/hyper-0.14.20/src/server/server.rs:546:12
    |
546 |         E: ConnStreamExec<<S::Service as HttpService<Body>>::Future, B>,
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `hyper::server::Builder::<I, E>::serve`

error: future cannot be sent between threads safely
   --> src/commands/serve.rs:353:75
    |
353 |     let server = hyper::Server::bind(&tcp_listener.local_addr().unwrap()).serve(make_svc);
    |                                                                           ^^^^^ future returned by `route` is not `Send`
    |
    = help: within `hyper::proto::h2::server::H2Stream<impl futures::Future<Output = std::result::Result<hyper::Response<hyper::Body>, error::Error>>, hyper::Body>`, the trait `std::marker::Send` is not implemented for `std::ptr::NonNull<std::collections::BTreeMap<std::string::String, config::Package>>`
    = help: the trait `hyper::common::exec::ConnStreamExec<F, B>` is implemented for `hyper::common::exec::Exec`
note: future is not `Send` as this value is used across an await
   --> src/package_doc.rs:330:6
    |
304 |       let mut all_packages = config.all_packages.borrow_mut();
    |           ---------------- has type `std::cell::RefMut<'_, std::collections::BTreeMap<std::string::String, config::Package>>` which is not `Send`
...
330 |       )
    |  ______^
331 | |     .await)
    | |__________^ await occurs here, with `mut all_packages` maybe used later
...
364 |   }
    |   - `mut all_packages` is later dropped here
note: required by a bound in `hyper::server::Builder::<I, E>::serve`
   --> /Users/amitu/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/hyper-0.14.20/src/server/server.rs:546:12
    |
546 |         E: ConnStreamExec<<S::Service as HttpService<Body>>::Future, B>,
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `hyper::server::Builder::<I, E>::serve`

error: future cannot be sent between threads safely
   --> src/commands/serve.rs:353:75
    |
353 |     let server = hyper::Server::bind(&tcp_listener.local_addr().unwrap()).serve(make_svc);
    |                                                                           ^^^^^ future returned by `route` is not `Send`
    |
    = help: the trait `std::marker::Sync` is not implemented for `std::cell::Cell<isize>`
    = help: the trait `hyper::common::exec::ConnStreamExec<F, B>` is implemented for `hyper::common::exec::Exec`
note: future is not `Send` as this value is used across an await
   --> src/package_doc.rs:330:6
    |
304 |       let mut all_packages = config.all_packages.borrow_mut();
    |           ---------------- has type `std::cell::RefMut<'_, std::collections::BTreeMap<std::string::String, config::Package>>` which is not `Send`
...
330 |       )
    |  ______^
331 | |     .await)
    | |__________^ await occurs here, with `mut all_packages` maybe used later
...
364 |   }
    |   - `mut all_packages` is later dropped here
note: required by a bound in `hyper::server::Builder::<I, E>::serve`
   --> /Users/amitu/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/hyper-0.14.20/src/server/server.rs:546:12
    |
546 |         E: ConnStreamExec<<S::Service as HttpService<Body>>::Future, B>,
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `hyper::server::Builder::<I, E>::serve`

error: future cannot be sent between threads safely
   --> src/commands/serve.rs:353:75
    |
353 |     let server = hyper::Server::bind(&tcp_listener.local_addr().unwrap()).serve(make_svc);
    |                                                                           ^^^^^ future returned by `route` is not `Send`
    |
    = help: the trait `std::marker::Send` is not implemented for `dyn futures::Future<Output = std::result::Result<(), error::Error>>`
    = help: the trait `hyper::common::exec::ConnStreamExec<F, B>` is implemented for `hyper::common::exec::Exec`
note: future is not `Send` as it awaits another future which is not `Send`
   --> src/sitemap.rs:899:17
    |
899 | /                 resolve_toc(
900 | |                     toc,
901 | |                     package_root,
902 | |                     current_package_root,
...   |
905 | |                     config,
906 | |                 )
    | |_________________^ await occurs here on type `std::pin::Pin<std::boxed::Box<dyn futures::Future<Output = std::result::Result<(), error::Error>>>>`, which is not `Send`
note: required by a bound in `hyper::server::Builder::<I, E>::serve`
   --> /Users/amitu/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/hyper-0.14.20/src/server/server.rs:546:12
    |
546 |         E: ConnStreamExec<<S::Service as HttpService<Body>>::Future, B>,
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `hyper::server::Builder::<I, E>::serve`

error[E0277]: the trait bound `hyper::common::exec::Exec: hyper::common::exec::ConnStreamExec<impl futures::Future<Output = std::result::Result<hyper::Response<hyper::Body>, error::Error>>, hyper::Body>` is not satisfied
   --> src/commands/serve.rs:362:27
    |
362 |     if let Err(e) = server.await {
    |                           ^^^^^^ the trait `hyper::common::exec::ConnStreamExec<impl futures::Future<Output = std::result::Result<hyper::Response<hyper::Body>, error::Error>>, hyper::Body>` is not implemented for `hyper::common::exec::Exec`
    |
    = help: the trait `hyper::common::exec::ConnStreamExec<F, B>` is implemented for `hyper::common::exec::Exec`
    = note: required because of the requirements on the impl of `futures::Future` for `hyper::server::server::new_svc::NewSvcTask<hyper::server::conn::AddrStream, impl futures::Future<Output = std::result::Result<hyper::service::util::ServiceFn<fn(hyper::Request<hyper::Body>) -> impl futures::Future<Output = std::result::Result<hyper::Response<hyper::Body>, error::Error>> {commands::serve::route}, hyper::Body>, std::convert::Infallible>>, hyper::service::util::ServiceFn<fn(hyper::Request<hyper::Body>) -> impl futures::Future<Output = std::result::Result<hyper::Response<hyper::Body>, error::Error>> {commands::serve::route}, hyper::Body>, hyper::common::exec::Exec, hyper::server::server::NoopWatcher>`
    = note: required because of the requirements on the impl of `hyper::common::exec::NewSvcExec<hyper::server::conn::AddrStream, impl futures::Future<Output = std::result::Result<hyper::service::util::ServiceFn<fn(hyper::Request<hyper::Body>) -> impl futures::Future<Output = std::result::Result<hyper::Response<hyper::Body>, error::Error>> {commands::serve::route}, hyper::Body>, std::convert::Infallible>>, hyper::service::util::ServiceFn<fn(hyper::Request<hyper::Body>) -> impl futures::Future<Output = std::result::Result<hyper::Response<hyper::Body>, error::Error>> {commands::serve::route}, hyper::Body>, hyper::common::exec::Exec, hyper::server::server::NoopWatcher>` for `hyper::common::exec::Exec`
    = note: 1 redundant requirement hidden
    = note: required because of the requirements on the impl of `futures::Future` for `hyper::Server<hyper::server::conn::AddrIncoming, hyper::service::make::MakeServiceFn<[closure@src/commands/serve.rs:349:52: 352:6]>>`
    = note: required because of the requirements on the impl of `std::future::IntoFuture` for `hyper::Server<hyper::server::conn::AddrIncoming, hyper::service::make::MakeServiceFn<[closure@src/commands/serve.rs:349:52: 352:6]>>`
help: remove the `.await`
    |
362 -     if let Err(e) = server.await {
362 +     if let Err(e) = server {
    |

error: future cannot be sent between threads safely
   --> src/commands/serve.rs:362:27
    |
362 |     if let Err(e) = server.await {
    |                           ^^^^^^ future returned by `route` is not `Send`
    |
    = help: within `config::Config`, the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<std::collections::BTreeMap<std::string::String, config::Package>>`
note: future is not `Send` as this value is used across an await
   --> src/commands/serve.rs:144:32
    |
144 |         serve_fpm_file(&config).await
    |                        ------- ^^^^^^ await occurs here, with `&config` maybe used later
    |                        |
    |                        has type `&config::Config` which is not `Send`
145 |     } else if path.eq(&camino::Utf8PathBuf::new().join("")) {
    |     - `&config` is later dropped here
help: remove the `.await`
    |
362 -     if let Err(e) = server.await {
362 +     if let Err(e) = server {
    |

error: future cannot be sent between threads safely
   --> src/commands/serve.rs:362:27
    |
362 |     if let Err(e) = server.await {
    |                           ^^^^^^ future returned by `route` is not `Send`
    |
    = help: within `std::option::Option<impl futures::Future<Output = std::result::Result<hyper::Response<hyper::Body>, error::Error>>>`, the trait `std::marker::Send` is not implemented for `std::ptr::NonNull<std::collections::BTreeMap<std::string::String, config::Package>>`
note: future is not `Send` as this value is used across an await
   --> src/package_doc.rs:330:6
    |
304 |       let mut all_packages = config.all_packages.borrow_mut();
    |           ---------------- has type `std::cell::RefMut<'_, std::collections::BTreeMap<std::string::String, config::Package>>` which is not `Send`
...
330 |       )
    |  ______^
331 | |     .await)
    | |__________^ await occurs here, with `mut all_packages` maybe used later
...
364 |   }
    |   - `mut all_packages` is later dropped here
help: remove the `.await`
    |
362 -     if let Err(e) = server.await {
362 +     if let Err(e) = server {
    |

error: future cannot be sent between threads safely
   --> src/commands/serve.rs:362:27
    |
362 |     if let Err(e) = server.await {
    |                           ^^^^^^ future returned by `route` is not `Send`
    |
    = help: the trait `std::marker::Sync` is not implemented for `std::cell::Cell<isize>`
note: future is not `Send` as this value is used across an await
   --> src/package_doc.rs:330:6
    |
304 |       let mut all_packages = config.all_packages.borrow_mut();
    |           ---------------- has type `std::cell::RefMut<'_, std::collections::BTreeMap<std::string::String, config::Package>>` which is not `Send`
...
330 |       )
    |  ______^
331 | |     .await)
    | |__________^ await occurs here, with `mut all_packages` maybe used later
...
364 |   }
    |   - `mut all_packages` is later dropped here
help: remove the `.await`
    |
362 -     if let Err(e) = server.await {
362 +     if let Err(e) = server {
    |

error: future cannot be sent between threads safely
   --> src/commands/serve.rs:362:27
    |
362 |     if let Err(e) = server.await {
    |                           ^^^^^^ future returned by `route` is not `Send`
    |
    = help: the trait `std::marker::Send` is not implemented for `dyn futures::Future<Output = std::result::Result<(), error::Error>>`
note: future is not `Send` as it awaits another future which is not `Send`
   --> src/sitemap.rs:899:17
    |
899 | /                 resolve_toc(
900 | |                     toc,
901 | |                     package_root,
902 | |                     current_package_root,
...   |
905 | |                     config,
906 | |                 )
    | |_________________^ await occurs here on type `std::pin::Pin<std::boxed::Box<dyn futures::Future<Output = std::result::Result<(), error::Error>>>>`, which is not `Send`
help: remove the `.await`
    |
362 -     if let Err(e) = server.await {
362 +     if let Err(e) = server {
    |

For more information about this error, try `rustc --explain E0277`.
error: could not compile `fpm` due to 9 previous errors

@amitu
Copy link
Contributor Author

amitu commented Sep 27, 2022

Sylvie: Actix uses multiple single-thread tokio runtimes instead of a single multi-threaded one by design. Specifically to allow !Send futures and minor performance optimizations like Rcs.

Me: Got it. Makes sense. Is it possible to emulate this behaviour with hyper?

Sylvie: Possible, yes; desirable, hard to tell.
Basically, you'd have to manually create the number of tokio runtimes you want and select the current-thread runtime flavor for each, then create separate instances of your hyper server on each runtime (this is what actix-rt does for you, and why their HttpServer::new takes a closure; this closure is called once for each worker runtime.)

So this is a pretty complex task, depending on the usecase you might want to consider making your types threadsafe instead.

Again depending on the exact usecase you could also just go for a single single-threaded runtime - you'd still be able to use your !Send types and .await points would still allow concurrency, but blocking code will block the entire service.

Me: My goal was to reduce depencies: This PR brings down total dependencies from 328 to 275.

Looks like not worth it for now.

Thanks for the help 🙏

Sylvie: Yeah, refactoring other major parts of a codebase or potentially having to re-invent the wheel doesn't sound worth it for shaving off like 50 dependencies.

Glad I could help!

Why exactly did you want to reduce dependencies btw?

If it's to improve compile times, I can also recommend completely turning off the macros feature on actix and doing everything with plain function calls; that helped a lot for me.

(But then again I didn't have many handlers or a lot of logic in them to begin with, so that made it easy to get rid of the few macros I was using).

@amitu
Copy link
Contributor Author

amitu commented Sep 27, 2022

Ilyvion: Actix promises single-thread execution per request to the best of my knowledge. I.e. it is multi-threaded, but any one request will start, run and complete on the same thread. This is how it can use things like Rc in per-request situations internally, e.g.

The documentation for Actix says that

To achieve similar performance to multi-threaded, work-stealing runtimes, applications using actix-rt will create multiple, mostly disconnected, single-threaded runtimes. This approach has good performance characteristics for workloads where the majority of tasks have similar runtime expense.

Source: https://docs.rs/actix-rt/latest/actix_rt/

So you'd have to do the same thing yourself, spin up a bunch of runtimes and then manage the handing over of requests to whatever runtime is free to accept a task.

I think it's mostly the "arbiter" doing this work, although fair warning: I'm not intimately familiar with Actix' inner workings. https://github.com/actix/actix-net/blob/master/actix-rt/src/arbiter.rs

@amitu amitu closed this Sep 27, 2022
@amitu amitu deleted the actix_gone_not_compiling branch September 27, 2022 13:49
@amitu amitu restored the actix_gone_not_compiling branch September 28, 2022 06:31
@gabhijit
Copy link

@amitu : The code is compiling for me locally. Now that is rather odd!!!

@amitu
Copy link
Contributor Author

amitu commented Sep 28, 2022

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants