[boost] [SPAM] Re: [SPAM] Re: [review] The review of Boost.DoubleEnded starts today: September 21 - September 30

Subject: [boost] [SPAM] Re: [SPAM] Re: [review] The review of Boost.DoubleEnded starts today: September 21 - September 30
From: Louis Dionne (ldionne.2_at_[hidden])
Date: 2017-10-10 23:05:44

> Hi Louis,
> 
> Thanks for the review. I forgot to give some feedback. Comments below.
> > Hi,
> > - I don't see the use for the uninitialized constructors and
> >    `unsafe_push_back`/`unsafe_push_front`. I mean, I do understand the
> >    use _in theory_, but I'd really like to see a good motivation for
> them
> >    with a more concrete use case. This is pretty dangerous and tricky to
> >    work with, so I think this deserves a good justification. I can't see
> >    myself using this container or recommending anyone to use the
> container
> >    without this.
> 
> Did you see the benchmarks?

I did. See below.

> Did you know Boost.Container has similar constructs? Would you then now 
> not use Boost.Container?

I did not know Boost.Container had similar constructs, and I could not find 
them in the documentation. Could you please share a link? If it does have 
them, I certainly hope they were justified properly when they were
introduced 
or when the library was reviewed.

> >>     - What is your evaluation of the documentation?
> > 
> 
> > - The design rationale for `devector` at the beginning of the
> documentation
> >    is slightly misleading. `devector` can't be a drop-in replacement for
> >    `std::vector`, because of iterator invalidation guarantees of move
> >    operations (see https://stackoverflow.com/a/41384937/627587).
> 
> Can you explain why ? ... it is not obvious that devector differs in 
> this respect.

With the small buffer optimization, I think moving containers will 
invalidate iterators.

> > 
> > - A simple diagram would be very helpful in quickly understanding what
> >    the `devector` is.
> 
> Agreed.
> 
> > - I would really like to see actual benchmarks, even if this needs
> coming
> >    up with more realistic use cases. IMO, it is not acceptable for a
> library
> >    tailored for performance (and having unsafe operations for that
> purpose)
> >    to provide only a listing of assembly at `-O2`.
> 
> Some have been posted. What is your take on them?

It seems like the benchmarks (if we're talking about the serialization and 
deserialization ones) are mostly about the facilities allowing to skip
initialization. This is nice, but would it be possible to construct 
benchmarks that do not use Boost.Serialization for demonstrating the 
same thing? Doing otherwise opens the door to speedups and slowdowns
caused by things unrelated to DoubleEnded itself. 

Also, my understanding is that these benchmarks only showcase the benefit
of not double-initializing containers, but not the actual main purpose of
the library, which is to provide double-ended containers. Benchmarks showing
why one cares about that (beyond std::vector and std::deque) would be 
welcome.

Finally, those should be added to the library documentation.

Regards,
Louis

--
Sent from: http://boost.2283326.n4.nabble.com/Boost-Dev-f2600599.html