Memory deallocation does not work.

[christophe.chazeau]

Hi,
I have a problem with a really simple chunk of code which should work
but does obviously does not.
This chunk of code is just a POC aimed at finding a bug in a larger
project in which the same problem seems to occur.
Here the deal : when I run this piece of code, I expect all the memory
allocated by the "Test" object to be freed but what I observe is that
after the second sleep (after all the additions to the vector), the
memory taken by the process is 750Meg which is normal but after the
last sleep, the memory occupied by the process stays as high as 590Meg
when I expected it to be freed completely.

What exactly is wrong with this code ??
Thanx a lot in advance


Code :

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <vector>
#include <iterator>


using namespace std;


class Test{
public :
unsigned char *data ;

Test(){

}
void Go(int taille)
{

data = (unsigned char*) malloc(taille);
printf("malloc : %p\n",data);
}

~Test()
{
printf("delete %p\n",data);
free(data);
}

};

vector<Test *> v;
int i = 0 ;

int main()
{

sleep(10);

for(i = 0;i<10000;i++)
{
Test *t = new Test();
t->Go(75000);
v.push_back(t);
}

sleep(10);

vector<Test *>::iterator it;
for(it=v.begin() ; it!=v.end();it++)
{

Test *t = *it;
delete(t);
}
sleep(10);

}

 

[Andre Kostur]

(e-mail address removed) wrote in @q2g2000cwa.googlegroups.com:

Hi,
I have a problem with a really simple chunk of code which should work
but does obviously does not.

No... it's not obvious that it does not "work".

This chunk of code is just a POC aimed at finding a bug in a larger
project in which the same problem seems to occur.
Here the deal : when I run this piece of code, I expect all the memory
allocated by the "Test" object to be freed but what I observe is that
after the second sleep (after all the additions to the vector), the
memory taken by the process is 750Meg which is normal but after the
last sleep, the memory occupied by the process stays as high as 590Meg
when I expected it to be freed completely.

What exactly is wrong with this code ??
Thanx a lot in advance

The assumption you have is that when you delete (or free) memory from your
program that it is necessarily handed back to the OS. That's
implementation-specific behaviour as to whether if and/or when that is
actually done.

 

[red floyd]

Hi,
I have a problem with a really simple chunk of code which should work
but does obviously does not.
This chunk of code is just a POC aimed at finding a bug in a larger
project in which the same problem seems to occur.
Here the deal : when I run this piece of code, I expect all the memory
allocated by the "Test" object to be freed but what I observe is that
after the second sleep (after all the additions to the vector), the
memory taken by the process is 750Meg which is normal but after the
last sleep, the memory occupied by the process stays as high as 590Meg
when I expected it to be freed completely.

What exactly is wrong with this code ??
Thanx a lot in advance

Other than the use of malloc/free instead of new[]/delete[]?

As far as I can tell, nothing. What is wrong is your expectation. free
and delete return the memory to the free store. Whether said memory is
returned to the operating system is completely system dependent.

So the memory is being deallocated in the sense that accessing memory
through the pointers is no undefined, as the memory is returned to the
free store, it's just not being returned to the OS.

If you need such behavior to occur, you will need to deal with OS
specifics. I suggest asking in a newsgroup dedicated to your platform.
You can find a partial list of such newsgroups at
http://www.parashift.com/c++-faq-lite/how-to-post.html#faq-5.9

 

[christophe.chazeau]

Hi,
I have a problem with a really simple chunk of code which should work
but does obviously does not.
This chunk of code is just a POC aimed at finding a bug in a larger
project in which the same problem seems to occur.
Here the deal : when I run this piece of code, I expect all the memory
allocated by the "Test" object to be freed but what I observe is that
after the second sleep (after all the additions to the vector), the
memory taken by the process is 750Meg which is normal but after the
last sleep, the memory occupied by the process stays as high as 590Meg
when I expected it to be freed completely.

What exactly is wrong with this code ??
Thanx a lot in advance

