tags:

views:

553

answers:

8

Hi all,

I just started out with C and have very little knowledge about performance issues with malloc() and free(). My question is this: if I were to call malloc() followed by free() inside a while loop that loops for, say, 20 iterations, would it run slower compared to calling free() outside the loop?

I am actually using the first method to allocate memory to a buffer, read a variable-length string from a file, perform some string operations, and then clear the buffer after every iteration. If my method results in a lot of overhead then I'd like to ask for a better way for me to achieve the same results.

Sorry for my bad English.

update 0: thanks so much for all the help, looks like I still have much to learn! ;D

Thanks and regards, K

+3  A: 

If you know the maximum length of the buffer - or can set a reasonable maximum - then you could use the same buffer for every iteration. Otherwise what you are doing should be fine.

Justin Ethier
A: 

It depends on the implementation of malloc and free.

The best way to respond to your question is to build a benchmark...

jag
+10  A: 

Definitely slower. (But remember you need to balance the number of malloc and free otherwise you get a memory leak.)

If the length varies, you can use realloc to expand the buffer size.

void* v = malloc(1024);
size_t bufsize = 1024;

while(cond) {
   size_t reqbufsize = get_length();
   if (reqbufsize > bufsize) {
      bufsize = reqbufsize * 2;
      v = realloc(v, bufsize);
   }
   // you may shrink it also.

   do_something_with_buffer(v);
}

free(v);
KennyTM
+1 -- but don't you think `bufsize = reqbufsize * 2;` is a little drastic? :P
Billy ONeal
Maybe. MSVC2k8 implementation of `std::vector` grows its buffer by 150% when reallocating.
Janusz Lenar
@BillyONeal: That's actually a pretty common way to do things.
Dinah
Buffers usually increase proportionally to their old size because it gets better amortized asymptotic running time. If you add a one slot per realloc and insert n elements, there are 1 + 2 + ... + (n - 1) + n = O(n^2) copies required. If, however, you double the buffer's size, inserting n elements only costs O(1) copies. You can find a more rigorous explanation by looking at the hash table Wikipedia article: http://en.wikipedia.org/wiki/Hash_table#Resizing_by_copying_all_entries
Mike Koval
Growing the buffer by a factor is a common pattern when you're trying to amortize allocation across many insertions. In this example, it seems `reqbufsize` is actually the required buffer size, so there's no need to scale it at all. Just `realloc` to `reqbufsize`.
Adrian McCarthy
If the realloc fails, you've leaked it. You need a temp pointer to realloc to, if it succeeds, copy that to the original pointer, else free the original pointer and abort with error message.
Arthur Kalliokoski
@Arthur. Yes. But you've a bigger problem than leaking if `realloc` fails :).
KennyTM
+1  A: 

It depends on what you need the buffer for.

Do you really need to clear it after every iterations or maybe a \0 char at the end would suffice to mark the end of a string? After all that's what the various str library calls use.

If you really need to clear it you can use bzero(). Certainly malloc'ing and free'ing at every iteration is a waste of resources, since you can happily re-use the buffer.

A different problem would arise if you were to parallelize the for loop, i.e. to have several concurrent threads using it.

Simple, real-life example: using a bucket to carry water. Suppose you need to do several trip with that bucket: would it make sense to pick it up, use it, put it down, pick it up again, use it, etc...? You can re-use the bucket as many times as possible. If, on the other hand, the bucket needs to be used by you and more people, either you organize access to the bucket or need more buckets.

Last suggestion: do not worry about performances now. They say that early optimization is the root of all evil, and you'll soon understand why.

Firstly, understand the problem: write code that can be thrown away. Experiment. Secondly, test it. Make sure it does what you need. Thirdly, optimize it. Make the loop run ten thousand times and measure how long it takes. Then move the malloc outside, and measure again (use the shell command time if under UNIX). Fourthly, rewrite it because your first experiment will most likely be a mess of patches of try-retry-not working code.

Rinse, repeat.

ps: have fun in the mean time. It's supposed to be interesting, not frustrating.

lorenzog
A: 

Generally anything that can be moved outside of a loop should be. Why repeat the same action when you can just do it once?

Justin Ethier is right, allocate a buffer that will comfortably fit the largest string and reuse that.

yosser
+6  A: 

For 20 iterations, you shouldn't be worrying about the performance of malloc/free.

Even for vastly more (a couple orders of magnitude), you shouldn't start thinking about optimizations until you profile the code and understand what is slow.

Finally, if you're going to free the buffer, there is no need to clear it first. Even if you were to move the malloc/free outside the loop (using a maximum buffer as suggested by Justin), you shouldn't need to explicitly clear the buffer.

Steve Madsen
+5  A: 

You can't call free outside the loop if you are calling malloc inside:

char * buffer;
for (int i = 0; i < num_files; i++) {
    buffer = malloc(proper_length(i));
    // do some things with buffer
}
free(buffer);

You will have malloc'ed num_files times, but only freed once - you leaked the memory from all but the last!

There are two main choices - malloc before the loop (or just use an array) if you know a size that will work for everything, or use realloc:

char * buffer = NULL;
for (int i = 0; i < num_files; i++) {
    buffer = realloc(proper_length(i));
    // do some things with buffer
}
free(buffer);
Jefromi
A: 

Process it better. Have some pseudo code:

#define BLOCK_SIZE 1024 // or about the bigger size of your strings.

char *buffer = (char *) malloc(BLOCK_SIZE) 

for(int i=0; i<20; i++)
{
   while (more bytes left to read)
   {
    read full string or BLOCK_SIZE bytes at max // most calls work this way
    proces bytes in buffer
   }
}

free(buffer);
RdM