Skip to main content
deleted 100 characters in body
Source Link
G. Sliepen
  • 61.7k
  • 3
  • 61
  • 152

Use C++17 aligned new or aligned_malloc()

_aligned_malloc() is a Windows-specific, non-portable function. Luckily, C++17 added the portable aligned_malloc() without an underscore. It also introduces a variant of new that allows aligned allocation. The latter would look like so:

#include <new>
...
MemoryBlock::MemoryBlock(size_t size, size_t alignment):
    p_rawMem((void *)new char[size, (std::align_val_t(alignmentalignemnt)]) char[size]),
    ...
{
}

assert() vs throw()

You use both in your code. If you throw exceptions, I would just use throw everywhere.

Unnecessary special case for p_remainingMemSize == 0

Why is this a special case in p_addCurrentMemLocation? Just adding delta to the current pointer is perfectly fine. A pointer that points just beyond the end of an array or an allocated memory region is still a valid pointer.

Consider avoiding the check for delta == 0

An if-statement is not free, especially not if the branch predictor cannot correctly predict the result of the condition. If delta is zero, then the rest of the code still works correctly, and it's just two additions and a subtraction.

Don't use floating point math

You should not need to use floating point math to calculate how much to advance p_currentMemLocationOffset inside getMemoryWith(). Depending on which processor your code is running on, converting to double and back to size_t can be an expensive operation. Furthermore, double has less precision than size_t on 64-bits machines, so the result might be wrong if very large memory blocks are used.

Instead of ceil(p_CurrentMemLocationOffset / (double)alignment), use the fact that integer division rounds down, and compensate for this by adding alignment - 1 first:

p_addCurrentMemLocation((((p_currentMemLocationOffset + alignment - 1) / alignment) * alignment) - p_currentMemLocationOffset);

Alternatively, use the modulo operation to determine how much to add:

p_addCurrentMemLocation((alignment - p_currentMemLocationOffset % alignment) % alignment);

The latter will be especially fast if the compiler can deduce that alignment is always a power of two.

char * vs. void *

It's probably easier to use char * internally for the pointers, since you are doing arithmetic on them. And if you do that, the only time you need to cast is when returning from getMemoryWith().

Use C++17 aligned new or aligned_malloc()

_aligned_malloc() is a Windows-specific, non-portable function. Luckily, C++17 added the portable aligned_malloc() without an underscore. It also introduces a variant of new that allows aligned allocation. The latter would look like so:

MemoryBlock::MemoryBlock(size_t size, size_t alignment):
    p_rawMem((void *)new char[size, std::align_val_t(alignment)]),
    ...
{
}

assert() vs throw()

You use both in your code. If you throw exceptions, I would just use throw everywhere.

Unnecessary special case for p_remainingMemSize == 0

Why is this a special case in p_addCurrentMemLocation? Just adding delta to the current pointer is perfectly fine. A pointer that points just beyond the end of an array or an allocated memory region is still a valid pointer.

Consider avoiding the check for delta == 0

An if-statement is not free, especially not if the branch predictor cannot correctly predict the result of the condition. If delta is zero, then the rest of the code still works correctly, and it's just two additions and a subtraction.

Don't use floating point math

You should not need to use floating point math to calculate how much to advance p_currentMemLocationOffset inside getMemoryWith(). Depending on which processor your code is running on, converting to double and back to size_t can be an expensive operation. Furthermore, double has less precision than size_t on 64-bits machines, so the result might be wrong if very large memory blocks are used.

Instead of ceil(p_CurrentMemLocationOffset / (double)alignment), use the fact that integer division rounds down, and compensate for this by adding alignment - 1 first:

p_addCurrentMemLocation((((p_currentMemLocationOffset + alignment - 1) / alignment) * alignment) - p_currentMemLocationOffset);

Alternatively, use the modulo operation to determine how much to add:

p_addCurrentMemLocation((alignment - p_currentMemLocationOffset % alignment) % alignment);

The latter will be especially fast if the compiler can deduce that alignment is always a power of two.

char * vs. void *

