
Hi to all, I've been playing with asio and here is my review. I've tried to see it through the eyes of embedded systems developer, so that we could, for example, use asio, in a router or gateway. ------------------------------------------------------- * Design: I like the overall design of the library, based on known C++ networking patterns. I also like the great use of boost::bind for callback functions and the idea of asio::buffer wrapper. The flexible error handling is in my opinion, a strong point. Optional exceptions should be common in C++ libraries, so that we can use them in real-time environments. But I think asio covers two aspects: asynchronous programming and network. I think both should be separated: --> boost::asio: a framework for asynchronous programming --> boost::net: a network library. I think both concepts are mixed now, and don't see how a developer can easily plug its asynchronous device (file, network, ipc, or other) in asio. Everyone can extend iostream to provide its own classes, but I don't see that explanation is asio, I suppose a user should implement asio concepts like asynchonous streams but a tutorial on this is a must. As some have mentioned, I think a separate boost::net library should contain synchronous (blocking) sockets that don't need a demuxer to work. I find the example of "Timer.1 - Using a timer synchronously" disturbing. I think boost::net should provide portable sockets (synchronous and asynchronous) that can be used (optionally) in asio framework. If both concepts are separated, boost::filesystem, could for example, be used with boost::asio to provide asynchronous file i/o (read, write, search, etc...) or this could be used also with inter-process communications (which are very similar to network communications) * Implementation Good overall. I've following the "Daytime.3 - An asynchronous TCP daytime server" example with the Visual 7.1 debugger and specially looking for mechanisms that hurt embedded systems. I've found some interesting things about memory allocations. I've commented the code with the number of calls to "operator new": int main() { //--> Two allocations constructing a shared_ptr before main enters try { //--> 4 calls to new inside the asio::demuxer constructor asio::demuxer demuxer; //--> 17 calls to new inside the asio::socket_acceptor constructor asio::socket_acceptor acceptor(demuxer, asio::ipv4::tcp::endpoint(13)); // -> 1 explicit new operator //--> 4 calls to new inside asio::stream_socket constructor asio::stream_socket* socket = new asio::stream_socket(demuxer); //--> 3 calls to new inside acceptor.async_accept(...) // -> 2 in the expression "if(impl == null())" to create // a socket_shared_ptr_type for null() // -> 1 to create new operation object. acceptor.async_accept(*socket, boost::bind(handle_accept, &acceptor, socket, asio::placeholders::error)); //--> 2 calls to new every new operation (accept or completion) // to create a new shared_ptr socket type for new connection demuxer.run(); } catch (asio::error& e) { std::cerr << e << std::endl; } return 0; } void handle_write(asio::stream_socket* socket, char* write_buf, const asio::error& /*error*/, size_t /*bytes_transferred*/) { using namespace std; // For free. free(write_buf); // -> 2 calls to new in the expression "if(impl == null())" // to destroy a socket // -> 2 calls to new when assigning impl = null() delete socket; } void handle_accept(asio::socket_acceptor* acceptor, asio::stream_socket* socket, const asio::error& error) { if (!error) { using namespace std; // For time_t, time, ctime, strdup and strlen. time_t now = time(0); char* write_buf = strdup(ctime(&now)); size_t write_length = strlen(write_buf); //2 news to create a new shared_ptr send_operation asio::async_write(*socket, asio::buffer(write_buf, write_length), boost::bind(handle_write, socket, write_buf, asio::placeholders::error, asio::placeholders::bytes_transferred)); // -> 1 explicit user new operator // -> 2 in the expression "if(impl == null())" to create // a socket_shared_ptr_type for null() socket = new asio::stream_socket(acceptor->demuxer()); // -> 2 calls to new in the expression "if(impl == null())" // to create a null socket_shared_ptr_type for null() acceptor->async_accept(*socket, boost::bind(handle_accept, acceptor, socket, asio::placeholders::error)); } else { delete socket; } } I find the number of allocations too big. I don't mind if the demuxer or the acceptor allocates memory dynamically because those objects have long lifetime. The user's explicit allocations are not a problem, because we can use a pool if we want to avoid fragmentation. But I find that repetitive operations (async_accept(), sync_send(), run()) make use of dynamic allocations, that I think can be problematic in high reliability systems. Some of them come to the fact that boost::shared_ptr is used widely in the library. For example, a socket is really containing a shared_ptr<unsigned int> and a new created socket (for example, when accepting a connection) needs 2 calls to new (one for the socket itself and another one for the shared_counter). This way, even the null socket is implemented with shared_ptr, so internal expressions like: if(impl == null()) create a temporary null socket that is a fully constructed shared_ptr<unsigned int>-> 2 allocations and 2 deallocations. I think that copying happily shared_ptr could be expensive because atomic operations are cheap but not free due to memory barriers (not maybe in asynchronous network operations because those have long latency, but maybe in other asynchronous ipc or asynchronous inter-thread communications). ---------------- It's clear that the use of runtime bind functions as callbacks, every asynchronous operation call needs to do a dynamic allocation to store a copy of the functor. Asynchronous Completion Tokens like windows' OVERLAPPED must be also allocated. asio allocates both in the same object and that's good, but there is no way to provide a custom allocator, so that the programmer can control and optimize memory use. I suggest to avoid implementing sockets as shared_ptr-s and giving the user the possibility to provide an allocator to allocate this per-asynchronous operation objects. This maybe is not easy, because each operation (accept, read, write, close, etc...) may require a different memory size, but maybe all could be unified in a single structure, (the structure would be dependent on <typename Async_Write_Stream, typename Handler> but the user knows the types that he uses, so maybe a internal type definition <typename Async_Write_Stream, typename Handler> struct async_context { typedef /* some_type */ type; }; could be used so that the user can create a pool of typename async_context<my_stream_t, my_handler_t>::type objects and asio could use it to allocate objects of that type. Just an idea. The goal would be that the user could avoid ANY repetitive dynamic allocation and build high reliability systems with asio, without fearing memory fragmentation issues. My review has been quite quick so maybe this is not possible because there are more subtle problems, but I would want to make asio ready to be used from routers to mainframes. * Documentation Nice. I think the tutorial is good, but I would add more complex examples using more classes. I think that it should contain a chapter explaining how asio can be extended to use other asynchronous devices. * Usefulness Extremely useful. C++ asynchronous programming and a general network implementation is a must. * Use I've used asio for some days compiling and debugging some tutorial examples in Visual 7.1. * Me I've done some asynchronous programming (although not with sockets) and I know some of its gotchas. Not an expert but a casual user of asynchronous programming * Summary I consider asio should be separated in two libraries: _boost::asio_ (as asynchronous programming framework) and _boost::net_ (providing socket and other network programming portable wrappers). I think that this second library should provide synchronous blocking (and non-blocking if possible) operations without the use of demuxers or other _boost::asio_ classes. I consider the documentation should contain a chapter explaining how you can extend asio to provide new asynchronous devices. I think the library should review a bit its memory allocation policy, and provide to the user the possibility to avoid _any_ repetitive memory allocation via customized memory allocation. This will make _boost::asio_ extremely portable. If all conditions are met, I vote STRONG YES to Boost inclusion because I think that asio is the best effort around and good basis for a Standard C++ network library. Regards, Ion