Other than the use of malloc/free instead of new[]/delete[]?

Yeah I know but we need to reallocate from time to time, so that's the
reason why we chose malloc/free
Do you know a better way of dealing with variable size buffer in C++
objects ?

As far as I can tell, nothing. What is wrong is your expectation. free
and delete return the memory to the free store. Whether said memory is
returned to the operating system is completely system dependent.

So the memory is being deallocated in the sense that accessing memory
through the pointers is no undefined, as the memory is returned to the
free store, it's just not being returned to the OS.

If you need such behavior to occur, you will need to deal with OS
specifics. I suggest asking in a newsgroup dedicated to your platform.
You can find a partial list of such newsgroups athttp://www.parashift.com/c++-faq-lite/how-to-post.html#faq-5.9

Thanx a lot for the answer : that's what I wanted to hear : the memory
is freed but not yet returned to the OS.
Would it eventually be returned after a certain amount of time ? That
is to say if the last sleep is long enough, would the used virtual
memory decrease in a top (Yeah I forgot to mention all this is
intended to run under Linux)

Anyway Thank you to you and Andre.

Christophe

 

[red floyd]

Other than the use of malloc/free instead of new[]/delete[]?

Yeah I know but we need to reallocate from time to time, so that's the
reason why we chose malloc/free
Do you know a better way of dealing with variable size buffer in C++
objects ?

std::vector

 

[Grizlyk]

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <vector>
#include <iterator>


using namespace std;

class Test{
public :
unsigned char *data ;

Test(){

}
void Go(int taille)
{

data = (unsigned char*) malloc(taille);
printf("malloc : %p\n",data);
}

~Test()
{
printf("delete %p\n",data);
free(data);
}

};

vector<Test *> v;

