ryanofsky
commented
3 weeks ago Thanks, this all seems helpful and correct reading it at a first pass. I think I would just tweak a few things and probably add a few additional details. It think it's especailly helpful this notes that libmultiprocess is just acting as a wrapper to make it easier to write and call cap'nproto interfaces from c++. But it is completely possible to call or implement the same interfaces without using libmultiprocess (especially from other languages like rust python and javascript).
One thing that is a little off in 86415b456bb3307afae406c2b1b5cbc5d0c143b3 is that it makes it sound like ProxyClient and ProxyServer objects wrap ends of socket, when it would be more accurate just to say that they communicate over sockets. Specifically when there is a socket connection there is not just one ProxyClient and one ProxyServer object sitting at either end of the socket, but one ProxyClient and one ProxyServer for each instantiated interface, and probably dozens or hundreds of ProxyClient and ProxyService objects all using the same socket.
If you want to mention the class that does sit at either end of a socket, that would just be the mp::Connection class defined in mp/proxy-io.h
Similarly, in diagram from PR description it could give wrong impression to show AsyncioContext containing AsyncIoStream, because the same context can be used to communicate across many streams. For example, if there is a bitcoin-node process, it may spawn a bitcoin-wallet and communicate across one stream with it, with but also accept bitcoin-mine connections and use other streams to communicate with them. All these streams will use the same EventLoop and AsyncioContext.
It is true in both of these 1:N relationships though, that not all of the N are equal. Even though there will be many ProxyClient/ProxyServer clients using the same socket, and many AsyncIoStreams using the same AsyncioContext and EventLoop, users are not undifferentiated and identical. While the EventLoop uses many AsyncIoStreams it does have one special wait_stream that is used to receive requests posted to the event loop. Similarly, when a connection is first established, there is a initial ProxyClient and ProxyServer pair wrapping an Init interface, which is special because it is the first interface, and it what is used to create all other interfaces, and ProxyClient and ProxyServer objects wrapping the interfaces that are created.
Anyway these changes look good, and I'm happy to merge them anytime you want.
TheCharlatan
I am not sure if the descriptions here are correct, or even all that useful, but I think some of the finer points are useful to document to serve as an anchor for future developers reading the code. Maybe I also missed some documentation in the headers and the other docs that already describe these things better.
~What could potentially also be useful are some diagrams to show the wrapping hierarchy as done below (again, not sure if actually correct, and given the polymorphism and templating going on it is also questionable if such a hierarchy can accurately be depicted as a diagram).~
Given the points made by ryanofsky below, I don't think anymore than an object diagram will be particularly helpful.
A sequence diagram that follows a call from its c++ callsite to the client invocation to the server receiving and processing it, the client resolving it and finally returning a c++ type again would probably be more useful.