|
Boost : |
From: Fernando Cacciola (fcacciola_at_[hidden])
Date: 2002-06-27 18:15:30
----- Original Message -----
From: "Joerg Walter" <jhr.walter_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Thursday, June 27, 2002 7:37 PM
Subject: Re: [boost] uBLAS formal review
>
> ----- Original Message -----
> From: "Fernando Cacciola" <fcacciola_at_[hidden]>
> To: <boost_at_[hidden]>
> Sent: Thursday, June 27, 2002 10:47 PM
> Subject: Re: [boost] uBLAS formal review
>
>
> > I continue to scan the code...
> >
> > storage.h
> > ~~~~~~
> >
> > The macro 'common()' MUST BE NAMED DIFFERENTLY, perhaps...
> > UBLAS_MACRO_FUNCTION_WHICH_IS_CALLED_COMMON()
> > :-) (pun intended)
>
> Ouch, yes.
>
> > ) Why does the default ctors for bounded_array<> et. al. dynamically
> > allocate an empty array (of size 0)?
>
> Why shouldn't they?
>
Because that array is never used? Isn't it significantly faster to
initialize data_ with NULL? Or is there a way that the array is used as is
instead of reallocated?
> > ) Why does the destructor of bounded_array<> et. al throw bad_alloc() if
> > data_ has not been allocated? Destructors should not throw exceptions,
and
> > even this one in particular seems highly unnecesary.
>
> Bad behaviour due to MSVC education.
>
> > ) Why do all dynamic allocations via 'new' are followed by 'if (!data_)
> > throw std::bad_alloc'? "new" already throws bad_alloc if memory is
> exausted.
>
> Not under MSVC.
>
Hmmm... which version?
In the worst case, new and delete should be overloaded for MSVC to do what
they are supposed to do. But the BLAS code should not be affected.
> > ) I highly dislike the names 'insert' and 'erase' here. They DO NOT
insert
> > or delete elements, they just set and reset the values of the already
> > allocated elements. I'd prefer 'set' and 'reset'.
>
> But they insert and erase in the sparse case and they are used generically
> (without partial specialization again due to MSVC).
>
OK. If they conceptually insert and erase in some cases that's fine.
> > ) both arrays could (should?) use valarray<> internally instead of
> explicit
> > heap allocated arrays. The difference would be significant because the
> calls
> > to std::copy could be replaced by valarray specific and usually
optimized
> > functions.
>
> Could, but shouldn't.
>
Hmmm. I think this would have to be disucussed a bit more.
Not using valarray<> prevents fast 'memcpy' optimizations... perhaps, ublas
could apply that optimization itself.
Cheers,
Fernando Cacciola
Sierra s.r.l.
fcacciola_at_[hidden]
www.gosierra.com
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk