Am I leaking with malloc?

[copthesaint]

I am actually using this in c++ but it is all c code some I thought I would post it here. Basically I want to know if I screwed anything up, or there is an even faster way to do this (execution speed), but not assembly


Single Pointer:

c:

T* m_Var = (T*) malloc(1 * sizeof(T)); new (m_Var) T(); m_Var->~T(); free(m_Var);

Single1D Array:

c:

int size = 5; T* m_Var = (T*) malloc(size * sizeof(T)); T* tempVar = m_Var; for (int i = 0; i < size; i++){         new (*tempVar) T();         tempVar++; } tempVar = m_Var; for (int i = 0; i < size; i++){         tempVar->~T();         tempVar++; } free(m_Var);

Array of Pointers

c:

int size = 5; T** m_Var = (T**) malloc(size * sizeof(T*)); T* tempVar = *m_Var; for (int i = 0; i < size; i++){         tempVar = (T*) malloc(1 * sizeof (T));         new (tempVar) T();         tempVar++; } tempVar = *m_Var; for (int i = 0; i < size; i++){         tempVar->~T();         free(m_Var);         tempVar++; } free(m_Var);

[Insectoid]

You actually don't need to use malloc() at all. You can just use new and new[]. new with no parameters will automatically allocate all the data it needs. I may be wrong here, but in your array example, this can actually cause issues if you call delete() on any element of the array, since it deallocates the memory. Delete also automatically calls the destructor on the object you want to delete. Your code is valid as far as I can tell (I didn't dig very deep, bear in mind), though generally you should not need to use malloc or free any time you use new or delete. malloc/free are C, while new/delete are C++.

[copthesaint]

@Insectoid Yes, you don't want to use new and delete with Malloc and free, that is not what I'm doing, I'm using placement new simply to call the constructor, no new memory is being created, If there is a faster way to call the constructor via early binding in C, let me know

Continued: I am well aware of new and delete, how to use them ect, however I am making a game, and yes new and delete are easier, but malloc, free, realloc calloc are faster. For all intents and purposes, I would like to only focus on the fastest way, within the language of C/C++, Not the easiest necessarly to code.

[Insectoid]

If you're going for the fastest code, you're doing it all wrong, and I'm not sure you understand what early binding is. Any time (more or less) that you use malloc or free is late binding.

code:

//This is early binding class Foo {}; Foo a; //Note that this does indeed call the constructor. //This is late binding Foo *b; b = (Foo*) malloc (sizeof (Foo)); new (*b) Foo; //This is also late binding Foo *c; c = new Foo;

malloc, calloc, realloc, free, new and delete are all constant-time operations, so there is no reason to use one over the other except that it's improper C++ to use malloc/free.

[crossley7]

Something you will want to keep in mind is that malloc and free, even if they are faster, the difference is not significant for it to matter on any scale and new and delete are type safe commands. In this way, if you mess up, new and delete can catch it for you while malloc, free will not.

That, and it is generally bad practice to use C style commands when C++ created new commands that generally replace them. They were replaced for a reason and so I highly recommend using them. Code safety should be something you are alert to since it will help with proper coding practices.

[Dreadnought]

copthesaint wrote:

Array of Pointers

c:

int size = 5; T** m_Var = (T**) malloc(size * sizeof(T*)); T* tempVar = *m_Var; for (int i = 0; i < size; i++){         tempVar = (T*) malloc(1 * sizeof (T));         new (tempVar) T();         tempVar++; } free(m_Var);

I'm not certain, but I'm pretty sure this is not doing what you want it to do. You probably want tempVar to be of type T**.

[Insectoid]

No, I think tempVar is fine. He's got m_Var, a pointer to a pointer to T, and tempVar pointing to m_Var's pointer content. Although this works, it's really dumb, and both m_Var and tempVar should be type T*.

Another note: You should follow a consistent naming strategy. Either use camelCase, came_Case, or under_scores, but don't mix it up.

[copthesaint]

@crossley7 This isn't a bad practice, That is simply a matter of opinion. And I do use them (New and delete), however is this case, I need to make dynamic memory allocation faster, and creating memory on the fly can be a bottleneck in games so the extra time I save will be worth it in this case. As for safty, I know what to check for, I just want to know if my code will create any memory leaks. Simple as that, no one seems to care to answer that, probably because of lack of experience with malloc/free

@Insectoid I know the difference between late and early binding, I used the wrong word when I meant late, Thank you for clearing up something I had no issue with. I love feedback, however I do not appreciate all the "dumb" feedback you have given me, "you're doing it all wrong" and "it's really dumb" isn't really helping me. I need to have memory dynamically allocated and if you do not know how to help me accomplish this with malloc and free and/or you don't agree with the way I am doing this, then take your opinions somewhere else. Also I am using camelCase, in my file m_Var would be a member variable of a class. This comment on my naming convention, I HAVE TO USE, is irrelevant, since this is the way my professors want my code to be written.

@Dreadnought It's an array of pointers, so I dereference the first pointer in the array I can get every pointer from the simple pointer math. This has it's uses in polymorphism creating an array of pointers. all the memory is in the heap together so the next pointer will be mine .


Can we please skip the necessity of telling me to use new and delete and focus on Malloc and free? I want to make sure I am using it correctly. I will not be using new / delete in this specific part of my project.

[DemonWasp]

This sounds like a serious case of optimization-before-profiling, as taught by How To Waste Time 101. Write it the simple way, profile it, figure out what's slow and replace only that part. Remember that "working but slow" is always better than "fast but broken and incomplete".

You should ask your professor to change that naming strategy because it's a disaster.

As far as I can tell, you aren't generating any leaks with your single-instance or 1D-array examples.

The allocation is probably better stated as:

code:

for ( T* tmp = m_Var + size - 1; tmp >= m_Var; --tmp) {     new (*tmp) T(); }

Similarly, destroy-loops can be:

code:

for ( T* tmp = m_Var + size - 1; tmp >= mVar; --tmp ) {     tmp->~T();     free(tmp); // only applies for array-of-pointers as written above }

For the array-of-pointers approach you show is leaky (and wrong, etc), but only because you free m_Var, not tmpVar. Double-free is really bad (worse than leaking). If you plan to follow this approach, try to use macros to save yourself from typos.

The array-of-pointers approach is also pretty awful. As written, it allocates tiny little blocks of space all over (wherever the implementation decides to put them). You should try to allocate enough space for the array and all of its contents contiguously; doing so will lead to much better memory locality (and therefore cache coherence) and far, far better performance.

Don't forget: allocation can fail! Check the return code from malloc() or expect your program to do unexpected things and then explode every so often.

What are you doing that you assume allocation will be a serious overhead?

[2goto1]

To answer your first question: "am I leaking with malloc / did you screw anything up", I think that Demonwasp answered that part very well. To answer your second question, "is there an even faster way"...I'm not sure! But again, I agree with Demonwasp et al. since you're making something (a game), and you're not just answering a test question on the fastest way to allocate memory.

In general, at least 99% of the time, it's always better to optimize later. I would recommend that you read about optimizing from some more credible industry folks, in addition to our esteemed members, such as Martin Fowler, http://martinfowler.com/ieeeSoftware/yetOptimization.pdf. Surely there are some experienced, smart guys out there with opinions about optimizing that will be worth considering.

Before you commit hours of brain power optimizing, first make your game! Once it's made, then profile, like Demonwasp suggested. Otherwise you're getting ahead of yourself. You could do what you're doing now, which is to make all of these little programs that benchmark memory allocation approaches, or this that and the other thing, and find out that all of your approaches are 50% faster than the norm, but without making a game and then profiling the entire game, what does your effort mean in the greater context? Improving the .1% CPU time in your game by 50% won't yield much benefit. Your game will probably be graded on "does it work" more so than "is it running super fast / smoothly". Just my critical thinking 2 cents.

[copthesaint]

Sorry, But I'm going to double post, first post is everything that isn't related to the topic, second is.

DemonWasp @ Thu Nov 28, 2013 wrote:

This sounds like a serious case of optimization-before-profiling, as taught by How To Waste Time 101. Write it the simple way, profile it, figure out what's slow and replace only that part. Remember that "working but slow" is always better than "fast but broken and incomplete".
...
What are you doing that you assume allocation will be a serious overhead?

I already have a stable version of my game, However I now have time to waste and am optimizing my code so that it runs more efficiently with a combination of rewriting pooling classes, to entity component systems ect, right now it is very sluggish during certain times of execution and there are some silly memory leeks I need to fix, until then I don't consider it finished.

DemonWasp @ Thu Nov 28, 2013 wrote:

You should ask your professor to change that naming strategy because it's a disaster.

Again, irrelevant, already asked him, he made a decent enough argument, but his main point is one day you will have to work for a company and follow whatever naming convention weather you like it or not, and if you don't you won't be working there very much longer.

DemonWasp @ Thu Nov 28, 2013 wrote:

Don't forget: allocation can fail! Check the return code from malloc() or expect your program to do unexpected things and then explode every so often.

Yea I'm aware, I just didn't include with the example to save time reading over the code.

2goto1 @ Thu Nov 28, 2013 wrote:

To answer your first question: "am I leaking with malloc / did you screw anything up", I think that Demonwasp answered that part very well. To answer your second question, "is there an even faster way"...I'm not sure! But again, I agree with Demonwasp et al. since you're making something (a game), and you're not just answering a test question on the fastest way to allocate memory.

In general, at least 99% of the time, it's always better to optimize later. I would recommend that you read about optimizing from some more credible industry folks, in addition to our esteemed members, such as Martin Fowler, http://martinfowler.com/ieeeSoftware/yetOptimization.pdf. Surely there are some experienced, smart guys out there with opinions about optimizing that will be worth considering.

Before you commit hours of brain power optimizing, first make your game! Once it's made, then profile, like Demonwasp suggested. Otherwise you're getting ahead of yourself. You could do what you're doing now, which is to make all of these little programs that benchmark memory allocation approaches, or this that and the other thing, and find out that all of your approaches are 50% faster than the norm, but without making a game and then profiling the entire game, what does your effort mean in the greater context? Improving the .1% CPU time in your game by 50% won't yield much benefit. Your game will probably be graded on "does it work" more so than "is it running super fast / smoothly". Just my critical thinking 2 cents.

I think I have already answered this with demonwasp, I'm not wasting time in my opinion, and it is my time after all. But I defiantly understand your opinion. To me, I know I have a game/project that works, and I will get the marks for it, but for me, programming has never just been to get it done, I like to do more. To put it simply. By your logic, I should use someone else's software to render 3D instead of writing my own OpenGL 4.0 code or DirectX equivalent. But that's not me even though that could be a valid answer.

[2goto1]

Definitely if your game is done, totally worth figuring out how to optimize it. Have you tried profiling yet?

[copthesaint]

DemonWasp @ Thu Nov 28, 2013 wrote:

code:

for ( T* tmp = m_Var + size - 1; tmp >= mVar; --tmp ) {     tmp->~T();     free(tmp); // only applies for array-of-pointers as written above }

Is there any reason why you decremented instead of incremented?

DemonWasp @ Thu Nov 28, 2013 wrote:

For the array-of-pointers approach you show is leaky (and wrong, etc), but only because you free m_Var, not tmpVar. Double-free is really bad (worse than leaking). If you plan to follow this approach, try to use macros to save yourself from typos.

The array-of-pointers approach is also pretty awful. As written, it allocates tiny little blocks of space all over (wherever the implementation decides to put them). You should try to allocate enough space for the array and all of its contents contiguously; doing so will lead to much better memory locality (and therefore cache coherence) and far, far better performance.

Good catch, didn't notice my typo with m_Var, but my real code doesn't have that problem. I understand why this will cause fragmentation in my memory while executing, but how should I or how would you suggest creating an array of pointers?I'm not sure how I can allocate a block of memory for the array and all its pointers while still being able to call the array's individual pointers. Can you go further into explaining your answer?

[copthesaint]

2goto1 @ Thu Nov 28, 2013 wrote:

Definitely if your game is done, totally worth figuring out how to optimize it. Have you tried profiling yet?

No I haven't tried profiling yet, I wanted to try rewriting the obvious performance issue in my program and seeing how it improves performance. Later If there is still time, I'll try using a profiler with my program or write some simple code to see what parts of my code are taking the longest time to execute.

[2goto1]

If you didn't profile, how did you determine what your obvious performance issue was? I imagine some sort of ad-hoc profiling? "This is running slow, stick a console message before / after to evaluate the time difference" kinda thing?