It is dangerous - possible memory leak because vector<Test *> does not own
the object pointer point to. Use any RAII memory wrapper (see RAII here:
http://www.hackcraft.net/raii ) instead of Test *

vector<auto_ptr<Test> > v;

or give each address (stored in vector<Test *>) to other object, which will
keep object.

{
auto_ptr<Test> owner(new Test);
vector<Test *> v;
v.push_back(*owner);
}

int i = 0 ;

int main()
{

sleep(10);

for(i = 0;i<10000;i++)
{
Test *t = new Test();
t->Go(75000);
v.push_back(t);
}

sleep(10);

vector<Test *>::iterator it;
for(it=v.begin() ; it!=v.end();it++)

Do you need "it" outside of "for"?
Try to inplement postfix operator ++, and you will see, that pferix ++ is
better.
Avoid repeating function call "v.end()" if you are sure, that the function
will always return the same.

vector<Test *>::iterator end(v.end());
for(vector<Test *>::iterator it(v.begin()); it!=end; ++it)

{

Test *t = *it;
delete(t);
}
sleep(10);

}

To return memory to OS, instead of std malloc/free you need to implement
native for you OS memory manager

namespace Nxxx_OS
{
void* malloc(); //will call OS specific allocate
void free(void*const); //will call OS specific free

//namespace Nxxx_OS
}

Note, you need to stay coherrent with all other memory management, so
probably you need to change all system memory allocation for the target.

 

[Andre Kostur]

It is dangerous - possible memory leak because vector<Test *> does not
own the object pointer point to. Use any RAII memory wrapper (see RAII
here: http://www.hackcraft.net/raii ) instead of Test *

Uh... not all RAII memory wrappers are usable for this purpose.

vector<auto_ptr<Test> > v;

Specifically auto_ptr is not!

or give each address (stored in vector<Test *>) to other object,
which will keep object.

{
auto_ptr<Test> owner(new Test);
vector<Test *> v;
v.push_back(*owner);
}

Only if you know these other objects are going to have a longer lifetime
than your vector...

 

[Kai-Uwe Bux]

It is dangerous - possible memory leak because vector<Test *> does not own
the object pointer point to. Use any RAII memory wrapper (see RAII here:
http://www.hackcraft.net/raii ) instead of Test *

vector<auto_ptr<Test> > v;

That is _bad_ and much more dangerous than vector<Test*>: auto_ptr<> does
not satisfy the requirements for use in a container. Formally, you have
undefined behavior. In practice, you may run into trouble as soon as vector
reallocates.

or give each address (stored in vector<Test *>) to other object, which
will keep object.

{
auto_ptr<Test> owner(new Test);
vector<Test *> v;
v.push_back(*owner);

*owner is a Test& not a Test*. Do you mean

v.push_back( owner.operator->() );

}

What happens at this "}"? The auto_ptr<Test> owner destroys the pointee. The
pointer stored in v now is invalid. Any further use is undefined behavior.

What exactly are you trying to to? Should you be headed for automatic memory
management, consider vector< shared_ptr<Test> > or a special container for
pointers that implements some ownership model.

Do you need "it" outside of "for"?

That is a good point.

Try to inplement postfix operator ++, and you will see, that pferix ++ is
better.

"better" in which way?

Should you think of performance, your advice amounts to premature
optimization: chances are that it++ is inlined and optimized to the same
code that ++it would generate.

Avoid repeating function call "v.end()" if you are sure, that the function
will always return the same.

Very likely, this is also premature optimization.

[snip]


Best

Kai-Uwe Bux

 

[Grizlyk]

That is _bad_ and much more dangerous than vector<Test*>: auto_ptr<> does
not satisfy the requirements for use in a container. Formally, you have
undefined behavior. In practice, you may run into trouble as soon as
vector
reallocates.

I have assumed you have write you own::auto_ptr<> with the same behaviour.
You can garantee at least runtime error is not sure, that std::vector<> does
usage of its data sequentally. The auto_ptr idiom (auto_ptr must move data
on copy) is so useful, that you can even replace std::vector<> by
own::vector<> if std::vector<> do not support auto_ptr idiom.

*owner is a Test& not a Test*. Do you mean
v.push_back( owner.operator->() );


What happens at this "}"? The auto_ptr<Test> owner destroys the pointee.
The
pointer stored in v now is invalid. Any further use is undefined behavior.

At the point "v" is also destroyed. Lifetime of "owner" evidently must be
more than lifetime of "user" ("v").

What exactly are you trying to to? Should you be headed for automatic
memory
management, consider vector< shared_ptr<Test> > or a special container for
pointers that implements some ownership model.

The "shared_ptr<Test>" is very, very, very rare used. I even can not
remember when I used it last time, of course, if you have no suitable

"better" in which way?

In all ways. Operation "*p++=0;" has perfomans sence only if it is single
opcode for POD.
For POD also builtin increment p++ and ++p can subst each other and compiler
can generate "*p=0, ++p;" automatically if needed.
For non POD both increments p++ and ++p are implemened by different user
functions, so for non POD just write "*p=0, ++p;".

Should you think of performance, your advice amounts to premature
optimization: chances are that it++ is inlined and optimized to the same
code that ++it would generate.

Maybe, but not required by C++. When you say "it++", you tell to compiler,
that you want a copy of "it" returned after operations, else you change
semantic of post-increment. And process of creation of copy can be hard to
eliminate. Anyway, you must not create copies if you no need it and you must
not assume, that creation of copy can be eliminated if you not use result.

Very likely, this is also premature optimization.

I do not know is "premature" bad or good, but you must not call function
many times, if you want only one value returned. You must not assume, that
unknown function do nothing.

 

[Grizlyk]

Uh... not all RAII memory wrappers are usable for this purpose.

Yes, all depends from usage.

Specifically auto_ptr is not!

Maybe std::auto_ptr can not, but auto_ptr is idiom, make own auto_ptr if you
want. Anyway, auto_ptr is just example of RAII wrapper.

Only if you know these other objects are going to have a longer lifetime
than your vector...

Yes.

 

[Grizlyk]

I do not know is "premature" bad or good, but you must not call function
many times, if you want only one value returned. You must not assume, that
unknown function do nothing.

[qoute]
“Premature optimization” is a phrase used to describe a situation where a
programmer lets performance considerations affect the design of a piece of
code. This can result in a design that is not as clean as it could have been
or code that is incorrect, because the code is complicated by the
optimization and the programmer is distracted by optimizing.
[/qoute]

It is evidently, that avoid a lot of useless calls of unknown function is
not satisfy definition of “Premature optimization” by
- design is not as clean
- code is complicated

And replace "it++" to "++it" is also does not satisfy definition of
“Premature optimization”.

If you think, that programmer must know how "it++" or "v.end()" will exactly
inlined, that you are wrong, because goal of any function is hiding its
implementation by function declaration: name, return value and list of
parameters. Any function is stuff for abstraction.

 

[Kai-Uwe Bux]

I do not know is "premature" bad or good, but you must not call function
many times, if you want only one value returned. You must not assume,
that unknown function do nothing.

[qoute]
“Premature optimization” is a phrase used to describe a situation where a
programmer lets performance considerations affect the design of a piece of
code. This can result in a design that is not as clean as it could have
been or code that is incorrect, because the code is complicated by the
optimization and the programmer is distracted by optimizing.
[/qoute]

It is evidently, that avoid a lot of useless calls of unknown function is
not satisfy definition of “Premature optimization” by
- design is not as clean
- code is complicated

The definition was given in the first sentence of the quote. It clearly
satisfies that. It may or may not result in the consequences described in
the second sentence of the quote. In the case of .end(), optimizing that
call away manually will at least result in

a) the introduction of an additional variable and
b) a requirement not to change the sequence within the for loop -- a
requirement that could be broken during maintenance programming and the
compiler will not warn.

And replace "it++" to "++it" is also does not satisfy definition of
“Premature optimization”.

If you do so for performance reasons, it does satisfy the definition,
although in this case without the negative impact.

If you think, that programmer must know how "it++" or "v.end()" will
exactly inlined, that you are wrong, because goal of any function is
hiding its implementation by function declaration: name, return value and
list of parameters. Any function is stuff for abstraction.

I did not say anything about "know". What I say, is that there is no way to
know performance without measurement. Unless a piece of code is proven to
be a performance bottleneck, performance considerations should not enter
the picture.


Best

Kai-Uwe Bux

 

[Grizlyk]

Avoid repeating function call "v.end()" if you are sure, that the
function
will always return the same.

Very likely, this is also premature optimization.

I do not know is "premature" bad or good, but you must not call function
many times, if you want only one value returned. You must not assume,
that unknown function do nothing.

[qoute]
“Premature optimization” is a phrase used to describe a situation where a
programmer lets performance considerations affect the design of a piece
of
code. This can result in a design that is not as clean as it could have
been or code that is incorrect, because the code is complicated by the
optimization and the programmer is distracted by optimizing.
[/qoute]

It is evidently, that avoid a lot of useless calls of unknown function is
not satisfy definition of “Premature optimization” by
- design is not as clean
- code is complicated

The definition was given in the first sentence of the quote. It clearly
satisfies that.

It may or may not result in the consequences described in
the second sentence of the quote. In the case of .end(), optimizing that
call away manually will at least result in

a) the introduction of an additional variable and
b) a requirement not to change the sequence within the for loop -- a
requirement that could be broken during maintenance programming and the
compiler will not warn.

