[boost] Re: [review] Formal review of the SmartContainerlibrarystarts today

From: Jonathan Turkanis (technews_at_[hidden])
Date: 2004-10-05 20:09:41

"Thorsten Ottosen" <nesotto_at_[hidden]> wrote in message
news:cjvavc$hot$1_at_sea.gmane.org...
> Hi Jonathan,

> | 4. Since performance is one of the main design goals, I'm suprised that
> | attempting to insert a null pointer causes an exception rather than an
> insertion
> | failure. The same goes for invoking the functions for back, front, pop_back
> and
> | pop_front with an empty container. By coincidence, this morning I was
> | experimenting with some code that invokes a function pointer with a pointer
> as
> | argument, like so
> |
> |     f_(p_)
> |
> | in an inner loop. I tried replacing the above code with
> |
> |    if (p_)
> |       f_(p_);
> |    else
> |         throw std::runtime_error(...);
> |
> | I found an enormous slowdown, which should be no surprise.
>
> yeah, I dunno, this might depend heavily on the compiler. what compiler do you
> use?

Good point. I was using VC7.1. I ran the same test with como4.3.3/VC7.1 Intel
8.0 for windows and GCC 3.4.3 (Cygwin) and didn't see a significant difference.
But the fact that it makes a difference for some compilers, esp. a widely used
one, should be significant.

> At least for back()/front() you can avoid a check by
> *ptr_begin()/*--ptr_end().
> For vector/deque you can unchecked random access too via operator[]().
>
> It is is true that performance was a design goal, but so was safety. The
> domain of pointers
> allowed me to do different trade-offs than with standard containers. For
> example, does
> it really matter if container.push_back( new T ) throw if 0 was inserted? IMO
> no, because
> the heap operation will dwarf any other operation by several magnitudes.
> The same considerations would be true for eg. pop_back() when calling the
> detructor
> on the object since we need to remove the object from the heap.

Also good point. I think it doesn't matter for popping, for the reason you
mentioned. For inserting elements, it won't matter most of the time, but
sometimes you might be transfering ownership of already constucted objects. I
think it's a bit odd to require someone who wants uncheked access to the final
element to use *--ptr_end().

> | 6. To answer possible review issue 13, I'd say you should parameterize the
> | provided clone managers by allocator type, defaulting to std::allocator<
> | [unspecified] >.
>
> why unspecified instead of T* ?

Right. I was looking around for a T, but I didn't see one since
heap_clone_manager and view_clone_manager are non-templates. Obviously you just
use the T from the container template declaration.

> | 7. I don't understand review issue 10. I don't see the erase()/insert()
> | functions that return pair<iterator, bool>.
>
> in set/map and ptr_set/ptr_map these functions returns this pair to say "we
> did/did not insert the element".

But I can't find these members in the documentation.

Best Regards,
Jonathan