tags:

views:

249

answers:

5

Is this good practice? Or should I just replace the code block between { and } with a function? It could be reusable (I admit) but my only motivation for doing this is to deallocate colsum since it's huge and not required so that I can free the allocated memory.

 vector<double> C;
 {
  vector<double> colsum;
  A.col_sum(colsum);
  C = At*colsum;
 }
 doSomething(C);
+13  A: 

Using brackets to scope automatic variables is fine in my book, but generally if you find yourself doing it a lot, especially multiple times in the same function, your function is probably doing too many different things and should be broken up.

bshields
+1: for the suggestion of breaking it up anyway - makes for more reusable / readable code :-)
Jon Cage
@bshileds: The main purpose as stated in the question is to force the deallocation of the vector. Is that what you meant (seems like it from your comments)?
Jacob
@Jacob I understand your intention and I'm saying it's a valid way to do it, cleaner than using the `swap` function as has been suggested by other answers because it makes use of RAII. It has it's place, especially for wrapping code using a `Mutex` object or something similar. BUT you just have to be careful because by using brackets you're acknowledging that you have this extra set of variables that you want to keep separate and use in their own scope to do some intermediate processing. You better have a good reason to not make that block its own function because it sure smells like one.
bshields
+5  A: 

The data for vectors is always dynamically allocated. Only the bookkeeping data is stored on the stack. Even if it weren't, stack memory allocation is essentially free. Deallocating from the stack is just changing the value of a register on most architectures.

EDIT

Regarding the dynamic deallocation, it will have to be deallocated at one point or another (particularly the end of the function). You're not actually losing anything by leaving the memory allocated until you want to allocate more and there isn't enough. Is the precise timing of when that deallocation occurs really something to be concerned about before you actually run into some problem?

/EDIT

But what's the point? It really seems like you're prematurely concerning yourself with optimization.

If you want to refactor your code, do it for the sake of clarity, not performance.

Cogwheel - Matthew Orlando
Not really. When `colsum` goes out of scope, it will deallocate the memory associated with it.
Jacob
The question is a little imprecise, but clearly the concern is the dynamically allocated memory and not the stack memory used. Depending on the size of the vector this could be a very valid heap memory usage concern, I don't think you can automatically write it off as pre-mature optimization.
bshields
Leaving the memory allocated until the function ends won't really hurt anything. If you find that you actually run into low memory situations, then your optimization is mature. jMerliN makes a good point as well. I'm not sure there's even any guarantee that the deallocation would occur at the closing brace. It may wait until the end of the function anyway.
Cogwheel - Matthew Orlando
There are ways of deallocating the vector memory without going through that hassle (i.e. `std::vector<double>().swap(colsum);`) I have written an answer to that effect.
David Rodríguez - dribeas
No the scoped behavior is well-defined. The variable goes out of scope and the object is destroyed, the memory will be deallocated.
bshields
Works for me :)
Cogwheel - Matthew Orlando
@bshields: (Brooke?) Thanks! Is there anything in the standard to that effect?
Jacob
+3  A: 

Vector doesn't store memory on the stack. Only the vector object itself is stored there, which is not very large. Scoping it like this does force a destruction which will free the memory it has allocated.

Also, I'm not sure that it's specified in the ISO that an implementation must pop a subscope variable off of the stack.

jMerliN
+3  A: 

As other have pointed out, the vector memory is not allocated in the stack. If you want to free that memory early on, the common idiom is:

vector<double> C;
vector<double> colsum;
A.col_sum(colsum);
C = At*colsum;
    std::vector<double>().swap(colsum);
doSomething(C);

That will create a temporary vector and swap the contents with your large vector. At the end of the instruction the temporary will be destroyed and the memory released. You will be left with an empty vector.

Note that colsum.resize(0) and colsum.clear() don't need to free the available memory, and in many cases they won't assuming that if the vector grew to that size before, chances are that it will do it again.

David Rodríguez - dribeas
But is this better than using the braces to limit the scope? While the swap trick is well known, it's kind of a roundabout way to express what you're trying to do.
Adrian McCarthy
To clear the vector I would use the swap trick as that is a common idiom, but I agree that the scope has its advantages. The first of them is that the identifier is free to be reused, and the second one is that it will work for types where there is no `swap` equivalent (scoped locks for example)`{ lock(mutex); .... }`
David Rodríguez - dribeas
A: 

If the inner code will be reused elsewhere, separate it into a function. If the inner code is called frequently (like in a loop), then it probably needs to be refactored so that the vector isn't being constantly created and destroyed in a loop. Otherwise, I don't think it is bad practice to do what you've suggested.

5ound