Let people decide: to call unknown "end()" 500 times or to declare const
temporary only one time and where is “Premature optimization” here.

a)It is interesting, do you beleive that call of end() will not require
hidden temporary or you disagree only with explicit name of const value
maked for const comparing?

b)"a requirement not to change the sequence within the for loop " do not
understand this.

If you do so for performance reasons, it does satisfy the definition,
although in this case without the negative impact.

Just to say "It clearly satisfies that." does not prove condition of satisfy
definition. I can say, that free using "it++" instead of "++it" can to speak
that the man, who is doing it, do not know differences between prefix and
postfix C++ increment operators.

I did not say anything about "know". What I say, is that there is no way
to
know performance without measurement. Unless a piece of code is proven to
be a performance bottleneck, performance considerations should not enter
the picture.

Let I have made measurement and what? Measurement prove poor perfomance and
what do? Code must not assume nothing about function implementation and good
user code can be compiled with high perfomans for any implementation of the
function, so implementation of the function can be developed independent
from user code.

 

[Gavin Deane]

Let people decide: to call unknown "end()" 500 times or to declare const
temporary only one time and where is "Premature optimization" here.

It is premature because you have not yet proven (by measurement - the
only way that matters) that calling end() on every iteration is
causing a peformance bottleneck. Unless and until you have done that,
clear human readable code that is robust in the face of changes to
code around it should be a higher concern than performance.

