patterncppMinor
Vector backed by memory pages
Viewed 0 times
memorypagesvectorbacked
Problem
The purpose of this class is to wrap a
```
#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);
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...
This can be very problematic, unless T has virtual destructor, you should place some
Similar problem is in
Minor note about
The above should be written with
void reset() {
//delete objects
for(T* obj : data) {
obj->~T(); // <<< will not call the appropriate destructor if not virtualThis 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 virtualtemplate<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.