delete [] v. free (Was: Re: kdelibs/kdecore)

[Roger Larsson]

I see that the necessary capacity is added in the newer version.
But this had more of other stuff that I like to comment on.

On Thursday 20 March 2003 18:36, Eray Ozkural wrote:
> CVS commit by exa:=20
>=20
> Implement the optimization mentioned in revision 1.84 and 1.85 without=20
breaking KDE big time (finally)
> This optimizes RArray class for the common use case of consecutive append=
s.

But malloc/realloc are usually optimized already!
It happens everytime realloc returns the same address as passed into it.

> Also implement memory allocation using malloc/realloc/free rather than=20
new/delete per Dirk's request.

But the implementations are not equivalent!

> Happy hacking,
> Note: I did test this patch and it doesn't seem to break anything like 1.=
85=20
did. That was gross, sorry again
> CCMAIL: kde-optimize@kde.org
>=20
>=20
>   M +16 -25    netwm.cpp   1.88
>=20
>=20
> --- kdelibs/kdecore/netwm.cpp  #1.87:1.88
> @@ -448,7 +448,9 @@ fprintf(stderr, "NETWM: Warning readIcon
>  template <class Z>
>  NETRArray<Z>::NETRArray()
> -  : sz( 0 ),
> -    d( NULL )
> +  : sz( 2 )
>  {
> +    // new/delete and malloc/free are not compatible
> +    d =3D (Z*) malloc(sizeof(Z)*sz); // allocate 2 elts
> +    memset( (void*) d, 0, sizeof(Z)*sz );

Space for two elements allocated and cleaned - OK.
But unneccessary - why 2, why not 8 or 0?
There is no point in doing anything until we know that at least
one element are to be added. And in that situation it is probably
better to allocate a minimum size of bytes instead....

>  }
> =20
> @@ -456,5 +458,5 @@ NETRArray<Z>::NETRArray()
>  template <class Z>
>  NETRArray<Z>::~NETRArray() {
> -    delete [] d;
> +    free(d);

So where is the destructor for each element called?
"delete [] d" calls destructors.
"free(d)" will not.

>  }
> =20
> @@ -462,31 +464,20 @@ NETRArray<Z>::~NETRArray() {
>  template <class Z>
>  void NETRArray<Z>::reset() {
> -    sz =3D 0;
> -    delete[] d;
> -    d =3D NULL;
> +    sz =3D 2;
> +    d =3D (Z*) realloc(d, sizeof(Z)*sz);
> +    memset( (void*) d, 0, sizeof(Z)*sz );

I see that this offset bug was been corrected in the new version.
But clearing memory is not the same thing as calling the constructor...

>  }
> =20
>  template <class Z>
>  Z &NETRArray<Z>::operator[](int index) {
> - - -
> +    if (index >=3D sz) {
> - - -
> +        // open table has amortized O(1) access time
> +        // when N elements appended consecutively -- exa
> +        int newsize =3D max(2*sz,  index+1);
> +        // copy into new larger memory block using realloc
> +        d =3D (Z*) realloc(d, sizeof(Z)*newsize);
> +        memset( (void*) &d[sz], 0, sizeof(Z)*(newsize-sz) );

memset with zero is not the same thinng as calling

There are libraries for memory management that can give statistics
of memory usage, and then you can tune the allocation functions
to fit that information. But if the application tries to be smart it ruins
that possibility...

/RogerL

=2D-=20
Roger Larsson
Skellefte=E5
Sweden