1.
iterator temp = v.end();
for (iterator i = v.begin(); i != temp; ++i)
{
// Do stuff that does not alter the sequence...
}

2.
for (iterator i = v.begin(); i != v.end(); ++i)
{
// Do stuff that does not alter the sequence...
}

For me, the second is less cluttered and so more eaily readable, which
is important. But to a certain extent human readability is in the eye
of the beholder. YMMV.

However, the second is definitely more robust. For example, if the
loop body is changed to:
// Do stuff that can (or will) alter the sequence...
If this happens, the first example may break because the value of
v.end() could change on each iteration of the loop, but the loop is
written to terminate when i is equal the value v.end() returned before
the first iteration.

a)It is interesting, do you beleive that call of end() will not require
hidden temporary or you disagree only with explicit name of const value
maked for const comparing?

This is simply not relevant compared to robustness and readability
unless and until I have measurements that prove I have a performance
bottleneck that must be fixed.

b)"a requirement not to change the sequence within the for loop " do not
understand this.

See above for an example.

Gavin Deane

 

[Emmanuel Deloget]

Yes, all depends from usage.



Maybe std::auto_ptr can not, but auto_ptr is idiom, make own auto_ptr if you
want. Anyway, auto_ptr is just example of RAII wrapper.


Yes.

--
Maksim A. Polyanin

"In thi world of fairy tales rolls are liked olso"
/Gnume/

Instead of rolling your own version of auto_ptr (which will not adhere
to the same semantics as auto_ptr, making it even more confusing), I
suggest proven and existing alternatives: tr1::shared_ptr<> or
boost::shared_ptr<> come to mind, and loki also implement another
variation on this.

