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

Subject: Re: [boost] [afio] Formal review of Boost.AFIO
From: Thomas Heller (thom.heller_at_[hidden])
Date: 2015-08-25 19:04:59

Am 25.08.2015 11:48 nachm. schrieb "Niall Douglas" <
s_sourceforge_at_[hidden]>:
>
> On 25 Aug 2015 at 21:10, Thomas Heller wrote:
>
> > > You're not, by any chance, saying that you think the AFIO design
> > > should be 1 afio::handle = 1 fd? And when you copy construct
> > > afio::handle, that calls dupfd()?
> >
> > That is what I would expect from a handle copy constructor, yes. Note
> > that there are also move operations and a user is still free to store
> > handles in any "container" (including smart pointers or whatever).
>
> Just reminding you I won't be online after this until Friday ...
>
> You may not have considered that:
>
> 1. On POSIX file descriptors held by a process is limited, especially
> on OS X which has a crazy low limit.
>
> 2. Metadata updates are not guaranteed to be immediately consistent
> across file descriptors. E.g. fstat() on one fd can return very
> different information from fstat() on another fd even to the same
> file in the same process. This particularly happens with NTFS and
> ZFS, and is a major pain in lock free file system concurrency. Making
> this go away most of the time by reusing the same fd for end users is
> a big win.
>
> 3. In scatter gather file i/o there is no concept of file position,
> and therefore there is no good reason to needlessly duplicate fds
> when one will do.
>
> 4. AFIO has to "spend" fd's on workarounds for annoying problems on
> networked filing systems. For example, every byte range locked file
> needs to create a shadow file which is locked rather than the real
> file. This makes the problem of running out of fd's even worse.
>
> 5. Because of the need to pin lifetimes of things to one another due
> to things like shadow lock files and delete-on-close semantics,
> you're going to need a reference counting system one way or another.
> You can either hide that internally, or expose it. I chose to expose
> it, as I suspect it is more useful to end user code who also need to
> manage lifetimes.
>
> Finally I would also suspect that dupfd() is going to be a lot slower
> than a shared_ptr increment, though granted you'd only call it
> non-frequently hopefully (unless you put a handle in a C++ 98
> container implementation).

I did of course not consider all points, however I am assuming that actual
copies are very rare in the general case. My main beef with the design you
chose is that it implies shared ownership where it seems to not be
necessary.

>
> > > Ah I think I'm beginning to understand now. You're assuming a world
> > > where only std::future and std::shared_future exist and the
> > > relationship between them is well understood.
> > >
> > > By which, I infer you're saying that one does not need anything more
> > > than std::future and std::shared_future in an asynchronous filesystem
> > > and file i/o library?
> >
> > I didn't say there is no place for other things than std::future or
> > std::shared_future. You should know that ...
> > BUT if you claim conformance to the Concurrency TS ...
>
> I claim conformance to a suite of *extensions* to the Concurrency TS.

I hope you are confirming to your own set of extensions, would be very bad
otherwise...

> AFIO needs to be able to schedule multiple continuations on a future,
> that's the single biggest extension I need. The Concurrency TS
> doesn't allow that because the first continuation is expected to
> consume the future.

That's true for std::future but as already stated not so much for
std::shared_future.

>
> The ability to avoid exception_ptr is nice as well though.
> exception_ptr is expensive.

So it's expensive in the exceptional case, so what?

>
> > > And therefore, the current approach taken by AFIO of custom futures
> > > is wrong?
> >
> > ... a design that clearly does not conform to anything defined in the
> > standard and only the names coincide, then yes, I consider the design
> > broken. In addition, I see no shortcomings in the design of
> > std::future/std::shared_future that wouldn't fit your usecase.
>
> Do bear in mind that up until v1.3, AFIO was using std::future and
> std::shared_future. I didn't go off and extend the Concurrency TS
> without two years of experience of what exactly needs extending in
> the TS and why.
>
> My extensions are actually very conservative because I remain 100%
> compatible with the TS.

I actually lost track of what exactly your extensions are... Implicit
conversions from T, exception_ptr and error_code? What else?

Would you consider afio::future to be a complaint implementation? To what
does it comply to?

> If you look at what others are doing to
> future-promise e.g. Sean Parent, they are reworking the fundamental
> design.

Sure, but that design is not up for review and doesn't claim conformance.

>
> > > Would this summary of your criticism be correct? If it is, I can
> > > address all those points, but it'll probably be Friday as tomorrow I
> > > am away and on Thursday we have no electricity all day, so no
> > > internet nor computers.
> >
> > It's a very short summary of it, yes. Please keep in mind that all I
> > said was under the assumption that anything "future" like in AFIO (or
> > the depend NotMonad library) is just like a shared_future/future with
> > some extensions (you repeated that multiple times on the list).
> > If you have something different that's totally fine but needs other and
> > special attention because fighting an uphill battle against an ISO
> > standard is not easy. Especially after all the claims that have been
made.
>
> That makes sense. And changes my perspective on what the
> documentation needs to focus on. Thanks.
>
> Niall
>
> --
> ned Productions Limited Consulting
> http://www.nedproductions.biz/
> http://ie.linkedin.com/in/nialldouglas/
>
>
>
>
> _______________________________________________
> Unsubscribe & other changes:
http://lists.boost.org/mailman/listinfo.cgi/boost