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 2 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
4 changes: 4 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,6 +1,7 @@
module;

#include <cstddef>
#include <source_location>

module core.memory.allocator;

Expand All @@ -11,6 +12,9 @@ namespace draco::memory
Slice *dst,
size_t size,
size_t align
#ifdef DEBUG
, std::source_location loc
#endif
)
{
return Error::NotImplemented;
Expand Down
48 changes: 47 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,6 +1,6 @@
module;

#include <cstddef>
#include <source_location>

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

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

Error nilFree(Allocator alloc, Slice block);

Error nilFreeAll(Allocator alloc);

void asAllocatorVoid(Allocator *dst, void *alloc, AllocatorVTbl *vtbl);
inline Error Allocator::alloc(
Slice *dst,
size_t size,
size_t 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 @@ -5,6 +5,7 @@ module;
#include <cstddef>
#include <cstdint>
#include <cstring>
#include <source_location>

module core.memory.bumpAllocator;

Expand Down Expand Up @@ -40,7 +41,15 @@ namespace draco::memory::bump
}
}

Error alloc(Allocator alloc, Slice *dst, size_t size, size_t align)
Error alloc(
Allocator alloc,
Slice *dst,
size_t size,
size_t 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
Expand Up @@ -4,6 +4,7 @@ module;
#include <cstddef>
#include <cstdint>
#include <cstring>
#include <source_location>

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

void deinit(BumpAllocator *alloc);

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

Error freeAll(Allocator alloc);

Expand Down
23 changes: 11 additions & 12 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 @@ -13,12 +13,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,12 +34,11 @@ 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(uint8_t), 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(uint8_t), 4);
REQUIRE(err == Error::Okay);
REQUIRE((((uintptr_t)a.data) & (2 - 1)) == 0);
REQUIRE((((uintptr_t)b.data) & (4 - 1)) == 0);
Expand All @@ -63,12 +62,12 @@ TEST_CASE("Bump allocator data is well packed")
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(uint32_t), 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(uint64_t), alignof(Foo));
REQUIRE(err == Error::Okay);
a = (uint32_t*)aSlice.data;
b = (uint64_t*)bSlice.data;
Expand All @@ -89,9 +88,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 +110,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
4 changes: 4 additions & 0 deletions engine/native/core/memory/fixedAllocator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module;
#include <cassert>
#include <cstddef>
#include <cstdint>
#include <source_location>

module core.memory.fixedAllocator;

Expand All @@ -21,6 +22,9 @@ namespace draco::memory::fixed
Slice *dst,
size_t size,
size_t align
#ifdef DEBUG
, std::source_location loc
#endif
)
{
FixedAllocator *allocData = (FixedAllocator*)alloc.allocatorData;
Expand Down
11 changes: 10 additions & 1 deletion engine/native/core/memory/fixedAllocator.cppm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module;
#include <cassert>
#include <cstddef>
#include <cstdint>
#include <source_location>

export module core.memory.fixedAllocator;
export import core.memory.allocator;
Expand All @@ -21,7 +22,15 @@ export namespace draco::memory

void init(FixedAllocator *alloc, Slice block);

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

Error freeAll(Allocator alloc);

Expand Down
2 changes: 1 addition & 1 deletion engine/native/core/memory/fixedAllocator.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ TEST_CASE("Fixed allocator is correctly aligned")
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.

}
10 changes: 10 additions & 0 deletions engine/native/core/memory/pageAllocator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module;

#include <cstddef>
#include <source_location>
#ifdef __unix__
#include <sys/mman.h>
#include <unistd.h>
Expand All @@ -20,6 +21,9 @@ namespace draco::memory::page
Slice *dst,
size_t size,
size_t align
#ifdef DEBUG
, std::source_location loc
#endif
)
{
int pageSizeSub1 = getpagesize() - 1;
Expand Down Expand Up @@ -57,6 +61,9 @@ namespace draco::memory::page
Slice *dst,
size_t size,
size_t align
#ifdef DEBUG
, std::source_location loc
#endif
)
{
SYSTEM_INFO sysinfo;
Expand Down Expand Up @@ -89,6 +96,9 @@ namespace draco::memory::page
Slice *dst,
size_t size,
size_t align
#ifdef DEBUG
, std::source_location loc
#endif
)
{
size_t pageSize = GetLargePageMinimum();
Expand Down
11 changes: 10 additions & 1 deletion engine/native/core/memory/pageAllocator.cppm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module;

#include <cstddef>
#include <source_location>

export module core.memory.pageAllocator;
export import core.memory.allocator;
Expand All @@ -10,7 +11,15 @@ export namespace draco::memory
{
namespace page
{
Error alloc(Allocator alloc, Slice *dst, size_t size, size_t align);
Error alloc(
Allocator alloc,
Slice *dst,
size_t size,
size_t align
#ifdef DEBUG
, std::source_location loc
#endif
);

Error free(Allocator alloc, Slice block);

Expand Down
1 change: 1 addition & 0 deletions engine/native/core/memory/root.cppm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ export import core.memory.allocator;
export import core.memory.fixedAllocator;
export import core.memory.pageAllocator;
export import core.memory.bumpAllocator;
export import core.memory.trackingAllocator;
Loading