Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c++ > #13817 > unrolled thread

Question about destructors

Back to article view | Back to comp.lang.c++

Contents

  Question about destructors Gus Gassmann <horand.gassmann@googlemail.com> - 2012-02-07 06:53 -0800
    Re: Question about destructors "Fred Zwarts \(KVI\)" <F.Zwarts@KVI.nl> - 2012-02-07 16:11 +0100
    Re: Question about destructors "Asger Joergensen" <Junk@Asger-P.dk> - 2012-02-07 15:38 +0000
      Re: Question about destructors Gus Gassmann <horand.gassmann@googlemail.com> - 2012-02-07 09:13 -0800
        Re: Question about destructors "Asger Joergensen" <Junk@Asger-P.dk> - 2012-02-07 17:34 +0000
          Re: Question about destructors Victor Bazarov <v.bazarov@comcast.invalid> - 2012-02-07 12:42 -0500
            Re: Question about destructors 88888 Dihedral <dihedral88888@googlemail.com> - 2012-02-07 23:39 -0800
              Re: Question about destructors Ian Collins <ian-news@hotmail.com> - 2012-02-08 22:32 +1300
                Re: Question about destructors Gareth Owen <gwowen@gmail.com> - 2012-02-08 17:51 +0000
                  Re: Question about destructors Gert-Jan de Vos <gert-jan.de.vos@onsneteindhoven.nl> - 2012-02-08 10:47 -0800
        Re: Question about destructors SG <s.gesemann@gmail.com> - 2012-02-10 04:35 -0800
          Re: Question about destructors 88888 Dihedral <dihedral88888@googlemail.com> - 2012-02-11 08:17 -0800
    Re: Question about destructors Joshua Maurice <joshuamaurice@gmail.com> - 2012-02-07 15:12 -0800
    Re: Question about destructors Juha Nieminen <nospam@thanks.invalid> - 2012-02-08 06:38 +0000
      Re: Question about destructors Peter Remmers <p.remmers@expires-2011-04-30.arcornews.de> - 2012-02-09 00:30 +0100
        Re: Question about destructors Peter Remmers <p.remmers@expires-2011-04-30.arcornews.de> - 2012-02-09 00:56 +0100
        Re: Question about destructors Joshua Maurice <joshuamaurice@gmail.com> - 2012-02-09 14:34 -0800
          Re: Question about destructors Jorgen Grahn <grahn+nntp@snipabacken.se> - 2012-02-13 17:48 +0000

#13817 — Question about destructors

I have been working with C++ for about four years now, mostly self-
taught, and I get along reasonably well. However, last week I detected
a memory leak in some code, and trying to locate it I realized that I
do not know enough about destructors. I have created a number of use
cases. How can I make sure in each case that the memory gets destroyed
properly?

1. int *x[10];

2. int *y = new int[10];

3. Object *obj = new Object();

4. Object **obj = new Object[n];
    for (int i=0; i<n; i++) obj[i] = new Object();

5. Object *obj = new Object();
    obj->intArray = new int[10];

6. Object **obj = new Object[n];
    for (int i=0; i<n; i++) {
        obj[i] = new Object();
        obj[i]->intArray = new int[10];
    }

In advance, thanks very much.

[toc] | [next] | [standalone]

#13818

"Gus Gassmann"  wrote in message 
news:ba1d69e7-bd02-4c9a-9ab0-1f674fb3bc56@do4g2000vbb.googlegroups.com...
>
>I have been working with C++ for about four years now, mostly self-
>taught, and I get along reasonably well. However, last week I detected
>a memory leak in some code, and trying to locate it I realized that I
>do not know enough about destructors. I have created a number of use
>cases. How can I make sure in each case that the memory gets destroyed
>properly?

Of course this depens on the conext. But in general, use containers from the 
standard library, or make your own containers to avoid the use of "naked" 
new and delete.

>1. int *x[10];

std::vector<std::vector<int> > x;

>2. int *y = new int[10];

std::vector<int> y(10);


etc.