It's probably easier to use char * internally for the pointers, since you are doing arithmetic on them. And if you do that, the only time you need to cast is when returning from getMemoryWith().

Use C++17 aligned new or aligned_malloc()

_aligned_malloc() is a Windows-specific, non-portable function. Luckily, C++17 added the portable aligned_malloc() without an underscore. It also introduces a variant of new that allows aligned allocation. The latter would look like so:

#include <new>
...
MemoryBlock::MemoryBlock(size_t size, size_t alignment):
    p_rawMem((void *)new(std::align_val_t(alignemnt)) char[size]),
    ...
{
}

Unnecessary special case for p_remainingMemSize == 0

Why is this a special case in p_addCurrentMemLocation? Just adding delta to the current pointer is perfectly fine. A pointer that points just beyond the end of an array or an allocated memory region is still a valid pointer.

Consider avoiding the check for delta == 0

An if-statement is not free, especially not if the branch predictor cannot correctly predict the result of the condition. If delta is zero, then the rest of the code still works correctly, and it's just two additions and a subtraction.

Don't use floating point math

You should not need to use floating point math to calculate how much to advance p_currentMemLocationOffset inside getMemoryWith(). Depending on which processor your code is running on, converting to double and back to size_t can be an expensive operation. Furthermore, double has less precision than size_t on 64-bits machines, so the result might be wrong if very large memory blocks are used.

Instead of ceil(p_CurrentMemLocationOffset / (double)alignment), use the fact that integer division rounds down, and compensate for this by adding alignment - 1 first:

p_addCurrentMemLocation((((p_currentMemLocationOffset + alignment - 1) / alignment) * alignment) - p_currentMemLocationOffset);

Alternatively, use the modulo operation to determine how much to add:

p_addCurrentMemLocation((alignment - p_currentMemLocationOffset % alignment) % alignment);

The latter will be especially fast if the compiler can deduce that alignment is always a power of two.

char * vs. void *

It's probably easier to use char * internally for the pointers, since you are doing arithmetic on them. And if you do that, the only time you need to cast is when returning from getMemoryWith().

Source Link
G. Sliepen
  • 61.7k
  • 3
  • 61
  • 152

Use C++17 aligned new or aligned_malloc()

_aligned_malloc() is a Windows-specific, non-portable function. Luckily, C++17 added the portable aligned_malloc() without an underscore. It also introduces a variant of new that allows aligned allocation. The latter would look like so:

MemoryBlock::MemoryBlock(size_t size, size_t alignment):
    p_rawMem((void *)new char[size, std::align_val_t(alignment)]),
    ...
{
}

assert() vs throw()

You use both in your code. If you throw exceptions, I would just use throw everywhere.

Unnecessary special case for p_remainingMemSize == 0

Why is this a special case in p_addCurrentMemLocation? Just adding delta to the current pointer is perfectly fine. A pointer that points just beyond the end of an array or an allocated memory region is still a valid pointer.

Consider avoiding the check for delta == 0

An if-statement is not free, especially not if the branch predictor cannot correctly predict the result of the condition. If delta is zero, then the rest of the code still works correctly, and it's just two additions and a subtraction.

Don't use floating point math

You should not need to use floating point math to calculate how much to advance p_currentMemLocationOffset inside getMemoryWith(). Depending on which processor your code is running on, converting to double and back to size_t can be an expensive operation. Furthermore, double has less precision than size_t on 64-bits machines, so the result might be wrong if very large memory blocks are used.

Instead of ceil(p_CurrentMemLocationOffset / (double)alignment), use the fact that integer division rounds down, and compensate for this by adding alignment - 1 first:

p_addCurrentMemLocation((((p_currentMemLocationOffset + alignment - 1) / alignment) * alignment) - p_currentMemLocationOffset);

Alternatively, use the modulo operation to determine how much to add:

p_addCurrentMemLocation((alignment - p_currentMemLocationOffset % alignment) % alignment);

The latter will be especially fast if the compiler can deduce that alignment is always a power of two.

char * vs. void *

It's probably easier to use char * internally for the pointers, since you are doing arithmetic on them. And if you do that, the only time you need to cast is when returning from getMemoryWith().