-
Notifications
You must be signed in to change notification settings - Fork 14
Add basic allocators #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
29839cd
Allocator infrastructure
VictorSohier 70bad5f
Primitive allocator types
VictorSohier 74d592c
Bump allocator with tests
VictorSohier 35f2b85
Fix: Account for spillover
VictorSohier 48b6bc6
Code rabbit fixes
VictorSohier 7911606
Fix a bug with potential under-allocation
VictorSohier b4c02d8
Initialize next pointer to null
VictorSohier d72c697
Fix signature change
VictorSohier a9c63bf
Deal with Windows' bullshit
VictorSohier 54056ac
Fix alignment bug
VictorSohier 8432f36
Better guarantees against poor alignment
VictorSohier 5df8a12
Notes for the clanker
VictorSohier ff79662
BumpAllocator alignment fix
VictorSohier 84b2e28
Move implementation to cpp files
VictorSohier 3e930ed
Fix windows returns
VictorSohier c21c596
Because windows has to be special
VictorSohier 5d2003f
Avoid potential null pointer dereference without additional branch.
VictorSohier 45bd49b
Failure for illegal addresses
VictorSohier fbb9a2c
Remove extraneous forward declaration
VictorSohier b3c1d24
Fix the size check again. This has no null dereference, it checks aga…
VictorSohier File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| add_modules_library(definitions PIC) | ||
| add_modules_library(math) | ||
| add_modules_library(memory) | ||
| target_link_libraries(math PUBLIC definitions) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| export module core; | ||
| export import core.defs; | ||
| export import core.math; | ||
| export import core.memory; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| module; | ||
|
|
||
| #include <cstddef> | ||
|
|
||
| module core.memory.allocator; | ||
|
|
||
| namespace draco::memory | ||
| { | ||
| Error nilAlloc( | ||
| Allocator alloc, | ||
| Slice *dst, | ||
| size_t size, | ||
| size_t align | ||
| ) | ||
| { | ||
| return Error::NotImplemented; | ||
| } | ||
|
|
||
| Error nilFree(Allocator alloc, Slice block) | ||
| { | ||
| return Error::NotImplemented; | ||
| } | ||
|
|
||
| Error nilFreeAll(Allocator alloc) | ||
| { | ||
| return Error::NotImplemented; | ||
| } | ||
|
|
||
| void asAllocatorVoid(Allocator *dst, void *alloc, AllocatorVTbl *vtbl) | ||
| { | ||
| dst->allocatorData = (void*)alloc; | ||
| dst->vtbl = vtbl; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| module; | ||
|
|
||
| #include <cstddef> | ||
|
|
||
| export module core.memory.allocator; | ||
| export import core.memory.slice; | ||
|
|
||
| export namespace draco::memory | ||
| { | ||
| enum class Error | ||
| { | ||
| Okay, | ||
| OutOfMemory, | ||
| NotImplemented, | ||
| IllegalAddressRange, | ||
| Other, // This one shouldn't be needed. If you see it returned, make a | ||
| // new error. | ||
| }; | ||
|
|
||
| struct AllocatorVTbl; | ||
|
|
||
| struct Allocator | ||
| { | ||
| AllocatorVTbl *vtbl; | ||
| void *allocatorData; | ||
| }; | ||
|
|
||
| struct AllocatorVTbl | ||
| { | ||
| using AllocFn = Error (*)( | ||
| Allocator alloc, | ||
| Slice *dst, | ||
| size_t size, | ||
| size_t align | ||
| ); | ||
| using FreeFn = Error (*)(Allocator alloc, Slice block); | ||
| using FreeAllFn = Error (*)(Allocator alloc); | ||
| AllocFn alloc; | ||
| FreeFn free; | ||
| FreeAllFn freeAll; | ||
| }; | ||
|
|
||
| Error nilAlloc( | ||
| Allocator alloc, | ||
| Slice *dst, | ||
| size_t size, | ||
| size_t align | ||
| ); | ||
|
|
||
| Error nilFree(Allocator alloc, Slice block); | ||
|
|
||
| Error nilFreeAll(Allocator alloc); | ||
|
|
||
| void asAllocatorVoid(Allocator *dst, void *alloc, AllocatorVTbl *vtbl); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| module; | ||
|
|
||
| #include <algorithm> | ||
| #include <cassert> | ||
| #include <cstddef> | ||
| #include <cstdint> | ||
| #include <cstring> | ||
|
|
||
| module core.memory.bumpAllocator; | ||
|
|
||
| namespace draco::memory::bump | ||
| { | ||
| void init( | ||
| BumpAllocator *alloc, | ||
| Allocator baseAlloc, | ||
| // one page by default on unix-like systems | ||
| size_t minAllocRequest | ||
| ) | ||
| { | ||
| memset(alloc, 0, sizeof(BumpAllocator)); | ||
| alloc->base = baseAlloc; | ||
| alloc->minAllocRequest = minAllocRequest; | ||
| } | ||
|
|
||
| void deinit(BumpAllocator *alloc) | ||
| { | ||
| Node *lastNode; | ||
| Node *node = alloc->first; | ||
| while (node != nullptr) | ||
| { | ||
| lastNode = node; | ||
| node = node->next; | ||
| alloc->base.vtbl->free( | ||
| alloc->base, | ||
| { | ||
| .data = (void*)lastNode, | ||
| .size = lastNode->size + sizeof(Node), | ||
| } | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| Error alloc(Allocator alloc, Slice *dst, size_t size, size_t align) | ||
| { | ||
| Error err; | ||
| BumpAllocator *allocData = (BumpAllocator *)alloc.allocatorData; | ||
| uintptr_t alignMask = align - 1; | ||
| Node **lastNode; | ||
| Node **node = &(allocData->first); | ||
| size_t pos = allocData->allocated; | ||
| size_t oldPos = pos; | ||
| size_t reqSize = size; | ||
| size_t spillover = 0; | ||
| Slice newBlock; | ||
| uintptr_t currentPtr; | ||
| assert(std::popcount(align) == 1); | ||
| lastNode = node; | ||
| while (((*node) != nullptr) & (pos > 0)) | ||
| { | ||
| oldPos = pos; | ||
| pos -= std::min((*node)->size, pos); | ||
| lastNode = node; | ||
| node = &((*node)->next); | ||
| } | ||
| assert(pos == 0); // fraudulent mark provided | ||
| currentPtr = ((uintptr_t)(*lastNode)) + sizeof(Node) + oldPos; | ||
| reqSize = size + ((align - (currentPtr & alignMask)) & alignMask); | ||
| if (!(*lastNode) || (reqSize > ((*lastNode)->size - oldPos))) | ||
| { | ||
| if (*lastNode) | ||
| { | ||
| spillover = ((*lastNode)->size - oldPos); | ||
| } | ||
| reqSize = (sizeof(Node) + size + alignMask) & ~alignMask; | ||
| err = allocData->base.vtbl->alloc( | ||
| allocData->base, | ||
| &newBlock, | ||
| std::max(allocData->minAllocRequest, reqSize), | ||
| std::max(alignof(Node), align) | ||
| ); | ||
| if (err != Error::Okay) | ||
| { | ||
| return Error::OutOfMemory; | ||
| } | ||
| (*node) = (Node *)newBlock.data; | ||
| (*node)->next = nullptr; | ||
| (*node)->size = newBlock.size - sizeof(Node); | ||
| pos = 0; | ||
| lastNode = node; | ||
| oldPos = 0; | ||
| } | ||
| currentPtr = ((uintptr_t)&((*lastNode)->data[oldPos])); | ||
| reqSize = size + ((align - (currentPtr & alignMask)) & alignMask); | ||
| currentPtr = (currentPtr + alignMask) & ~alignMask; | ||
| allocData->allocated += reqSize + spillover; | ||
|
VictorSohier marked this conversation as resolved.
|
||
| dst->data = (void*)currentPtr; | ||
| dst->size = size; | ||
| return Error::Okay; | ||
| } | ||
|
|
||
| Error freeAll(Allocator alloc) | ||
| { | ||
| BumpAllocator *allocData = (BumpAllocator *)alloc.allocatorData; | ||
| allocData->allocated = 0; | ||
| return Error::Okay; | ||
| } | ||
|
|
||
| size_t saveMark(BumpAllocator *self) | ||
| { | ||
| return self->allocated; | ||
| } | ||
|
|
||
| void resumeMark(BumpAllocator *self, size_t mark) | ||
| { | ||
| self->allocated = mark; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| module; | ||
|
|
||
| #include <cassert> | ||
| #include <cstddef> | ||
| #include <cstdint> | ||
| #include <cstring> | ||
|
|
||
| export module core.memory.bumpAllocator; | ||
| export import core.memory.allocator; | ||
| export import core.memory.slice; | ||
|
|
||
| export namespace draco::memory | ||
| { | ||
| namespace bump | ||
| { | ||
| struct Node | ||
| { | ||
| Node *next; | ||
| size_t size; | ||
| uint8_t data[]; | ||
| }; | ||
|
|
||
| struct BumpAllocator | ||
| { | ||
| Allocator base; | ||
| Node *first; | ||
| size_t minAllocRequest; | ||
| size_t allocated; | ||
| }; | ||
|
|
||
| void init( | ||
| BumpAllocator *alloc, | ||
| Allocator baseAlloc, | ||
| // one page by default on unix-like systems | ||
| size_t minAllocRequest = (1 << 12) | ||
| ); | ||
|
|
||
| void deinit(BumpAllocator *alloc); | ||
|
|
||
| Error alloc(Allocator alloc, Slice *dst, size_t size, size_t align); | ||
|
|
||
| Error freeAll(Allocator alloc); | ||
|
|
||
| AllocatorVTbl bumpAllocatorVtbl = { | ||
| .alloc = alloc, | ||
| .free = nilFree, | ||
| .freeAll = freeAll, | ||
| }; | ||
|
|
||
| size_t saveMark(BumpAllocator *self); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| void resumeMark(BumpAllocator *self, size_t mark); | ||
|
|
||
| inline void asAllocator(Allocator *dst, BumpAllocator *alloc) | ||
| { | ||
| asAllocatorVoid(dst, (void*)alloc, &bumpAllocatorVtbl); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: Redot-Engine/DraconicEngine
Length of output: 593
Add missing
<bit>header forstd::popcountusage.std::popcountat line 56 requires the<bit>header. Relying on transitive includes is unreliable across different toolchains and compiler configurations.Suggested fix
`#include` <algorithm> +#include <bit> `#include` <cassert> `#include` <cstddef>📝 Committable suggestion
🤖 Prompt for AI Agents