Jump to content

  • Log In with Google      Sign In   
  • Create Account

Jeffreyp

Member Since 24 Jan 2011
Offline Last Active Jul 07 2013 04:39 AM

Posts I've Made

In Topic: Attempting to create a pool allocator

08 March 2013 - 09:00 AM

Thanks for all the responses !

A lot of useful information : )


In Topic: Attempting to create a pool allocator

07 March 2013 - 02:42 PM

Your use of class member variables should take note of being a member variable by either:

 

this->at
or naming it

m_at; (m_ noting it is a 'member' variable)



You may also want to return a void* instead of char* since you are technically return a pointer to not a char but a random object (could be a GameObject*, int*, Texture*)

 

Thanks for your response.

 

Your use of class member variables should take note of being a member variable by either:
 
this->at
or naming it
m_at; (m_ noting it is a 'member' variable)

 

Isn't naming class members with m_ a bad habit? The reason I'm saying this is because a few months ago I had an internship and

I showed some c++ source code and they said that naming c++ class members with m_ is a bad habit from the C language. Just curious : )

 

You may also want to return a void* instead of char* since you are technically return a pointer to not a char but a random object (could be a GameObject*, int*, Texture*)

 

Oké, any other reasons I should use void* instead of char *? I saw both when I was googling around so I'm not really sure.

 

Thanks !


In Topic: Attempting to create a pool allocator

07 March 2013 - 09:28 AM

You should not have the -1 in the comparison. For example, if you allocate 1000 bytes and then attempt to obtain 1000 bytes, it should succeed. But as it is you can only obtain 999 bytes.

More importantly, you should be returning the value of 'at' before adding 'amount' to it, or you will overrun the end of the buffer.

 

Oké, thanks for the explanation


In Topic: Attempting to create a pool allocator

07 March 2013 - 07:26 AM

If you're interested in creating a pool allocator (not a stack allocator, like Hodgman noted), I've written about that on my blog

 

Thanks I will check it out!


In Topic: Attempting to create a pool allocator

07 March 2013 - 07:25 AM

That's not a pool allocator, it's a stack allocator.

 

In the destructor you shouldn't be deleting both memory and at. at is inside memory (and at wasn't created with new), it's already deleted when you delete memory.

The atAmount variable is always zero. What's it for?
Instead of "atAmount + amount > size - 1", you should be using "at + amount > memory + size".
 
You should merge the allocate method into the constructor, so it's created like:
PoolAllocator allocator(1000);
At the moment, someone could call allocate twice, and your class would cause a memory leak. 


The allocator should probably be non-copyable, because at the moment if I write PoolAllocator other = allocator; your code will crash.
e.g.

class PoolAllocator
{
public:
...
private:
    PoolAllocator(const PoolAllocator&);//not implemented
    PoolAllocator& operator=(const PoolAllocator&);//prevents accidentally copying
...
};

 

If the free method does nothing, it shouldn't exist. At the moment, it will only fool your users into thinking that it's necessary, or that they are actually freeing memory.

 

The obtain function does not take alignment into account. If someone obtains a char and then an int, the int will be misaligned.

 

Thanks for the response!

Some really informative things in there.

 

 

That's not a pool allocator, it's a stack allocator.

 

Yes you're right. I read it up on wikipedia and understand it now.

 

 

The atAmount variable is always zero. What's it for?
Instead of "atAmount + amount > size - 1", you should be using "at + amount > memory + size".

 

atAmount actually does the same as

at + amount > memory + size

 

But shouldn't be

at + amount > memory + size - 1

 

Because if

at + amount == memory + size

it would crash at

at+= amount
right?

 

 

If the free method does nothing, it shouldn't exist. At the moment, it will only fool your users into thinking that it's necessary, or that they are actually freeing memory.

 

Actually I don't know how I should implement this method. Can you give me some tips?

 

Thanks.


PARTNERS