1

Currently I'm looking into this C++ source code. I'm not a C++ dev.

void SomeClass::SomeMethod() const 
{ 
    vector<Thing> things(count); 

    ...
    //Elements are added or replaced in things but no deallocation of things here
}

SomeMethod is called a lot of times. Could anyone confirm that there is no leak and things is allocated only once, reference would be appreciated.

Kimi
  • 13,621
  • 9
  • 55
  • 84
  • 1
    It's allocated once per call to the function. Without more code you cannot get a correct answer on whether this usage is correct or can be replaced by something less costly in memory allocation and release. – Steve Townsend Dec 04 '10 at 15:51

4 Answers4

5

The vector is created everytime you enter the function and destroyed (destroying all objects and freeing all the memory) when it leaves scope (when the function ends). There's no leak, but there's a lot of allocations and deallocations if you call that function frequently.

You have 2 solutions to avoid that:

  • (preferred) Make this vector a class field (with attribute mutable in order to allow it to be changed by a const method),
  • Make this vector a static variable.
Kos
  • 70,399
  • 25
  • 169
  • 233
  • 1
    +1 but that said, once set member or static you'll have to add some managing code to limit reallocation, first by using the vector's reserve() function, next by trying to get the maximum size en reserve it from start and after that make sure the vector is cleared before new use. – Klaim Dec 04 '10 at 15:47
  • 1
    Correct, but not really necessary imho, since clear() doesn't free any memory on like, all implementations I've seen. So if the vector load will be similar on all function calls, then only the first call will be expensive and all the next calls won't have any reallocations since the vector will already have the required memory pool allocated. – Kos Dec 04 '10 at 15:50
  • You'd need to see a lot more code before knowing that singleton allocation of `things` is appropriate, wouldn't you? Doesn't the fact that this function is `const` suggest that the original coder knows to some degree what they were doing? – Steve Townsend Dec 04 '10 at 15:50
  • I'm not planning to modify code, I was surprised that this method was not leaking :) – Kimi Dec 04 '10 at 15:53
  • Thanks, Steve; I'm trying to answer the OP's question as well as I can under the given circumstances - by confirming that no, there are no leaks and yes, there is a separate allocation (and reallocations) every call. Since this vector is local, it's not part of the object's state, so making it a static variable wouldn't violate the design of that class, but just improve the memory usage. – Kos Dec 04 '10 at 15:56
  • Yes agreed. +1 for offering options that might apply. – Steve Townsend Dec 04 '10 at 18:10
2

Provided Thing is correctly implemented insofar as its destructor and other member functions (especially copy constructor because that's used in vector housekeeping) correctly handle all memory for its data members, what this will do is create a new vector<Thing> on each call to the function.

The resulting local variable things gets correctly freed, including destruction of every Thing member, when the variable goes out of scope (i.e. on function exit).

It's impossible to be more definitive without seeing all the code in the method and in Thing, but this usage is on the face of it correct.

Steve Townsend
  • 53,498
  • 9
  • 91
  • 140
2

"things" is a local auto variable. Another post has a answer for this: Where are the local, global, static, auto, register, extern, const, volatile variables are stored?

Community
  • 1
  • 1
Huang F. Lei
  • 1,835
  • 15
  • 23
1

That lays localy in that function. When it goes out of scope, it will delokate the memory by itself; all of the STL containers do.

Ørjan Tufte
  • 63
  • 4