Skip to content

Tracking allocator #24

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.

Sign up for GitHub

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
mcdubhghlas merged 11 commits into from
May 21, 2026
Merged

Tracking allocator #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2dfc6ed
Testing allocator
VictorSohier May 18, 2026
e40d512
Address code rabbit comments.
VictorSohier May 21, 2026
9df9f6f
Add common types into definitions
JoltedJon May 18, 2026
a057d17
Move defs to own file and replace usages
JoltedJon May 18, 2026
c90ea8d
Migrate math to stdtypes
VictorSohier May 20, 2026
bee83d8
Move allocators to stdtypes
VictorSohier May 21, 2026
db81175
Add common types into definitions
VictorSohier May 21, 2026
79de9ac
Move defs to own file and replace usages
VictorSohier May 21, 2026
cf3b4d0
Add missing bit header
VictorSohier May 21, 2026
6b8e8b6
Testing allocator
VictorSohier May 21, 2026
130cda0
Merge branch 'master' into trackingAllocator
VictorSohier May 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions engine/native/core/memory/allocator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
module;

#include <source_location>

module core.memory.allocator;
import core.stdtypes;

Expand All @@ -10,6 +12,9 @@ namespace draco::memory
Slice *dst,
usize size,
usize align
#ifdef DEBUG
, std::source_location loc
#endif
)
{
return Error::NotImplemented;
Expand Down
49 changes: 48 additions & 1 deletion engine/native/core/memory/allocator.cppm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
module;

#include <source_location>

export module core.memory.allocator;
export import core.memory.slice;
export import core.stdtypes;
Expand All @@ -21,7 +23,17 @@ export namespace draco::memory
struct Allocator
{
AllocatorVTbl *vtbl;
void *allocatorData;
rawptr allocatorData;
inline Error alloc(
Slice *dst,
usize size,
usize align
#ifdef DEBUG
, std::source_location loc = std::source_location::current()
#endif
);
inline Error free(Slice block);
inline Error freeAll();
};

struct AllocatorVTbl
Expand All @@ -31,6 +43,9 @@ export namespace draco::memory
Slice *dst,
usize size,
usize align
#ifdef DEBUG
, std::source_location loc
#endif
);
using FreeFn = Error (*)(Allocator alloc, Slice block);
using FreeAllFn = Error (*)(Allocator alloc);
Expand All @@ -44,11 +59,43 @@ export namespace draco::memory
Slice *dst,
usize size,
usize align
#ifdef DEBUG
, std::source_location loc
#endif
);

Error nilFree(Allocator alloc, Slice block);

Error nilFreeAll(Allocator alloc);

void asAllocatorVoid(Allocator *dst, rawptr alloc, AllocatorVTbl *vtbl);
inline Error Allocator::alloc(
Slice *dst,
usize size,
usize align
#ifdef DEBUG
, std::source_location loc
#endif
)
{
return vtbl->alloc(
*this,
dst,
size,
align
#ifdef DEBUG
, loc
#endif
);
}

inline Error Allocator::free(Slice block)
{
return vtbl->free(*this, block);
}

inline Error Allocator::freeAll()
{
return vtbl->freeAll(*this);
}
}
14 changes: 13 additions & 1 deletion engine/native/core/memory/bumpAllocator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module;
#include <bit>
#include <cassert>
#include <cstring>
#include <source_location>

module core.memory.bumpAllocator;
import core.stdtypes;
Expand Down Expand Up @@ -40,7 +41,15 @@ namespace draco::memory::bump
}
}

