Initial ethcore-io refactor

[debris]

Currently ethcore-io event loop is based on deprecated part of mio. Let's replace it with tokio

[tomaka]

Can you actually use tokio (actual question, I don't know ethcore-io enough to know)?
I think the Core and the Handle are not Send or Sync, so you cannot send sockets or (futures that contain a socket) amongst threads.

[0x7CFE]

@tomaka, please correct me if I'm wrong, but I do not see a problem here.

Looks like Parity internals use util::io facade to get even loop functionality. And if we look at the API we may see, that most of the Message generics are bounded by Send + Sync.

I even tried forcing + Sync bound to IoChannel, IoMessage, Handlers and other stuff and it all compiled, which makes me think, that at the moment we do not have messages that require Send or Sync.

Could you please give me some more details on a part of the system where you expect problems? What particular Core and Handle you've mentioned before?

[tomaka]

Right now the sockets are identified by a StreamToken, which is just a usize, but it is essentially the mio events loop that owns the sockets.

If you switch to tokio, however, you would need to manipulate TcpStream objects instead of usizes, and these are not Send or Sync if I'm not mistaken (could be wrong here).
Since TcpStream implements traits from the futures crate, the code that uses ethcore_io is supposed to manipulate them directly. What I mean by that is that the role they play is more than simply identifiers.

It should be possible to wrap around tokio's API so that each TcpStream stays in a dedicated I/O processing threads, and use channels from std::sync::mpsc in order to communicate data, but that would add a large overhead I think.

Could you please give me some more details on a part of the system where you expect problems? What particular Core and Handle you've mentioned before?

https://docs.rs/tokio-core/0.1.16/tokio_core/reactor/struct.Core.html
https://docs.rs/tokio-core/0.1.16/tokio_core/reactor/struct.Handle.html

[tomaka]

Also, once you build a future you need to send it back to ethcore_io in the correct thread so that it is passed to the Core::run function. This further complicates the problem.

[tomaka]

Another problem is that even if you track which thread each TcpStream belongs to, a particular user of ethcore_io may want to merge multiple TcpStream objects together, and these streams may belong to multiple different threads.

I think tokio in general is not meant to be used from multiple different threads.
It could make sense to use only a single worker and make Parity as a whole entirely "single-threaded" except for futures-cpupool which would manage background CPU processing threads. But that's a big decision to make.

[0x7CFE]

@tomaka, thank you very much for the detailed explanation! It is much clearer now. I would try to find a solution.

[debris]

cc @tomaka

If you switch to tokio, however, you would need to manipulate TcpStream objects instead of usizes, and these are not Send or Sync if I'm not mistaken (could be wrong here).

You could also split &TcpStream into ReadHalf and WriteHalf. There would still be a single place owning streams, but halfs could be safely passed between threads

[debris]

wouldn't that be enough?

[debris]

ah, actually that would not work... That's how I 'solved' this problem in parity-bitcoin

[tomaka]

I don't think Arc<TcpStream> would solve the problem, because it wouldn't be Send either.

I don't know how parity-bitcoin is organized, but from a quick look you don't seem to use a thread for I/O (and therefore not facing the same issues).

[0x7CFE]

Maybe we could solve the issue by not trying to send stuff that isn't Send in the first place? I may imagine an architecture, where requests are delegated to TcpStream (that resides in the same thread where it was created) using messages that are dispatched by the event loop.

Was suggested above already.

[0x7CFE]

If you switch to tokio, however, you would need to manipulate TcpStream objects instead of usizes, and these are not Send or Sync if I'm not mistaken (could be wrong here).

@tomaka, as for versions 0.1.x I believe they are implemented both in tokio and tokio_core. I even tried to move TcpStream between threads on playground and rustc didn't complain.

[tomaka]

Oh, I guess I was mistaken then.
But I suppose that you still have to run them in the correct thread afterwards, right?

[twittner]

I think more recent mio versions and tokio >= 0.1 are Send + Sync from tip to toe (except for explicit current thread execution), so moving a TcpStream over thread boundaries or using it concurrently should not be an issue AFAICT.

[adria0]

Closing the issue due to its stale state