Regarding OP's second question ( Would it eventually be returned after
a certain amount of time ? That is to say if the last sleep is long
enough, would the used virtual memory decrease in a top (Yeah I forgot
to mention all this is intended to run under Linux ): maybe, maybe
not. In order to give the memory back to the OS, the freed memory must
be a contiguous block and must not be followed by a non-freed memory
block. And even if that condition is satisfied, it is up to the OS to
deal with this kind of issue (ie. if the OS supports that feature, it
is likely to do so. Otherwise, you're screwed).

In the end, that's an OS related question, not a C++ one.

Regards,

-- Emmanuel Deloget

 

[Grizlyk]

It is premature because you have not yet proven (by measurement - the
only way that matters) that calling end() on every iteration is
causing a peformance bottleneck. Unless and until you have done that,
clear human readable code that is robust in the face of changes to
code around it should be a higher concern than performance.

1.
iterator temp = v.end();
for (iterator i = v.begin(); i != temp; ++i)
{
// Do stuff that does not alter the sequence...
}

2.
for (iterator i = v.begin(); i != v.end(); ++i)
{
// Do stuff that does not alter the sequence...
}

Tell me, you really think that implementation of "v.end()" must be const
during lifetime of program?

However, the second is definitely more robust. For example, if the
loop body is changed to:
// Do stuff that can (or will) alter the sequence...
If this happens, the first example may break because the value of
v.end() could change on each iteration of the loop, but the loop is
written to terminate when i is equal the value v.end() returned before
the first iteration.

We must make differences between loop with const v.end() in "for" condition
and loop with non-const v.end() in for" condition. The loops with const
v.end() in for" condition can be optimised, by analogy, there are
differences between copy constructor and assignment operator.

This is simply not relevant compared to robustness and readability
unless and until I have measurements that prove I have a performance
bottleneck that must be fixed.

Say "yes" (hidden temporary will be created) or "no" (never will do).

 

[Grizlyk]

Yes, all depends from usage.



Maybe std::auto_ptr can not, but auto_ptr is idiom, make own auto_ptr if
you
want. Anyway, auto_ptr is just example of RAII wrapper.


Instead of rolling your own version of auto_ptr (which will not adhere
to the same semantics as auto_ptr, making it even more confusing).

No any confusing. If the man can not write simplest RAII wrapper how he will
write his own programs, that must be more, more complex?

The stdlib is not a Bible, it is just ordinary library and can be easy
replaced.

The behaviour of trivial data types, represented in stdlib as classes
"list", "vector" etc is so well-defined, that can not be no one question
about its usage.

If you can not directly use stdlib trivial data type
- you out of concrete interface, you write "it.next()" instead of "++it".
- stdlib implementation is incorrect

No other possibilities exist here.

I suggest proven and existing alternatives: tr1::shared_ptr<> or
boost::shared_ptr<> come to mind, and loki also implement another
variation on this.

Shared_ptr<> does not prevent you from your own classes, because
implementation of shared_ptr<> idiom is overhead in comparison with
auto_ptr<> idiom.

 

[Gavin Deane]

Tell me, you really think that implementation of "v.end()" must be const
during lifetime of program?

I'm not sure what you mean.

The *implementation* of v.end() is some code, not an object that may
or may not be const. Unless we are in a template function and the type
of v depends on template parameters, or end() is a virtual function,
then the code that constitutes the implementation of v.end() is of
course constant. Code does not spontaneously change at run-time.

But that's a fairly flippant answer and that makes me think that I may
have misunderstood you.

We must make differences between loop with const v.end() in "for" condition
and loop with non-const v.end() in for" condition.

No. We absolutely must *not* make such a distinction. Because if we
write the for statement in such a way that assumes that the value of
v.end() can not change during an iteration of the loop (my example 1
above), then there is a risk of introducing a bug when the loop body
is altered such that the value of v.end() might change during an
iteration. That risk can be easily avoided by writing the for
statement as in my example 2, and IMO you get a readbility benefit
too. The only down side of doing this is that, for cases where the
value of v.end() can not be modified by the loop body, the code might
take longer and/or use more memory than necessary. But that is
irrelevant unless and until you have proof that the speed of execution
and/or memory footprint of this code is preventing you from meeting
your users' requirements.

The loops with const
v.end() in for" condition can be optimised, by analogy, there are
differences between copy constructor and assignment operator.

If by that analogy you mean the difference between my two examples is
equivalent to the difference betweem

1.
some_type foo = initial_value;

2.
some_type foo;
foo = initial_value;

then I don't believe the analgoy holds. In this case I would recommend
1 because it is never worse and sometimes better than 2. For the same
reason, I would recommend pre-increment over post-increment when you
don't need the side effect of post-increment because pre-increment is
never worse and sometimes better.

But in the for loop example, both examples have pros and cons. In that
case, 2 has better readability and better robustness but potentially
worse performance. In order to decide between the options, you need to
decide which of those factors are important. I am saying that
robustness and readability are important enough that the only time it
is worth sacrificing either of them them in favour of performance is
when you are unable to meet your users' requirements without doing so.

Say "yes" (hidden temporary will be created) or "no" (never will do).

I don't need to say "yes" or "no" because I have explained why I
believe the question to be irrelevant. When choosing between style 1
and style 2 for a for loop, I am no more concerned about your question
[*] than I am concerned about what film I might like to see at the
cinema that evening. Both questions are equally irrelevant to me.

[*] (unless I already know I have a performance bottleneck).

Gavin Deane