>3. Object *obj = new Object();
>
>4. Object **obj = new Object[n];
>    for (int i=0; i<n; i++) obj[i] = new Object();
>
>5. Object *obj = new Object();
>    obj->intArray = new int[10];
>
>6. Object **obj = new Object[n];
>    for (int i=0; i<n; i++) {
>        obj[i] = new Object();
>        obj[i]->intArray = new int[10];
>    }
>
>In advance, thanks very much.
>

[toc] | [prev] | [next] | [standalone]

#13819

Hi Gus

Gus Gassmann wrote:

> I have been working with C++ for about four years now, mostly self-
> taught, and I get along reasonably well. However, last week I detected
> a memory leak in some code, and trying to locate it I realized that I
> do not know enough about destructors. I have created a number of use
> cases. How can I make sure in each case that the memory gets destroyed
> properly?

Freds solution is the best because it is exception safe, but the
golden rule is:

when ever You use the keyword new it should be matched with the
keyword delete
and when You use new []  it should be matched with delete[]

1. int *x[10];
nothing.

2. int *y = new int[10];
delete[] y;

3. Object *obj = new Object();
delete obj;

etc.

But again Freds solution is better, because the vector takes care of the 
deletion when the vector runs out of scoop, even if there is an exception.


Best regards
Asger-P

[toc] | [prev] | [next] | [standalone]

#13826

On Feb 7, 11:38 am, "Asger Joergensen" <J...@Asger-P.dk> wrote:
> Hi Gus
>
> Gus Gassmann wrote:
> > I have been working with C++ for about four years now, mostly self-
> > taught, and I get along reasonably well. However, last week I detected
> > a memory leak in some code, and trying to locate it I realized that I
> > do not know enough about destructors. I have created a number of use
> > cases. How can I make sure in each case that the memory gets destroyed
> > properly?
>
> Freds solution is the best because it is exception safe, but the
> golden rule is:
>
> when ever You use the keyword new it should be matched with the
> keyword delete
> and when You use new []  it should be matched with delete[]
>
> 1. int *x[10];
> nothing.
>
> 2. int *y = new int[10];
> delete[] y;
>
> 3. Object *obj = new Object();
> delete obj;
>
> etc.
>
> But again Freds solution is better, because the vector takes care of the
> deletion when the vector runs out of scoop, even if there is an exception.

Some of the code is not written by me, so I am afraid Fred`s solution
does not work. Let me then summarize, just to be absolutely clear:

> 4. Object **obj = new Object[n];
>    for (int i=0; i<n; i++) obj[i] = new Object();

     for (int i=0; i<n; i++) delete obj[i];
     delete[] obj;

> 5. Object *obj = new Object();
>    obj->intArray = new int[10];

     delete[] obj->intArray;
     delete obj;

> 6. Object **obj = new Object[n];
>    for (int i=0; i<n; i++) {
>         obj[i] = new Object();
>         obj[i]->intArray = new int[10];
>    }

     for (int i=0; i<n; i++) {
         delete[] obj[i]->intArray;
         delete obj[i];
     }
     delete[] obj;

Will that do it? Thanks!

[toc] | [prev] | [next] | [standalone]

#13827

Hi Gus

Gus Gassmann wrote:

> Some of the code is not written by me, so I am afraid Fred`s solution
> does not work. Let me then summarize, just to be absolutely clear:
...
> 
> Will that do it? 

It looks fine to me, but allocating and deleting memory inside an object
from the outside is really bad practice, You should at least ad a 

void setIntSize( int size );
{
   delete[] intArray;
   intArray = new int[size]
}

to the object and then clean up in the destructor of the Object:

~Object()
{
   delete[] intArray;
} 

Do remember to zerro the intArray in the constructor.
Object(): intArray(0){}

Best regards
Asger-P

[toc] | [prev] | [next] | [standalone]

#13828

