Skip to content

Commit 0d93567

Browse files
committed
Revised some PR review comments, pending bug related to constructors not being called when freeing list
1 parent e3981d9 commit 0d93567

File tree

3 files changed

+35
-34
lines changed

3 files changed

+35
-34
lines changed

include/nbl/core/alloc/AddressAllocatorBase.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,7 @@ namespace core
2525
public:
2626
_NBL_DECLARE_ADDRESS_ALLOCATOR_TYPEDEFS(_size_type);
2727

28-
AddressAllocatorBase() :
29-
reservedSpace(nullptr), addressOffset(invalid_address), alignOffset(invalid_address),
30-
maxRequestableAlignment(invalid_address), combinedOffset(invalid_address) {}
31-
32-
28+
AddressAllocatorBase() { invalidate(); }
3329

3430
inline _size_type max_alignment() const noexcept
3531
{

include/nbl/core/alloc/aligned_allocator_adaptor.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,19 @@ template <class Alloc, size_t overAlign=_NBL_DEFAULT_ALIGNMENT(typename std::all
1818
class NBL_FORCE_EBO aligned_allocator_adaptor : public Alloc
1919
{
2020
public:
21-
typedef std::allocator_traits<Alloc> traits;
21+
using traits = std::allocator_traits<Alloc>;
2222

2323
template< class U > struct rebind
2424
{
25-
typedef aligned_allocator_adaptor<typename Alloc::template rebind<U>::other,overAlign> other;
25+
using other = aligned_allocator_adaptor<typename Alloc::template rebind<U>::other,overAlign>;
2626
};
2727

2828
constexpr static size_t m_type_align = alignof(typename traits::value_type);
2929
constexpr static size_t m_align = overAlign>m_type_align ? overAlign:m_type_align;
3030
constexpr static size_t m_align_minus_1 = m_align-1u;
3131

32-
typedef typename traits::template rebind_alloc<uint8_t> ubyte_alloc;
33-
typedef typename ubyte_alloc::pointer ubyte_ptr;
32+
using ubyte_alloc = typename traits::template rebind_alloc<uint8_t>;
33+
using ubyte_ptr = typename ubyte_alloc::pointer;
3434

3535
private:
3636
// better and faster than boost for stateful allocators (no construct/destruct of rebound allocator on every alloc/dealloc)

include/nbl/core/containers/DoublyLinkedList.h

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,11 @@ struct alignas(void*) SDoublyLinkedNode
6464
}
6565
};
6666

67-
template<typename Value>
67+
template<typename Value, class allocator = core::allocator<SDoublyLinkedNode<Value>> >
6868
class DoublyLinkedList
6969
{
7070
public:
71+
using allocator_t = allocator;
7172
using address_allocator_t = PoolAddressAllocator<uint32_t>;
7273
using node_t = SDoublyLinkedNode<Value>;
7374
using value_t = Value;
@@ -121,7 +122,7 @@ class DoublyLinkedList
121122
{
122123
disposeAll();
123124

124-
m_addressAllocator = std::unique_ptr<address_allocator_t>(new address_allocator_t(m_reservedSpace, 0u, 0u, 1u, m_cap, 1u));
125+
m_addressAllocator = address_allocator_t(m_reservedSpace, 0u, 0u, 1u, m_cap, 1u);
125126
m_back = invalid_iterator;
126127
m_begin = invalid_iterator;
127128

@@ -186,9 +187,9 @@ class DoublyLinkedList
186187
// Must at least make list grow
187188
if (newCapacity <= m_cap)
188189
return false;
189-
// Same as code found in ContiguousMemoryLinkedListBase to create aligned space
190+
// Have to consider allocating enough space for list AND state of the address allocator
190191
const auto firstPart = core::alignUp(address_allocator_t::reserved_size(1u, newCapacity, 1u), alignof(node_t));
191-
void* newReservedSpace = _NBL_ALIGNED_MALLOC(firstPart + newCapacity * sizeof(node_t), alignof(node_t));
192+
void* newReservedSpace = reinterpret_cast<void*>(m_allocator.allocate(firstPart + newCapacity * sizeof(node_t)));
192193

193194
// Malloc failed, not possible to grow
194195
if (!newReservedSpace)
@@ -198,9 +199,9 @@ class DoublyLinkedList
198199
// Copy memory over to new buffer
199200
memcpy(newArray, m_array, m_cap * sizeof(node_t));
200201
// Create new address allocator from old one
201-
m_addressAllocator = std::make_unique<address_allocator_t>(newCapacity, std::move(*(m_addressAllocator)), newReservedSpace);
202+
m_addressAllocator = address_allocator_t(newCapacity, std::move(m_addressAllocator), newReservedSpace);
202203
// After address allocator creation we can free the old buffer
203-
_NBL_ALIGNED_FREE(m_reservedSpace);
204+
m_allocator.deallocate(reinterpret_cast<node_t*>(m_reservedSpace));
204205
m_cap = newCapacity;
205206
m_array = newArray;
206207
m_reservedSpace = newReservedSpace;
@@ -210,13 +211,15 @@ class DoublyLinkedList
210211
}
211212

212213
//Constructor, capacity determines the amount of allocated space
213-
DoublyLinkedList(const uint32_t capacity, disposal_func_t&& dispose_f = disposal_func_t()) : m_dispose_f(std::move(dispose_f))
214+
DoublyLinkedList(const uint32_t capacity, disposal_func_t&& dispose_f = disposal_func_t(), const allocator_t& _allocator = allocator_t())
215+
: m_dispose_f(std::move(dispose_f)), m_allocator(_allocator)
214216
{
217+
// Have to consider allocating enough space for list AND state of the address allocator
215218
const auto firstPart = core::alignUp(address_allocator_t::reserved_size(1u, capacity, 1u), alignof(node_t));
216-
m_reservedSpace = _NBL_ALIGNED_MALLOC(firstPart + capacity * sizeof(node_t), alignof(node_t));
219+
m_reservedSpace = reinterpret_cast<void*>(m_allocator.allocate(firstPart + capacity * sizeof(node_t)));
217220
m_array = reinterpret_cast<node_t*>(reinterpret_cast<uint8_t*>(m_reservedSpace) + firstPart);
218221

219-
m_addressAllocator = std::make_unique<address_allocator_t>(m_reservedSpace, 0u, 0u, 1u, capacity, 1u);
222+
m_addressAllocator = address_allocator_t(m_reservedSpace, 0u, 0u, 1u, capacity, 1u);
220223
m_cap = capacity;
221224
m_back = invalid_iterator;
222225
m_begin = invalid_iterator;
@@ -230,35 +233,36 @@ class DoublyLinkedList
230233

231234
DoublyLinkedList& operator=(DoublyLinkedList&& other)
232235
{
236+
m_allocator = std::move(other.m_allocator);
233237
m_addressAllocator = std::move(other.m_addressAllocator);
234-
m_reservedSpace = other.m_reservedSpace;
235-
m_array = other.m_array;
236-
m_dispose_f = std::move(other.m_dispose_f);
237-
m_cap = other.m_cap;
238-
m_back = other.m_back;
238+
m_reservedSpace = std::move(other.m_reservedSpace);
239+
m_array = std::move(other.m_array);
240+
m_dispose_f = std::move(std::move(other.m_dispose_f));
241+
m_cap = std::move(other.m_cap);
242+
239243
m_begin = other.m_begin;
244+
m_back = other.m_back;
245+
other.m_begin = invalid_iterator;
246+
other.m_back = invalid_iterator;
240247

241-
// Nullify other
242-
other.m_addressAllocator = nullptr;
243-
other.m_reservedSpace = nullptr;
244-
other.m_array = nullptr;
245-
other.m_cap = 0u;
246-
other.m_back = 0u;
247-
other.m_begin = 0u;
248248
return *this;
249249
}
250250

251251
~DoublyLinkedList()
252252
{
253253
disposeAll();
254-
_NBL_ALIGNED_FREE(m_reservedSpace);
254+
// Could be null if list was moved out of
255+
if (m_reservedSpace)
256+
{
257+
m_allocator.deallocate(reinterpret_cast<node_t*>(m_reservedSpace));
258+
}
255259
}
256260

257261
private:
258262
//allocate and get the address of the next free node
259263
inline uint32_t reserveAddress()
260264
{
261-
uint32_t addr = m_addressAllocator->alloc_addr(1u, 1u);
265+
uint32_t addr = m_addressAllocator.alloc_addr(1u, 1u);
262266
return addr;
263267
}
264268

@@ -301,7 +305,7 @@ class DoublyLinkedList
301305
if (m_dispose_f)
302306
m_dispose_f(get(address)->data);
303307
get(address)->~node_t();
304-
m_addressAllocator->free_addr(address, 1u);
308+
m_addressAllocator.free_addr(address, 1u);
305309
}
306310

307311
inline void common_detach(node_t* node)
@@ -312,7 +316,8 @@ class DoublyLinkedList
312316
get(node->prev)->next = node->next;
313317
}
314318

315-
std::unique_ptr<address_allocator_t> m_addressAllocator;
319+
allocator_t m_allocator;
320+
address_allocator_t m_addressAllocator;
316321
void* m_reservedSpace;
317322
node_t* m_array;
318323

0 commit comments

Comments
 (0)