Skip to content

Conversation

Raelr
Copy link
Contributor

@Raelr Raelr commented Aug 24, 2025

Description

...

The PR has been...

  • provided a reasonable name that is not just the branch name (e.g "Added Vulkan render delegate")
  • linked to its related issue
  • assigned a reviewer from the team
  • labelled appropriately

The code has been...

  • made mergable and free of conflicts in relation to master (according to GitHub)
  • tested in a packaged state using the package targets
  • pulled to the reviewer's machine and reasonably tested

@Raelr Raelr requested a review from jonjondev August 24, 2025 22:23
@Raelr Raelr self-assigned this Aug 24, 2025
@Raelr Raelr added enhancement New feature or request utils Relating to the engine's utils labels Aug 24, 2025
@Raelr Raelr changed the title Tlsf alloc Added TLSF Allocator for dyamic heap allocation Aug 24, 2025
@Raelr Raelr force-pushed the tlsf-alloc branch 2 times, most recently from f112f5e to 12b07c4 Compare September 1, 2025 08:54
Copy link
Member

@jonjondev jonjondev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonably bulletproof - local run was fine. Just the few comments is all, but good to merge.

/**
* A shorthand macro for making deallocations easier to manage.
*/
#define FREE(ptr) Deallocate((void**) &ptr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming should be a bit more explicit for the global namespace

* @param header the current header
* @return true if the previous block is free, otherwise false
*/
bool PrevBlockIsFree(BlockHeader* header);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functions missing const-correctness

* bytes
* @return a pointer to the memory block
*/
void* Allocate(const T& size);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check if this is compatible with stl allocation conventions

BlockHeader* GetHeader(FreeBlockNode* node);

/**
* @brief Returns the header of a data pointer. Assumes that the data is pointing to the start
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brief can be dropped

void* TlsfAllocator<T>::Allocate(const T& size)
{
T requiredSize = sizeof(BlockHeader) + size + sizeof(BlockFooter);
if (!data || capacity == 0 || size == 0 || requiredSize < size ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be helpful to spit out an error in each of these cases with more info

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request utils Relating to the engine's utils
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants