Cannot understand this memory leak.

[Simon]

Hi,

I have some old code that I am trying to use in my current project.
But my compiler is reporting a memory leak. (it does not tell me where the
leak comes from).

I have narrowed down the code as follow where the error could be.

// --
// in the header
std::string *m_pData;

//-------
// in the constructor
m_pData = new std::string[ someKnownSize + 1]
memset( m_pData, 0, (n+1)*sizeof(std::string) );
...

//-------
// The destructor
if( NULL != m_pData )
delete [] ( m_pData );
m_pData = NULL;
m_nSize = 0;
//-------
// in a function
void add( std::string &s, int len )
{
m_pData[m_nSize++] = std::string(s, len ) ;
}

//-------

if I remove the line "m_pData[m_nSize++] = std::string(s, len ) ;" then I
have no leak.

What could be the problem and how can I find the leak?

Many thanks

Simon

[Victor Bazarov]

Simon wrote:

Hi,

I have some old code that I am trying to use in my current project.
But my compiler is reporting a memory leak. (it does not tell me
where the leak comes from).

I have narrowed down the code as follow where the error could be.

// --
// in the header
std::string *m_pData;

//-------
// in the constructor
m_pData = new std::string[ someKnownSize + 1]
memset( m_pData, 0, (n+1)*sizeof(std::string) );

^^^^^^^^^^^^^^^^
Throw this 'memset' away (comment it out). You shouldn't do it
at all.

..

//-------
// The destructor
if( NULL != m_pData )
delete [] ( m_pData );

There is no need to check for NULL.

m_pData = NULL;
m_nSize = 0;

Setting _your own_ data member values in the destructor is
pointless, they are going away.

>

//-------
// in a function
void add( std::string &s, int len )
{
m_pData[m_nSize++] = std::string(s, len ) ;
}

//-------

if I remove the line "m_pData[m_nSize++] = std::string(s, len ) ;"
then I have no leak.

What could be the problem and how can I find the leak?

There does not seem to be the reason for any leak, but your library
implementation may be buggy or your tool is reporting false positives
on the leaks.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

[Tristan Wibberley]

On Wed, 2008-01-23 at 21:51 +0200, Simon wrote:

[C-style code with bug reminiscent of the days before I started using C++ snipped]

I suggest this instead:


// --
// in the header
std::vector<std::stringdata;

//-------
// in the constructor
data.reserve(someKnownSize + 1);
..

//-------
// The destructor
// GONE! Yay! all simple

//-------
// in a function
void add( std::string &s, int len )
{
data.push_back(std::string(s, len )) ;
}

//-------

--
Tristan Wibberley

Any opinion expressed is mine (or else I'm playing devils advocate for
the sake of a good argument). My employer had nothing to do with this
communication.

[Simon]

// --

// in the header
std::vector<std::stringdata;

//-------
// in the constructor
data.reserve(someKnownSize + 1);
..

//-------
// The destructor
// GONE! Yay! all simple

//-------
// in a function
void add( std::string &s, int len )
{
data.push_back(std::string(s, len )) ;
}

//-------

Thanks, doing it this way is far easier and does work.
As mentioned the memset was probably the problem anyway.

The reason why I did this in the first place was because I was told that
push_back() was very time consuming.

As I know the size I was hopping that declaring an array would buy me a few
ticks.

do you think that using push_back is the fastest way I can fill my array?

Thanks again.

Simon

[Victor Bazarov]

Simon wrote:

>// --
// in the header
std::vector<std::stringdata;

//-------
// in the constructor
data.reserve(someKnownSize + 1);
..

//-------
// The destructor
// GONE! Yay! all simple

//-------
// in a function
void add( std::string &s, int len )
{
data.push_back(std::string(s, len )) ;
}

//-------

Thanks, doing it this way is far easier and does work.
As mentioned the memset was probably the problem anyway.

The reason why I did this in the first place was because I was told
that push_back() was very time consuming.

As I know the size I was hopping that declaring an array would buy me
a few ticks.

do you think that using push_back is the fastest way I can fill my
array?

'push_back' is relatively fast in filling the pre-allocated vector
(for which you did 'reserve'), since it does not require any
reallocations. If you're concerned, profile your application to
see how much time is spent in 'push_back' and then decide if it is
acceptable.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

[Puppet_Sock]

On Jan 24, 2:02*pm, "Simon" <[e-mail]:
[snip]

The reason why I did this in the first place was because I was told that
push_back() was very time consuming.

As I know the size I was hopping that declaring an array would buy me a few
ticks.

Do you in fact have some kind of spec on how fast
your program has to be? If not, why are you wasting
your time trying to optimize it for speed?

Get it right. Then get it fast. Don't worry about
omtimizing on such little issues as this until
you actually have something like a working code.
Then if it fails to meet the performance spec,
go looking for optimizations. And be sure to take
a stop watch with you to determine if a given
optimization in fact makes any appreciable change.

do you think that using push_back is the fastest way I can fill my array?

I think that you need to worry about other things
before you worry about doing things the fastest
way you can.
Socks

[Simon]

>The reason why I did this in the first place was because I was told that

>push_back() was very time consuming.

As I know the size I was hopping that declaring an array would buy me a
few
ticks.

>Do you in fact have some kind of spec on how fast
your program has to be? If not, why are you wasting
your time trying to optimize it for speed?

Well the files are text files loaded at run time, (once with some kind of
progress bar).
The faster the file is loaded the better.

Currently it takes between 10 to 20 seconds on a 'normal/average' machine.

Ideally I would convert the file so it is loaded in my array as fast as
possible.
It is all about user experience.

Get it right. Then get it fast. Don't worry about
omtimizing on such little issues as this until
you actually have something like a working code.

The code is working, because I am reading 40000 lines I am hopping that any
tick I can shave off will buy me a few seconds in the long run.

Then if it fails to meet the performance spec,
go looking for optimizations. And be sure to take
a stop watch with you to determine if a given
optimization in fact makes any appreciable change.

The specs are already met, loading the file.
But I want to load the file as fast as possible to make the user experience
even more enjoyable.

>do you think that using push_back is the fastest way I can fill my array?

I think that you need to worry about other things
before you worry about doing things the fastest
way you can.

Well, it does work now.
I can use the program as it is now, but the faster will make the user
experience more enjoyable.
Yes I could tell them to wait 30 seconds, but waiting 10 secs would be even
better.

Simon

[Jerry Coffin]

In article <60*************@mid.individual.net>, sp********@example.com
says...

[ ... ]

Well the files are text files loaded at run time, (once with some kind of
progress bar). The faster the file is loaded the better.

Currently it takes between 10 to 20 seconds on a 'normal/average' machine.

My guess is that roughly 99.999% of this is time to read the disk, and
what you do with the data after you read it won't make any noticeable
difference.

Ideally I would convert the file so it is loaded in my array as fast as
possible.
It is all about user experience.

The time to resize a vector is measured in microseconds -- far below
what a user can hope to detect.

Get it right. Then get it fast. Don't worry about
omtimizing on such little issues as this until
you actually have something like a working code.

The code is working, because I am reading 40000 lines I am hopping that any
tick I can shave off will buy me a few seconds in the long run.

What you're doing right now is about equivalent to somebody who's
grossly obese trying to lose weight by trimming his fingernails. You
need to look at the time to actually read the data, not the time taken
by the processor to store the data after it's been read. Chances are
good that maximum performance will require non-portable code, such as
memory mapping the file, or doing some of the reading in the background,
so you can start doing some processing before you've finished reading
the file.

--
Later,
Jerry.

The universe is a figment of its own imagination.

[Simon]

>

[ ... ]

>Well the files are text files loaded at run time, (once with some kind of
progress bar). The faster the file is loaded the better.

Currently it takes between 10 to 20 seconds on a 'normal/average'
machine.

My guess is that roughly 99.999% of this is time to read the disk, and
what you do with the data after you read it won't make any noticeable
difference.

I read the entire file first into memory, then I read each 'lines'

>Ideally I would convert the file so it is loaded in my array as fast as
possible.
It is all about user experience.

The time to resize a vector is measured in microseconds -- far below
what a user can hope to detect.

>The code is working, because I am reading 40000 lines I am hopping that
any
tick I can shave off will buy me a few seconds in the long run.

What you're doing right now is about equivalent to somebody who's
grossly obese trying to lose weight by trimming his fingernails.

Maybe, but when will I be able to calculate that I am going as fast as the
system can go?
When will fast enough be enough.

Because, if I get it down to 5 seconds, the user might want it down to 1
second and so on.

need to look at the time to actually read the data,

Less than half a second.
I use the file size, (fseek (hf , 0 , SEEK_END); + ftell)

And then I read the whole thing into a char *

Simon

[Matthias Buelow]

Jerry Coffin wrote:

Something here doesn't add up. It's almost impossible to say what since
you don't seem to have posted the actual code, but if you're able to
read the data from the disk in less than half a second, you definitely
should NOT need anywhere close to "10 to 20 seconds" to break that up
into lines, store it in a vector, or _almost_ anything else you feel
like doing.

Perhaps he's reading a large file into memory, and then essentially
copies the stuff, exhausting the physical RAM and the machine starts
swapping madly. Hard to tell without any further details, of course.