Boost logo

Boost :

Subject: Re: [boost] [review] The review of Boost.DoubleEnded starts today: September 21 - September 30
From: Thorsten Ottosen (tottosen_at_[hidden])
Date: 2017-09-27 14:37:16


Den 26-09-2017 kl. 19:22 skrev Zach Laine via Boost:
> On Tue, Sep 26, 2017 at 10:59 AM, Thorsten Ottosen via Boost <

>> Can you explain how that works for vector when growing the front?
>
>
> Sure:
>
> std::vector<int> v;
>
> v.push_back(1);
> v.push_back(2);
> v.push_back(3);
>
> for (auto it = v.rbegin(), end = rend(); it != end; ++it) {
> // whatever
> }
>
> Just as you don't care whether your stack grow up or down, you don't care
> whether you're pushing to the "front" or "back" if you only push/pop to one
> side -- because you can always view the resulting sequence in reverse.

Got it, thanks.

>> Are you opposed to such containers or is it "all the extra" stuff (which
>>>> you don't have to use btw) that makes you uneasy?
>>>>
>>>>
>>> Up until now, in this post all my objections have been related to runtime
>>> efficiency. But now you've brought back up a major complaint that I have,
>>> which is the unsafety of the interface. To say that I don't have to use
>>> it
>>> is a bit disingenuous to me. Adding an unsafe API weakens the invariants
>>> of the type, making it more difficult to reason about and use. The idea
>>> that I can add elements that will be manually destructed but that are not
>>> yet constructed is a serious defect in my opinion. Any throwing operation
>>> I do between the creation of such elements and their construction is now
>>> unsafe.
>>>
>>
>> For the record, I'm not trying to be disingeneous. Let me explain.
>>
>> The invariant of the container remains the same no matter you do.
>>
>
> They do not. The most basic container invariant is RAII. That is,
> elements that have been constructed in the container will be destructed
> properly when the container is destructed, and -- importantly -- nothing
> else.

I could have been clearer: the invariant of the type is something
static. An object of that type should maintain the invariant. And some
of these special functions require you to maintain the invariant of the
object momentarily.

> But again, don't get me wrong -- I'm not opposed to doing these kinds of
> low-level hacks to get performance. For instance, move operations are
> unsafe in a similar fashion (though not as unsafe, IMO), and I use them all
> the time.
>
> The issue to me is that these are high-level container types, and yet they
> have invariants that are less safe than std::array. I feel they are not
> sufficiently justified in such a container.
>
> By the way, there are other ways to get similar performance gains without
> sacrificing safety:
>
> template <typename T>
> struct devector_guts
> {
> // low level unsafe stuff goes here, "here be dragons", etc.
>
> T * elements_;
> size_t size_;
> size_t capacity_;
> };
>
> template <typename T>
> struct devector
> {
> // only safe operations here
>
> devector_guts steal_guts () &&; // returns guts; empties *this
>
> private:
> devector_guts guts_;
> };

maybe a slightly more encapsulated approach would be:

devector<T> devec;
...
devector_access<T> devec_access( devec );
// a non-copyable class that stores reference to devector<T>
devec_access.nongrowing_push_back( x );
devec_access.uninitialized_resize( k );
// etc

So whenever you have to do something a bit out of the ordinary, you do
it via devector_access. That type is then in seperate header and
documented under advanced usage.

?

>> Well, there is nothing that requires you to use
>> reserve_front/reserve_back. Just like there is no one that requires you to
>> use reserve in vector.
>
>
> Sure. I'm trying to point out that when I *do* want to use it, the current
> separate-and-fixed scheme is hard to use, because you must guess something
> close to the relative ratio of front and back pushes.

I don't get this. Why do one want to use it if one has no clue about
what to expect?

> It's not a criticism of devector at all. It's an open question for which I
> have no answer, meaning I have to 1) do a lot of homework (profiling, etc.)
> before knowing if I should even consider devector, and 2) weigh any
> possible gains in performance against loss of ability to reason about my
> code.
>
> It is, however, a criticism of the library, which should do this profiling
> homework and proper encapsulation for me.

I agree that it would be good with some basic tests. Its hard to cover
everything because there are so many situations. Its not something every
Boost library is doing, take Boost.Container where I don't see any
benchmarks.

>> [snip]
>>
>> First, please demonstrate that I should care about those few instructions.
>>> Second, I agree that the difference is slight, but I disagree that it's ok
>>> to make that slight change. One of the chief invariants of a standard
>>> container is that it manages its storage. This breaks that.
>>>
>>
>> It's not an invariant of vector that push_back allocates memory.
>
>
> Sure it is. 26.3.11.5/1 says:

[snip]

> Remarks: Causes reallocation if the new size is greater than the old
> capacity. Reallocation invalidates all the references...

I think we operate with different definitions of (informal) invariants.
In my world something is invariant if it always holds. push_back may
allocate, but it certainly doesn't always. I would rather say that the
postcondition for push_back is so and so. Anyway, this is not important ...

>> buffer.unsafe_uninitialized_resize_back(recvSize);
>>>>>
>>>>
>> It changes the size of the container so you can't read uninitialized
>>>> values.
>>>>
>>>>
>>> That's true, though some sort of span would accomplish the same thing with
>>> no loss of safety.
>>>
>>
>> But then you just postponed the double initialization to when the span
>> needs to be copied into a proper container!
>>
>
> Well, in the limited case where I copy it, sure. But that's also true if I
> copy buffer after calling
>
> buffer.unsafe_uninitialized_resize_back(recvSize);
>
> above. The point is you want to use the [0, recvSize) subsequence of
> buffer, and you don't need to have a unsafe_uninitialized_resize_back()
> member to do that.

Maybe you can make your span-based snippet a little more concrete? In
particular, where does the memory come from? While doing so, assume that
the memory has to outlive the function that calls recv() and that the
correctly initialized span has to be iterated over at some point after
the function that calls recv().

kind regards

Thorsten


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk