HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Vector backed by memory pages

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
memorypagesvectorbacked

Problem

The purpose of this class is to wrap a std::vector in a class so that never a new object is added. We don't allocate a new object on the stack but we trying to fit it on the current memory page, or create a new page for it.

```
#include

template
class PagedVector {

public :
PagedVector(int initialCapacity, float load_factor = 1.5f):
pageSize(static_cast(load_factorinitialCapacitysizeof(T))),
current_byte_offset(0),
current_page(0)
{
data.reserve(initialCapacity);

//initialise one page:
char* ptr = static_cast(malloc(pageSize));
pages.push_back(std::make_pair(ptr, pageSize));
}

template
U* create() {
char* memoryPtr = getNextAdress(sizeof(U), std::alignment_of::value);
U* objectPtr = new (memoryPtr) U();
data.push_back(objectPtr);
return objectPtr;
}

~PagedVector() {
//delete objects
for(T* obj : data) {
obj->~T();
}

//free memory
for(auto& pair : pages) {
free(pair.first);
}
}

void reset() {
//delete objects
for(T* obj : data) {
obj->~T();
}
data.clear();

//keep memory but reset page count
current_page = 0;
current_byte_offset = 0;
}

const std::vector& getData() const {
return data;
}

private :
std::vector data;
std::vector > pages;

size_t pageSize;
size_t current_byte_offset;
int current_page;

char* getNextAdress(size_t size, size_t align) {
//get next available adress for type with given size and aligment

char* current_page_adress = pages[current_page].first + current_byte_offset;
char* end_page_adress = pages[current_page].first + pages.back().second;

size_t remainder_align = reinterpret_cast(current_page_adress) % align;
size_t extra_alignment_padding = remainder_align==0 ? 0 : (align - remainder_align);

Solution

I usually focus on the design and this review won't be different. You seem to have created some custom universal allocator, that will place all the (different) objects in continuous space (a page) or create new page if needed. The whole space can be released all at once, but no individual objects. So...

void reset() {
    //delete objects
    for(T* obj : data) {
        obj->~T(); // <<< will not call the appropriate destructor if not virtual


This can be very problematic, unless T has virtual destructor, you should place some static_assert:

template
class  PagedVector {
    static_assert(std::has_virtual_destructor::value,
        "Objects without virtual destructor cannot be used");


Similar problem is in create allowing any class, should allow derived only:

template
U* create() {
    static_assert(std::is_same::value || std::is_base_of::value,
        "Objects not derived from T are not allowed");


Minor note about = delete:

//non-copiable:
PagedVector(PagedVector&);
PagedVector& operator=(PagedVector&);


The above should be written with const and = delete to avoid using it inside the class:

//non-copiable:
PagedVector(const PagedVector&) = delete;
PagedVector& operator=(const PagedVector&) = delete;

Code Snippets

void reset() {
    //delete objects
    for(T* obj : data) {
        obj->~T(); // <<< will not call the appropriate destructor if not virtual
template<class T>
class  PagedVector {
    static_assert(std::has_virtual_destructor<T>::value,
        "Objects without virtual destructor cannot be used");
template<class U>
U* create() {
    static_assert(std::is_same<U,T>::value || std::is_base_of<T,U>::value,
        "Objects not derived from T are not allowed");
//non-copiable:
PagedVector(PagedVector&);
PagedVector& operator=(PagedVector&);
//non-copiable:
PagedVector(const PagedVector&) = delete;
PagedVector& operator=(const PagedVector&) = delete;

Context

StackExchange Code Review Q#67209, answer score: 6

Revisions (0)

No revisions yet.