On 2/7/2012 12:34 PM, Asger Joergensen wrote:
> Hi Gus
>
> Gus Gassmann wrote:
>
>> Some of the code is not written by me, so I am afraid Fred`s solution
>> does not work. Let me then summarize, just to be absolutely clear:
> ...
>>
>> Will that do it?
>
> It looks fine to me, but allocating and deleting memory inside an object
> from the outside is really bad practice, You should at least ad a
>
> void setIntSize( int size );
> {
>     delete[] intArray;
>     intArray = new int[size]
> }
>
> to the object and then clean up in the destructor of the Object:
>
> ~Object()
> {
>     delete[] intArray;
> }
>
> Do remember to zerro the intArray in the constructor.
> Object(): intArray(0){}

To the OP: Why not use std::vector?  Or did you already answer that?

V
-- 
I do not respond to top-posted replies, please don't ask

[toc] | [prev] | [next] | [standalone]

#13838

It slows down as the list in other high level languages in basic operations.

The vector part in C++ offers auto-expansion and the boundary checking mechanism
absent from an  array. For a list or a vector in C++ of tens of thousands of objects in non-heavy computing tasks it can save troubles. 

But for digital images and videos of tens of millions of pixels to be processed
the C++ vector adds overheads, too. 
    
 

[toc] | [prev] | [next] | [standalone]

#13841

On 02/ 8/12 08:39 PM, 88888 Dihedral wrote:
> It slows down as the list in other high level languages in basic operations.
>
> The vector part in C++ offers auto-expansion and the boundary checking mechanism
> absent from an  array. For a list or a vector in C++ of tens of thousands of objects in non-heavy computing tasks it can save troubles.
>
> But for digital images and videos of tens of millions of pixels to be processed
> the C++ vector adds overheads, too.

No more than naked dynamic arrays.

-- 
Ian Collins

[toc] | [prev] | [next] | [standalone]

#13856

Ian Collins <ian-news@hotmail.com> writes:
> On 02/ 8/12 08:39 PM, 88888 Dihedral wrote:
>> But for digital images and videos of tens of millions of pixels to be
>> processed the C++ vector adds overheads, too.
>
> No more than naked dynamic arrays.

Well, possibly a little bit more (default initialisation, storing the
length, and possibly some address calculation if you want to address it
as if it were multidimensional, in zigzag fashion, say. Nothing you
couldn't fix with a templated custom class, though.) 

And if those are the speed determining step of your algorithm, you're
doing it wrong.

[toc] | [prev] | [next] | [standalone]

#13857

On Feb 8, 6:51 pm, Gareth Owen <gwo...@gmail.com> wrote:
> Ian Collins <ian-n...@hotmail.com> writes:
> > On 02/ 8/12 08:39 PM, 88888 Dihedral wrote:
> >> But for digital images and videos of tens of millions of pixels to be
> >> processed the C++ vector adds overheads, too.
>
> > No more than naked dynamic arrays.
>
> Well, possibly a little bit more (default initialisation, storing the
> length, and possibly some address calculation if you want to address it
> as if it were multidimensional, in zigzag fashion, say. Nothing you
> couldn't fix with a templated custom class, though.)
>
> And if those are the speed determining step of your algorithm, you're
> doing it wrong.

Default initialization was a major overhead in some of my image
processing
code. I replaced std::vector with a home-grown dynamic_array<T> vector
like container that doesn't do default initialization. Problem solved.
It would be nice if it could be done with a custom allocator but you
can't.

[toc] | [prev] | [next] | [standalone]

#13879

On Feb 7, 6:13 pm, Gus Gassmann wrote:
>
> Some of the code is not written by me, so I am afraid Fred`s solution
> does not work. Let me then summarize, just to be absolutely clear:
>
> > 4. Object **obj = new Object[n];
> >    for (int i=0; i<n; i++) obj[i] = new Object();
>
>      for (int i=0; i<n; i++) delete obj[i];
>      delete[] obj;
>
> > 5. Object *obj = new Object();
> >    obj->intArray = new int[10];
>
>      delete[] obj->intArray;
>      delete obj;
>
> > 6. Object **obj = new Object[n];
> >    for (int i=0; i<n; i++) {
> >         obj[i] = new Object();
> >         obj[i]->intArray = new int[10];
> >    }
>
>      for (int i=0; i<n; i++) {
>          delete[] obj[i]->intArray;
>          delete obj[i];
>      }
>      delete[] obj;
>
> Will that do it?

