Re: [boost] [afio] Formal review of Boost.AFIO

Subject: Re: [boost] [afio] Formal review of Boost.AFIO
From: Antony Polukhin (antoshkka_at_[hidden])
Date: 2015-08-24 14:46:24

2015-08-23 4:08 GMT+03:00 Ahmed Charles <acharles_at_[hidden]>:

> The formal review of the Boost.AFIO library starts, August 21st and ends
> on Monday August 31st.
>

Hi,
I've started reading the tutorials and a bunch of thoughts rise in my head:

1) "Hello World, asynchronously!"

This looks really good and impressive. However the
`afio::current_dispatcher_guard h(afio::make_dispatcher().get());` code
seems like a boiler plate code that is used all around the examples. Could
it be simplified? For example adding a default constructor to the
`current_dispatcher_guard` will significaly reduce typing.

Also `afio::future<>` misses destructor description. This is essential,
because unfortunately in C++ we have different behavior for futures: future
returned by `std::async` blocks on destruction, default `std::future`
abandons the shared state and does not block.

I've failed to find a description for `when_all_p` function in reference
section. Is it documented?

2) "A less toy example: Concatenating files"

That's example not nice in a multiple ways. First of all, it combines
explicit usage of `boost::afio::dispatcher` with implicit
`current_dispatcher_guard`. It would be good to have single way of dealing
with dispatchers in example. `current_dispatcher_guard` seems more simple
(and that's what i like in examples :) )

`atomic<off_t> written(0);` popped in my eye. After that in reference
section I've found that the `make_dispatcher` accepts a thread pool. Is it
possible to follow Boost.Asio convention with `io_service::run` like calls?
I often reuse the main thread for ASIO's async operations, so that feature
could be useful for some resource/performance paranoid users. Also, thread
count 1 seems more right and less error prone (by default this will require
no threads synchronization from users).

Please rename the `utils::file_buffer_allocator` into
`utils::page_allocator`. In that way it's shorter and self-documenting in
some way.

Now the biggest problem of the second example. It allocates a lot of memory
and does a lot of data copies from kernel to usersapce and back.
Additionally this example is hard to understand and requires a lot of code
while solves a very common problem. Could AFIO reuse POSIX splice/vmsplice
and provide an async_copy function? If yes, then amount of code could be
significantly reduced and no memory allocations will be required.

-- 
Best regards,
Antony Polukhin