Arena(bump) allocator

[spiral anvil]

This is what I have currently

#include <cstddef>
#include <iostream>
#include <memory>

class Region {
public:
  Region(size_t size_) : size(size_), memory(new char[size_]), offset(0) {}
  static Region *createRegion(size_t size) { return new Region(size); }
  void *allocate(size_t alignment, size_t offset_) {

    if (offset + offset_ > size) {
      grow(size);
    }
    void *currentPtr = memory + offset;

    void *alignedPtr = std::align(alignment, offset_, currentPtr, size);

    if (!alignedPtr) {
      throw std::runtime_error("Out of memory or alignment issues!");
    }

    // add padding based on alignment to the offset
    std::size_t padding = static_cast<char *>(alignedPtr) - (memory + offset);
    offset += padding + offset_;

    return memory + offset;
  }
  void grow(size_t offset_) {
    size_t newSize = std::max(size * 2, size + offset_);
    char *NewMemory = new char[newSize];

    std::copy(memory, memory + offset, NewMemory);
    delete[] memory;
    memory = NewMemory;
    size = newSize;
  }
  ~Region() { delete[] memory; }

private:
  char *memory;
  size_t offset;
  size_t size;
};

struct Node {
  int a;
  Node(int a_) : a(a_) {}
};

int main() {
  Region *r1 = Region::createRegion(sizeof(Node) * 3);

  Node *n1 = new (r1->allocate(sizeof(Node), alignof(Node))) Node(1);
  Node *n2 = new (r1->allocate(sizeof(Node), alignof(Node))) Node(2);
  Node *n3 = new (r1->allocate(sizeof(Node), alignof(Node))) Node(3);
  Node *n4 = new (r1->allocate(sizeof(Node), alignof(Node))) Node(4);
  // is it properly constructed & allocated?
  std::cout << n1->a;
  std::cout << n2->a;
  std::cout << n3->a;
  std::cout << n4->a;
  delete r1;
  return 0;
}

Everything works fine until I need to grow the arena, and as a consequence, the whole memory block gets messed up(I assume). Getting rid of n4 makes it correct again.
What can I improve on this arena allocator?

[cyan tundra]

When the 4th one is reallocated, the memory addresses change, so n1-3 are now invalidated.

You would need to create new pointers or use offsets from the beginning of the arena region to keep it stable.

[rapid storm]

I'm genuinely too lazy to provide you with a proper review considering how much of a mess this is

[spiral anvil]

thanks

[rapid storm]

I'm only swinging by because you pinged me and I have 5 min of free time right this instant

dx already mentioned the core issue

and you not keeping the "allocated" memory stable for the people already using it suggests you aren't clear on how an allocator is used, or that you don't understand how new[]/delete[] work

also I'm half certain you mixed up sizeof/alignof

your allocate function takes alignment first, and size second

which is not how you used it

there's also an argument for kicking out char and using unsigned char or std::byte instead, but that's minor compared to fixing your argument order, and providing memory stability

also random remark that you might want to just have a templated overload for allocate that takes the type directly, and forwards the right size/alignment to your current allocate

[spiral anvil]

Instead of invalidating the pointers I could just create a new arena in the region and return the pointer from there. I can just traverse the linked list when I want to free the whole region.

[rapid storm]

if I understand your description properly, that's what the other screenshot you shared the other day did

[spiral anvil]

struct Arena {
  Arena(std::size_t alignment_, std::size_t size_)
      : alignment(alignment_), size(size_), memory(new std::byte[size_]), offset(0) {}
  template <typename T> void *allocate() {
    return allocate(alignof(T), sizeof(T));
  }
  void *allocate(size_t alignment, size_t offset_) {
    if (offset + offset_ > size) {
      // create new arena link it to linked list
      Arena*newA = new Arena(alignment, std::max(offset + offset_, static_cast<std::size_t>(1024)));
      next = newA;
      return newA->allocate(alignment, offset_);
    }
    void *currentPtr = memory + offset;

    void *alignedPtr = std::align(alignment, offset_, currentPtr, size);

    if (!alignedPtr) {
      throw std::runtime_error("Out of memory or alignment issues!");
    }

    // add padding based on alignment to the offset
    std::size_t padding =
        static_cast<std::byte *>(alignedPtr) - (memory + offset);
    offset += padding + offset_;

    return memory + offset;
  }
  ~Arena() { delete[] memory; }
  std::size_t alignment;
  Arena *next = nullptr;
  std::byte *memory;
  std::size_t offset;
  std::size_t size;
};

class Region {
public:
  Region(size_t size_) : size(size_) {}
  static Region *createRegion(size_t size) { return new Region(size); }
  template <typename T> void *allocate() {
    return allocate(alignof(T), sizeof(T));
  }
  void *allocate(size_t alignment, size_t offset_) {
    if(!head) {
      head = new Arena(alignment, size);
    }
    return head->allocate(alignment, offset_);
  }

  ~Region() {
    while (head) {
      auto *current = head;
      head = head->next;
      delete current;
    }
  }

private:
  // unsigned char or byte
  std::size_t size;
  Arena *head = nullptr;
};

struct Node {
  int a;
  Node(int a_) : a(a_) {}
};

i ended up with something like this but valgrind still gives me errors

[rapid storm]

imo this doesn't fit into "code review"

a quick look at your code and there are issues that stand out to me, but depending on what your user code is and what valgrind complains about it's unclear if those are/aren't the issue at hand

I'd recommend opening a help thread with a minimal repro