Not really. This is pretty fragile C++ code and doomed to break
eventually. For example, it's not exception-safe. Try to exploit the
strengths of C++. One of the biggest corner stones of modern C++
design is letting objects "own"/take care of other resources. C++
allows you to delegate the responsibility of cleaning up things so you
don't have to do it yourself.

  Fragile:
    int* p = new int[10];
    ...
    delete[] p;

  Good:
    vector<int> v (10);
    ...

To put it differently: If you want memory to be released if a pointer
goes out of scope, the raw pointer type would be a misuse because it
doesn't have the semantics you actually want.

Instead of new[] and delete[], prefer std::vector. Instead of "naked"
new, prefer the use of std::unique_ptr (or std::shared_ptr) so that
every heap-allocated object is "owned" by at least one other object.
This makes "delete" in user code rather obsolete. And even "new"
should be avoided in preference of std::make_shared and
std::make_unique (make_unique is not yet standard but you can write it
yourself

  template<class T, class...Args>
  std::unique_ptr<T> make_unique(Args&&...args)
  {
    return {new T(std::forward<Args>(args)...)};
  }

). In case you are worried about unnecessary copying with respect to
vectors and functions returning function-local vectors by value,
you'll be pleased to know that C++2011 will make sure this is about as
efficient as returning a raw pointer.

Cheers!
SG

[toc] | [prev] | [next] | [standalone]

#13903

在 2012年2月10日星期五UTC+8下午8时35分19秒，SG写道：
> On Feb 7, 6:13 pm, Gus Gassmann wrote:
> >
> > Some of the code is not written by me, so I am afraid Fred`s solution
> > does not work. Let me then summarize, just to be absolutely clear:
> >
> > > 4. Object **obj = new Object[n];
> > >    for (int i=0; i<n; i++) obj[i] = new Object();
> >
> >      for (int i=0; i<n; i++) delete obj[i];
> >      delete[] obj;
> >
> > > 5. Object *obj = new Object();
> > >    obj->intArray = new int[10];
> >
> >      delete[] obj->intArray;
> >      delete obj;
> >
> > > 6. Object **obj = new Object[n];
> > >    for (int i=0; i<n; i++) {
> > >         obj[i] = new Object();
> > >         obj[i]->intArray = new int[10];
> > >    }
> >
> >      for (int i=0; i<n; i++) {
> >          delete[] obj[i]->intArray;
> >          delete obj[i];
> >      }
> >      delete[] obj;
> >
> > Will that do it?
> 
> Not really. This is pretty fragile C++ code and doomed to break
> eventually. For example, it's not exception-safe. Try to exploit the
> strengths of C++. One of the biggest corner stones of modern C++
> design is letting objects "own"/take care of other resources. C++
> allows you to delegate the responsibility of cleaning up things so you
> don't have to do it yourself.
> 
>   Fragile:
>     int* p = new int[10];
>     ...
>     delete[] p;
> 
>   Good:
>     vector<int> v (10);
>     ...
> 
> To put it differently: If you want memory to be released if a pointer
> goes out of scope, the raw pointer type would be a misuse because it
> doesn't have the semantics you actually want.
> 
> Instead of new[] and delete[], prefer std::vector. Instead of "naked"
> new, prefer the use of std::unique_ptr (or std::shared_ptr) so that
> every heap-allocated object is "owned" by at least one other object.
> This makes "delete" in user code rather obsolete. And even "new"
> should be avoided in preference of std::make_shared and
> std::make_unique (make_unique is not yet standard but you can write it
> yourself
> 
>   template<class T, class...Args>
>   std::unique_ptr<T> make_unique(Args&&...args)
>   {
>     return {new T(std::forward<Args>(args)...)};
>   }
> 
> ). In case you are worried about unnecessary copying with respect to
> vectors and functions returning function-local vectors by value,
> you'll be pleased to know that C++2011 will make sure this is about as
> efficient as returning a raw pointer.
> 
> Cheers!
> SG

In C++  objects in  classes with several vectors or other containers
are allowed. Even copying an object is not so trivial.

[toc] | [prev] | [next] | [standalone]

#13830

On Feb 7, 6:53 am, Gus Gassmann <horand.gassm...@googlemail.com>
wrote:
> I have been working with C++ for about four years now, mostly self-
> taught, and I get along reasonably well. However, last week I detected
> a memory leak in some code, and trying to locate it I realized that I
> do not know enough about destructors. I have created a number of use
> cases. How can I make sure in each case that the memory gets destroyed
> properly?
>
> 1. int *x[10];
>
> 2. int *y = new int[10];
>
> 3. Object *obj = new Object();
>
> 4. Object **obj = new Object[n];
>     for (int i=0; i<n; i++) obj[i] = new Object();
>
> 5. Object *obj = new Object();
>     obj->intArray = new int[10];
>
> 6. Object **obj = new Object[n];
>     for (int i=0; i<n; i++) {
>         obj[i] = new Object();
>         obj[i]->intArray = new int[10];
>     }
>
> In advance, thanks very much.

Each call to new must be matched by exactly one call to delete, on the
exact same type of pointer, or a pointer to base class with a virtual
destructor. And the same for new[] and delete[] - each call to new[]
must be matched by exactly one call to delete[].

"Smart pointers" such as std::auto_ptr and std::unique_ptr, using
std::vector in place of new[] and delete[], using RAII in general,
etc., can help you ensure that you're not missing the matching calls
to delete and delete[].

[toc] | [prev] | [next] | [standalone]

#13837

Gus Gassmann <horand.gassmann@googlemail.com> wrote:
> How can I make sure in each case that the memory gets destroyed
> properly?

  Since C++ does not have garbage collection of dynamically allocated
memory, the approach at making safe code is different from many other
languages.

  As a general rule of thumb, if your code has 'new's and 'delete's
interprersed with other code, it's a big warning flag that you should
re-design your code. (Having 'new's and 'delete's in code, rather than
being encapsulated inside classes, usually means that your code is
error-prone and not exception-safe. It usually also means that it's more
complicated than it has to be.)

  It's better if all dynamically allocated memory (and in fact, all
dynamically allocated resources in general) are managed by a class that
takes advantage of RAII. (There may be certain situations which are
exceptions to this, but they are rare.)

  As suggested in the rest of the thread, the standard library already
offers you many classes that take care of managing memory for you, so
that you don't have to write a single 'new' or 'delete'. These are very
useful, and in fact I'd say at least 95% of dynamic memory management
in my own code is done using them.

  But of course you shouldn't just blindly use them without understanding
how they work and what are their limitations, drawbacks and overhead.
(For example, using std::vector as a class member when a static array
would do makes your class significantly less efficient.)

  While you can use the standard library containers and smart pointers
(especially since C++11) for something like at least 95% of the code, they
are not a panacea for all possible situations. There may be situations
where you just *have* to handle dynamically allocated memory yourself
(eg. if you need a data container type that the standard library does
not offer, or if you want to implement special management such as
copy-on-write or reference counting).

  In these cases you should definitely take note on how the standard
library containers have been made (at least in terms of their constructors,
assignment and destructor). Learn the proper way of designing such a class
(google for "rule of three C++" for an idea).

[toc] | [prev] | [next] | [standalone]

#13859

Am 08.02.2012 07:38, schrieb Juha Nieminen:

>   But of course you shouldn't just blindly use them without understanding
> how they work and what are their limitations, drawbacks and overhead.
> (For example, using std::vector as a class member when a static array
> would do makes your class significantly less efficient.)

This is just what happened today at work. We have a loop that iterates
over a buffer of 16 bit audio samples and uses a 64k look-up table for
each sample. The table was a class member vector<int16_t> with an
indexing function:

inline int16_t& lut(int16_t idx) { return table[idx]; }

That function was directly defined in the class definition, was marked
inline, and yet the compiler generated a call to it in the loop body.
When we marked that function with #pragma always_inline etc. the
compiler generated a warning that the function exceeded the speed/size
ratio, but that it would inline it anyway. When we looked at the
disassembly, there was still a call in the loop body, probably to the
operator[] of the vector (it was a long mangled name...).

Only when we changed the table to

int16_t table[65536];

the loop body became well optimized - even with the lut() function used.
And there was no need anymore to force it inline.

The code runs on a Blackfin DSP, and the compiler is Analog Devices'
blackfin compiler.

I was surprised because I thought the compiler should be able to inline
all the one-liners involved, both the lut() function and the vector's
indexing operator and properly optimize the loop. I was wrong...

No kidding about the "significantly less efficient" part.


Peter

[toc] | [prev] | [next] | [standalone]

#13860

Am 09.02.2012 00:30, schrieb Peter Remmers:

> inline int16_t& lut(int16_t idx) { return table[idx]; }

I wrote this from memory. Certainly, idx was not a signed type. I think
the samples are of some special fract16 type or something. Anyway, that
doesn't matter, as it's beside the point I wanted to make.


Peter

[toc] | [prev] | [next] | [standalone]

#13877

On Feb 8, 3:30 pm, Peter Remmers
<p.remm...@expires-2011-04-30.arcornews.de> wrote:
> Am 08.02.2012 07:38, schrieb Juha Nieminen:
>
> >   But of course you shouldn't just blindly use them without understanding
> > how they work and what are their limitations, drawbacks and overhead.
> > (For example, using std::vector as a class member when a static array
> > would do makes your class significantly less efficient.)
>
> This is just what happened today at work. We have a loop that iterates
> over a buffer of 16 bit audio samples and uses a 64k look-up table for
> each sample. The table was a class member vector<int16_t> with an
> indexing function:
>
> inline int16_t& lut(int16_t idx) { return table[idx]; }
>
> That function was directly defined in the class definition, was marked
> inline, and yet the compiler generated a call to it in the loop body.
> When we marked that function with #pragma always_inline etc. the
> compiler generated a warning that the function exceeded the speed/size
> ratio, but that it would inline it anyway. When we looked at the
> disassembly, there was still a call in the loop body, probably to the
> operator[] of the vector (it was a long mangled name...).
>
> Only when we changed the table to
>
> int16_t table[65536];
>
> the loop body became well optimized - even with the lut() function used.
> And there was no need anymore to force it inline.
>
> The code runs on a Blackfin DSP, and the compiler is Analog Devices'
> blackfin compiler.
>
> I was surprised because I thought the compiler should be able to inline
> all the one-liners involved, both the lut() function and the vector's
> indexing operator and properly optimize the loop. I was wrong...
>
> No kidding about the "significantly less efficient" part.

Well, all of the usual claims of good C++ efficiency with good C++
style assume a good compiler. Apparently this one needs some tuning...

[toc] | [prev] | [next] | [standalone]

#13988

On Thu, 2012-02-09, Joshua Maurice wrote:
> On Feb 8, 3:30 pm, Peter Remmers
> <p.remm...@expires-2011-04-30.arcornews.de> wrote:
...
>> over a buffer of 16 bit audio samples and uses a 64k look-up table for
>> each sample. The table was a class member vector<int16_t> with an
>> indexing function:
>>
>> inline int16_t& lut(int16_t idx) { return table[idx]; }
...
>> The code runs on a Blackfin DSP, and the compiler is Analog Devices'
>> blackfin compiler.
...
>> No kidding about the "significantly less efficient" part.
>
> Well, all of the usual claims of good C++ efficiency with good C++
> style assume a good compiler. Apparently this one needs some tuning...

Or perhaps some debug version of std::vector was used.

/Jorgen

-- 
  // Jorgen Grahn <grahn@  Oo  o.   .     .
\X/     snipabacken.se>   O  o   .

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.c++

csiph-web