Subject: Re: [boost] [AFIO] Review (or lack of it)
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2015-08-27 22:28:22
On 27 Aug 2015 at 12:54, Roland Bock wrote:
> a) There is so much going on about personal hardships (e.g. hundreds of
> hours of family time and 12-4am sessions and fried disks and no
> electricity and suspected vendettas and supposedly useless reviews).
> None of this has anything to do with a library review. Why mention it,
> even if you feel it? All of this is bound to make it hard to be impartial.
Well, I don't know what to say to this, so I'll say nothing and leave
your words speak for themselves.
> c) There are older versions in there. There is 1.3 and 1.4 at least (are
> there more?). Please choose one. Go for it.
AFIO provides strong ABI and API evolution guarantees. It guarantees
that if you compile and link to v2 of the ABI, your code will
continue to compile and link no matter what changes next in AFIO.
Some would call that a *huge* feature very atypical of Boost
libraries and one of exceptional value. You appear to consider it a
> e) Documentation
> * Please drop the "using namespace XYZ" from all examples. Maybe use
> namespace aliases.
I am unaware of any case where I imported a namespace except into a
local context. You may have been confused by the very commonly used:
... which is binding "something" from AFIO into the local or global
namespace. This pattern is safe.
> Why would I need a monad there?
The original idea was to show how monad abstracts away whether an
interface is synchronous or asynchronous. It confused people. It was
> Why is there a writable flag (why isn't constness enough to prevent
I have no idea what you're going on about here, but constness at the
language level has nothing to do with whether you open an on-disk
store for reading or writing.
> Why isn't the directory created at construction once and for all?
You appear to have not realised that this step enables invariance to
the store directory being relocated by third party processes during
operation. This is despite the tutorial text and the comments saying
> BTW: The name validation fails to check for empty string.
Logged to https://github.com/BoostGSoC13/boost.afio/issues/99.
> * https://boostgsoc13.github.io/boost.afio/doc/html/afio/quickstart/workshop/naive_afio_async/naive_racy.html
> mentions AFIO v1.40 (thats 36 numbers in the future). If the
> pictures are not loaded, the performance numbers in the table do not
> say what they are (the per sec is missing).
Logged to https://github.com/BoostGSoC13/boost.afio/issues/100.
> * "The /final/ named blob store design" starts with "Caution: This
> section was not finished".
> * "The performance of and problems with this third generation design"
> consists of "This section was not completed in time[...]"
> * So basically the crucial parts of the blob store sequence are
> incomplete at best.
The final example is over 1000 lines of code. I didn't think it
mattered hugely to the review.
> f) Code (since the implementation is going to change anyway, this might
> all be useless):
> * /attic: Seriously?
AFIO will be three years old soon. It had a life before being ported
to Boost in 2013. There are still bits and pieces in the attic
directory of use to me.
> * I am not a big fan of #if 0 in release code
For the examples, logged at
In the implementation, if it's #if 0 it's almost always debug assist
code which I turn on from time to time to help fix some reported bug.
Or it's code which isn't fully baked yet and end users shouldn't use
it e.g. byte range locking.
> * I doubt that std::cerr should be in this library's code.
Use of std::cerr is only when something truly unusual occurs, like we
are about to fatal exit the process and I would assume the user would
like to know why we just called std::terminate. Or we are about to
deadlock, and again the user probably wants to know why their
application has just hanged itself.
> * I wonder why there really is a need to have your own path stuff on
> top of boost::filesystem. If boost::filesystem is insufficient,
> wouldn't it be better to contribute to that one? Haven't looked too
> closely, though.
afio::path subclasses boost::filesystem::path or
std::filesystem::path. It is not a separate filesystem path
implementation, and in fact it is a very thin wrapper. Its purpose is
purely to add type safety for NT kernel path conversions from Win32
paths so the compiler will complain if you try to do something stupid
like feed a NT kernel path to a filesystem path consuming function.
I did take the opportunity to fix up the deviations in
boost::filesystem::path from the Filesystem TS however.
> * I looked briefly into detail/. I would not want to look for a bug
> there. I have not done any detailed analysis to base this on, but: I
> would assume that with some refactoring (moving the platform
> specific parts into platform specific headers), the readability
> could be increased dramatically. Again: This is just a guess.
The platform specific parts are already in platform specific files.
afio.ipp is for POSIX which includes MSVCRT. afio_iocp.ipp is for
> I recommend to treat the current review period as a pre-review and
> * Do a review of Monad (or whatever it will be named) first, maybe API
> Bind, too, depending on whether that is going to be used in AFIO
> * Remove all old versions, everything deprecated and do the
> implementation change that is planned
> * Cleanup the documentation. Get it read by people with different
> knowledge of the problem domain. I am willing to read it as somebody
> who is interested but certainly not very knowledgeable.
> * Let the dust settle a bit
> * Start a new review
> * Even if you believe in vendettas, stay calm and answer in a
> matter-of-fact way. My experience with the boost list is, that there
> are strong voices here which have a very finely tuned sense of fairness.
I do not believe in vendettas, and you will never have seen me pursue
one anywhere on the internet anywhere in the past decade. Feel free
to search google if you want to prove this in the nearly 30,000
public posts I believe I have made by now. I do however respond when
people are attacking me behind the scenes and the leadership have
specifically disclaimed doing anything about it (they claim to be
unaware of anything happening). If I did not defend myself, nobody
The fact you are aware of any of this is because of my efforts to
make public the fact this stuff was going on behind the scenes. If I
had done nothing, nobody here would be the wiser. It is what it is.
> I did invest several hours in this review, reading the mail threads,
> reading the documentation, looking through some code, writing this mail.
> As stated above, I do consider myself interested but not very
> knowledgeable in the problem domain.
> I would like to see a library for asynchronous file IO in boost, but I
> don't think AFIO is ready yet.
Thank you for your review. You caught some points nobody else had,
-- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk