DevShiftTeam / AppShift-MemoryPool

A very fast cross-platform memory pool mechanism for C++ built using a data-oriented approach (3 to 24 times faster than regular new or delete, depending on operating system & compiler)
Apache License 2.0
214 stars 25 forks source link

Can it be used in a production environment #22

Closed romanholidaypancakes closed 2 years ago

romanholidaypancakes commented 2 years ago

I don't see any test suites,it makes me worry about stability.

romanholidaypancakes commented 2 years ago

https://github.com/DevShiftTeam/AppShift-MemoryPool/blob/dea09308bc8e56aa4bdfa23140d1b3bfd30ee8d6/MemoryPool.cpp#L46

I found that it still uses malloc to allocate memory instead of allocating on large memory blocks.


void* bigPool = malloc(1024*1024*4);
MemoryPool pool(bigPool);
pool.maolloc(size);  

pool.maolloc(size); should be allocated in bigPool instead of using malloc/new

LessComplexity commented 2 years ago

@inaryart Thanks for the comment :)

https://github.com/DevShiftTeam/AppShift-MemoryPool/blob/dea09308bc8e56aa4bdfa23140d1b3bfd30ee8d6/MemoryPool.cpp#L46

I found that it still uses malloc to allocate memory instead of allocating on large memory blocks.

This line of code is the one that is creating a new large block to allocate on, it is not part of the allocate function itself, unless there is not enough space on the current block then it will create a new one and use it for allocations. Strictly speaking, the malloc function is only executed when the block space runs out and a new large block needs to be created.

void* bigPool = malloc(1024*1024*4);
MemoryPool pool(bigPool);
pool.maolloc(size);  

pool.maolloc(size); should be allocated in bigPool instead of using malloc/new

If you will go through the README.md file carefully you can see that the usage is as follows:

MemoryPool pool(1024 * 1024 * 4);
pool.allocate(size);

The allocate function will not call malloc, you may debug and see for yourself :) Also I think I understand your confusion here - you don't need to create your own block, you just pass the size to the pool and let it create the block for you. The memory pool makes sure for you that if the space on the block runs out, then it creates a new block for you.

Another good way to implement the same code is as follows, because I have an override on the new instruction:

MemoryPool pool(1024 * 1024 * 4);
new (&pool) MY_TYPE[SIZE];

When you use the new instruction this way, then it also won't use the malloc function.

Have an awesome day! 😃

LessComplexity commented 2 years ago

@inaryart

I don't see any test suites,it makes me worry about stability.

You can use it in production, tough you still need to be careful about allocation and deallocation order as it affects performance and memory usage because of the way that the pool is implemented. Tests have been done by me and other developers but I haven't uploaded the tests to the current release. In the next release I plan to add a variety of test suites, expand functionalities and add more to the tutorial on this pool on how to use it most effectively. You can view the development branch if you are curious 😃

romanholidaypancakes commented 2 years ago

@inaryart Thanks for the comment :)

https://github.com/DevShiftTeam/AppShift-MemoryPool/blob/dea09308bc8e56aa4bdfa23140d1b3bfd30ee8d6/MemoryPool.cpp#L46

I found that it still uses malloc to allocate memory instead of allocating on large memory blocks.

This line of code is the one that is creating a new large block to allocate on, it is not part of the allocate function itself, unless there is not enough space on the current block then it will create a new one and use it for allocations. Strictly speaking, the malloc function is only executed when the block space runs out and a new large block needs to be created.

void* bigPool = malloc(1024*1024*4);
MemoryPool pool(bigPool);
pool.maolloc(size);  

pool.maolloc(size); should be allocated in bigPool instead of using malloc/new

If you will go through the README.md file carefully you can see that the usage is as follows:

MemoryPool pool(1024 * 1024 * 4);
pool.allocate(size);

The allocate function will not call malloc, you may debug and see for yourself :) Also I think I understand your confusion here - you don't need to create your own block, you just pass the size to the pool and let it create the block for you. The memory pool makes sure for you that if the space on the block runs out, then it creates a new block for you.

Another good way to implement the same code is as follows, because I have an override on the new instruction:

MemoryPool pool(1024 * 1024 * 4);
new (&pool) MY_TYPE[SIZE];

When you use the new instruction this way, then it also won't use the malloc function.

Have an awesome day! 😃

Well, I see, I didn't look at the code in detail before. https://github.com/DevShiftTeam/AppShift-MemoryPool/blob/dea09308bc8e56aa4bdfa23140d1b3bfd30ee8d6/MemoryPool.cpp#L70

romanholidaypancakes commented 2 years ago

@inaryart

I don't see any test suites,it makes me worry about stability.

You can use it in production, tough you still need to be careful about allocation and deallocation order as it affects performance and memory usage because of the way that the pool is implemented. Tests have been done by me and other developers but I haven't uploaded the tests to the current release. In the next release I plan to add a variety of test suites, expand functionalities and add more to the tutorial on this pool on how to use it most effectively. You can view the development branch if you are curious 😃

ok, I'll try the current main branch version first.