Error alloc(Allocator alloc, Slice *dst, usize size, usize align)
Error alloc(
Allocator alloc,
Slice *dst,
usize size,
usize align
#ifdef DEBUG
, std::source_location loc
#endif
)
{
Error err;
BumpAllocator *allocData = (BumpAllocator *)alloc.allocatorData;
Expand Down Expand Up @@ -77,6 +86,9 @@ namespace draco::memory::bump
&newBlock,
std::max(allocData->minAllocRequest, reqSize),
std::max(alignof(Node), align)
#ifdef DEBUG
, loc
#endif
);
if (err != Error::Okay)
{
Expand Down
11 changes: 10 additions & 1 deletion engine/native/core/memory/bumpAllocator.cppm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module;

#include <cassert>
#include <source_location>

export module core.memory.bumpAllocator;
export import core.memory.allocator;
Expand Down Expand Up @@ -35,7 +36,15 @@ export namespace draco::memory

void deinit(BumpAllocator *alloc);

Error alloc(Allocator alloc, Slice *dst, usize size, usize align);
Error alloc(
Allocator alloc,
Slice *dst,
usize size,
usize align
#ifdef DEBUG
, std::source_location loc
#endif
);

Error freeAll(Allocator alloc);

Expand Down
40 changes: 20 additions & 20 deletions engine/native/core/memory/bumpAllocator.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include <doctest.h>

import core.memory;
import core.stdtypes;

TEST_CASE("Bump allocator provides distinct pointers on allocation")
{
Expand All @@ -13,12 +14,12 @@ TEST_CASE("Bump allocator provides distinct pointers on allocation")
Error err;
bump::init(&bumpAlloc, page::pageAllocator);
bump::asAllocator(&alloc, &bumpAlloc);
err = alloc.vtbl->alloc(alloc, &a, sizeof(int), alignof(int));
err = alloc.alloc(&a, sizeof(int), alignof(int));
REQUIRE(err == Error::Okay);
REQUIRE(bumpAlloc.first != nullptr);
// REQUIRE(bumpAlloc.first->next == nullptr);
REQUIRE(bumpAlloc.first->size > bumpAlloc.allocated);
err = alloc.vtbl->alloc(alloc, &b, sizeof(int), alignof(int));
err = alloc.alloc(&b, sizeof(int), alignof(int));
REQUIRE(err == Error::Okay);
REQUIRE((((ptrdiff_t)b.data) - ((ptrdiff_t)a.data)) >= sizeof(int));
bump::deinit(&bumpAlloc);
Expand All @@ -34,44 +35,43 @@ TEST_CASE("Bump allocator aligns pointers correctly")
Error err;
bump::init(&bumpAlloc, page::pageAllocator);
bump::asAllocator(&alloc, &bumpAlloc);
err = alloc.vtbl->alloc(alloc, &a, sizeof(uint8_t), 2);
err = alloc.alloc(&a, sizeof(draco::u8), 2);
REQUIRE(err == Error::Okay);
REQUIRE(bumpAlloc.first != nullptr);
// REQUIRE(bumpAlloc.first->next == nullptr);
REQUIRE(bumpAlloc.first->size > bumpAlloc.allocated);
err = alloc.vtbl->alloc(alloc, &b, sizeof(uint8_t), 4);
err = alloc.alloc(&b, sizeof(draco::u8), 4);
REQUIRE(err == Error::Okay);
REQUIRE((((uintptr_t)a.data) & (2 - 1)) == 0);
REQUIRE((((uintptr_t)b.data) & (4 - 1)) == 0);
REQUIRE((((draco::uintptr)a.data) & (2 - 1)) == 0);
REQUIRE((((draco::uintptr)b.data) & (4 - 1)) == 0);
bump::deinit(&bumpAlloc);
}

TEST_CASE("Bump allocator data is well packed")
{
struct Foo
{
uint32_t a;
uint64_t b;
draco::u32 a;
draco::u64 b;
};
using namespace draco::memory;
bump::BumpAllocator bumpAlloc;
Allocator alloc;
Slice aSlice;
Slice bSlice;
uint32_t *a;
uint64_t *b;
draco::u32 *a;
draco::u64 *b;
Error err;
bump::init(&bumpAlloc, page::pageAllocator);
bump::asAllocator(&alloc, &bumpAlloc);
err = alloc.vtbl->alloc(alloc, &aSlice, sizeof(uint32_t), alignof(Foo));
err = alloc.alloc(&aSlice, sizeof(draco::u32), alignof(Foo));
REQUIRE(err == Error::Okay);
REQUIRE(bumpAlloc.first != nullptr);
// REQUIRE(bumpAlloc.first->next == nullptr);
REQUIRE(bumpAlloc.first->size > bumpAlloc.allocated);
err = alloc.vtbl->alloc(alloc, &bSlice, sizeof(uint64_t), alignof(Foo));
err = alloc.alloc(&bSlice, sizeof(draco::u64), alignof(Foo));
REQUIRE(err == Error::Okay);
a = (uint32_t*)aSlice.data;
b = (uint64_t*)bSlice.data;
a = (draco::u32*)aSlice.data;
b = (draco::u64*)bSlice.data;
*a = 69;
*b = 420;
REQUIRE(((Foo*)bumpAlloc.first->data)->a == 69);
Expand All @@ -89,9 +89,9 @@ TEST_CASE("Bump allocator allocates second page when available")
Error err;
bump::init(&bumpAlloc, page::pageAllocator);
bump::asAllocator(&alloc, &bumpAlloc);
err = alloc.vtbl->alloc(alloc, &aSlice, 8192, 1);
err = alloc.alloc(&aSlice, 8192, 1);
REQUIRE(err == Error::Okay);
err = alloc.vtbl->alloc(alloc, &bSlice, 8192, 1);
err = alloc.alloc(&bSlice, 8192, 1);
REQUIRE(err == Error::Okay);
REQUIRE(aSlice.data != bSlice.data);
REQUIRE(aSlice.size == 8192);
Expand All @@ -111,11 +111,11 @@ TEST_CASE("Exact alignment")
Error err;
bump::init(&bumpAlloc, page::pageAllocator);
bump::asAllocator(&alloc, &bumpAlloc);
err = alloc.vtbl->alloc(alloc, &aSlice, 5, 1);
err = alloc.alloc(&aSlice, 5, 1);
REQUIRE(err == Error::Okay);
err = alloc.vtbl->alloc(alloc, &bSlice, 8, 8);
err = alloc.alloc(&bSlice, 8, 8);
REQUIRE(err == Error::Okay);
err = alloc.vtbl->alloc(alloc, &cSlice, 4, 4);
err = alloc.alloc(&cSlice, 4, 4);
REQUIRE(err == Error::Okay);
REQUIRE(bumpAlloc.allocated == 20);
bump::deinit(&bumpAlloc);
Expand Down
14 changes: 9 additions & 5 deletions engine/native/core/memory/fixedAllocator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@ module;

#include <bit>
#include <cassert>
#include <cstdint>
#include <source_location>

module core.memory.fixedAllocator;
import core.stdtypes;

namespace draco::memory::fixed
{
void init(FixedAllocator *alloc, Slice block)
{
alloc->buffer = (uint8_t *)block.data;
alloc->buffer = (u8 *)block.data;
alloc->size = block.size;
alloc->allocated = false;
}
Expand All @@ -20,21 +21,24 @@ namespace draco::memory::fixed
Slice *dst,
usize size,
usize align
#ifdef DEBUG
, std::source_location loc
#endif
)
{
FixedAllocator *allocData = (FixedAllocator*)alloc.allocatorData;
usize alignMask = align - 1;
usize alignedSize = allocData->size - (
(align - (((uintptr_t)allocData->buffer) & alignMask))
(align - (((uintptr)allocData->buffer) & alignMask))
& alignMask
);
assert(std::popcount(align) == 1);
if (allocData->allocated | (alignedSize < size))
{
return Error::OutOfMemory;
}
dst->data = (void *)(
((uintptr_t)&(allocData->buffer[alignMask])) & ~alignMask
dst->data = (rawptr)(
((uintptr)&(allocData->buffer[alignMask])) & ~alignMask
);
dst->size = alignedSize;
allocData->allocated = true;
Expand Down
14 changes: 11 additions & 3 deletions engine/native/core/memory/fixedAllocator.cppm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module;

#include <cassert>
#include <cstdint>
#include <source_location>

export module core.memory.fixedAllocator;
export import core.memory.allocator;
Expand All @@ -14,14 +14,22 @@ export namespace draco::memory
{
struct FixedAllocator
{
uint8_t *buffer;
u8 *buffer;
usize size;
bool allocated;
};

void init(FixedAllocator *alloc, Slice block);

Error alloc(Allocator alloc, Slice *dst, usize size, usize align);
Error alloc(
Allocator alloc,
Slice *dst,
usize size,
usize align
#ifdef DEBUG
, std::source_location loc
#endif
);

Error freeAll(Allocator alloc);

Expand Down
5 changes: 3 additions & 2 deletions engine/native/core/memory/fixedAllocator.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,17 @@
#include <doctest.h>

import core.memory;
import core.stdtypes;

TEST_CASE("Fixed allocator is correctly aligned")
{
using namespace draco::memory;
alignas(8) uint8_t buffer[1024];
alignas(8) draco::u8 buffer[1024];
fixed::FixedAllocator fixedAlloc;
Allocator alloc;
Slice block;
fixed::init(&fixedAlloc, { .data = buffer, .size = 1024 });
fixed::asAllocator(&alloc, &fixedAlloc);
alloc.vtbl->alloc(alloc, &block, 512, 16);
alloc.alloc(&block, 512, 16);
REQUIRE((((uintptr_t)block.data) & 15) == 0);
Comment on lines +16 to 17
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Assert allocation success before checking alignment.

Line 15 ignores the alloc.alloc(...) result. If allocation fails, Line 16 may assert on an invalid block.

Proposed fix 
-	alloc.alloc(&block, 512, 16);
+	Error err = alloc.alloc(&block, 512, 16);
+	REQUIRE(err == Error::Okay);
 	REQUIRE((((uintptr_t)block.data) & 15) == 0);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@engine/native/core/memory/fixedAllocator.test.cpp` around lines 15 - 16, The
test calls alloc.alloc(&block, 512, 16) but never verifies it succeeded before
inspecting block.data; change the test to assert the allocation returned success
(e.g., REQUIRE on the boolean/result of alloc.alloc) before the alignment check,
so verify alloc.alloc(...) is true and only then perform
REQUIRE((((uintptr_t)block.data) & 15) == 0); reference alloc.alloc and block in
the fixedAllocator.test.cpp test.

}
Loading