Skip to content

Commit 636a6ea

Browse files
Erfan-AhmadiErfan-Ahmadi
committed
Merge branch 'dtm_LRUCacheUpgrades' into dtm
2 parents 6d5e678 + 1c529e9 commit 636a6ea

File tree

2 files changed

+94
-44
lines changed

2 files changed

+94
-44
lines changed

‎include/nbl/core/containers/DoublyLinkedList.h

Lines changed: 44 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@ template<typename Value, class allocator = core::allocator<SDoublyLinkedNode<Val
6969
class DoublyLinkedList
7070
{
7171
public:
72+
template <bool Mutable>
7273
class Iterator;
74+
template <bool Mutable>
7375
friend class Iterator;
7476

7577
using allocator_t = allocator;
@@ -245,7 +247,15 @@ class DoublyLinkedList
245247

246248
DoublyLinkedList() = default;
247249

248-
DoublyLinkedList(const DoublyLinkedList& other) = delete;
250+
// Unless there's a way of knowing whether for our type a shallow copy is the same as invoking the copy constructor,
251+
// we must iterate over the whole list and copy instance by instance
252+
explicit DoublyLinkedList(const DoublyLinkedList& other) : DoublyLinkedList(other.m_cap, disposal_func_t(other.m_dispose_f), other.m_allocator)
253+
{
254+
if (!m_cap) return; // Allocation failed
255+
// Reverse iteration since we push from the front
256+
for (auto it = other.crbegin(); it != other.crend(); it++)
257+
pushFront(value_t(*it));
258+
}
249259

250260
DoublyLinkedList& operator=(const DoublyLinkedList& other) = delete;
251261

@@ -277,14 +287,14 @@ class DoublyLinkedList
277287
}
278288

279289
// Iterator stuff
280-
Iterator begin();
281-
Iterator end();
282-
Iterator cbegin() const;
283-
Iterator cend() const;
284-
std::reverse_iterator<Iterator> rbegin();
285-
std::reverse_iterator<Iterator> rend();
286-
std::reverse_iterator<Iterator> crbegin() const;
287-
std::reverse_iterator<Iterator> crend() const;
290+
Iterator<true> begin();
291+
Iterator<true> end();
292+
Iterator<false> cbegin() const;
293+
Iterator<false> cend() const;
294+
std::reverse_iterator<Iterator<true>> rbegin();
295+
std::reverse_iterator<Iterator<true>> rend();
296+
std::reverse_iterator<Iterator<false>> crbegin() const;
297+
std::reverse_iterator<Iterator<false>> crend() const;
288298

289299
private:
290300
//allocate and get the address of the next free node
@@ -361,13 +371,14 @@ class DoublyLinkedList
361371

362372
// Satifies std::bidirectional_iterator
363373
template<typename Value, class allocator>
374+
template<bool Mutable>
364375
class DoublyLinkedList<Value, allocator>::Iterator
365376
{
366-
using iterable_t = DoublyLinkedList<Value, allocator>;
367-
using this_t = iterable_t::Iterator;
368-
friend class iterable_t;
377+
using base_iterable_t = DoublyLinkedList<Value, allocator>;
378+
using iterable_t = std::conditional_t<Mutable, base_iterable_t, const base_iterable_t>;
379+
friend class base_iterable_t;
369380
public:
370-
using value_type = const Value;
381+
using value_type = std::conditional_t<Mutable, Value, const Value>;
371382
using pointer = value_type*;
372383
using reference = value_type&;
373384
using difference_type = int32_t;
@@ -409,68 +420,68 @@ class DoublyLinkedList<Value, allocator>::Iterator
409420
}
410421

411422
//Deref
412-
value_type& operator*() const
423+
reference operator*() const
413424
{
414425
return m_iterable->get(m_current)->data;
415426
}
416427

417-
value_type* operator->() const
428+
pointer operator->() const
418429
{
419430
return & operator*();
420431
}
421432
private:
422-
DoublyLinkedList<Value, allocator>::Iterator(const iterable_t* const iterable, uint32_t idx) : m_iterable(iterable), m_current(idx) {}
433+
Iterator(iterable_t* const iterable, uint32_t idx) : m_iterable(iterable), m_current(idx) {}
423434

424-
const iterable_t* m_iterable;
435+
iterable_t* m_iterable;
425436
uint32_t m_current;
426437
};
427438

428439
template<typename Value, class allocator>
429-
DoublyLinkedList<Value, allocator>::Iterator DoublyLinkedList<Value, allocator>::begin()
440+
DoublyLinkedList<Value, allocator>::Iterator<true> DoublyLinkedList<Value, allocator>::begin()
430441
{
431-
return Iterator(this, m_begin);
442+
return Iterator<true>(this, m_begin);
432443
}
433444

434445
template<typename Value, class allocator>
435-
DoublyLinkedList<Value, allocator>::Iterator DoublyLinkedList<Value, allocator>::cbegin() const
446+
DoublyLinkedList<Value, allocator>::Iterator<false> DoublyLinkedList<Value, allocator>::cbegin() const
436447
{
437-
return Iterator(this, m_begin);
448+
return Iterator<false>(this, m_begin);
438449
}
439450

440451
template<typename Value, class allocator>
441-
DoublyLinkedList<Value, allocator>::Iterator DoublyLinkedList<Value, allocator>::end()
452+
DoublyLinkedList<Value, allocator>::Iterator<true> DoublyLinkedList<Value, allocator>::end()
442453
{
443-
return Iterator(this, invalid_iterator);
454+
return Iterator<true>(this, invalid_iterator);
444455
}
445456

446457
template<typename Value, class allocator>
447-
DoublyLinkedList<Value, allocator>::Iterator DoublyLinkedList<Value, allocator>::cend() const
458+
DoublyLinkedList<Value, allocator>::Iterator<false> DoublyLinkedList<Value, allocator>::cend() const
448459
{
449-
return Iterator(this, invalid_iterator);
460+
return Iterator<false>(this, invalid_iterator);
450461
}
451462

452463
template<typename Value, class allocator>
453-
std::reverse_iterator<typename DoublyLinkedList<Value, allocator>::Iterator> DoublyLinkedList<Value, allocator>::rbegin()
464+
std::reverse_iterator<typename DoublyLinkedList<Value, allocator>::Iterator<true>> DoublyLinkedList<Value, allocator>::rbegin()
454465
{
455-
return std::reverse_iterator<Iterator>(Iterator(this, invalid_iterator));
466+
return std::reverse_iterator<Iterator<true>>(Iterator<true>(this, invalid_iterator));
456467
}
457468

458469
template<typename Value, class allocator>
459-
std::reverse_iterator<typename DoublyLinkedList<Value, allocator>::Iterator> DoublyLinkedList<Value, allocator>::crbegin() const
470+
std::reverse_iterator<typename DoublyLinkedList<Value, allocator>::Iterator<false>> DoublyLinkedList<Value, allocator>::crbegin() const
460471
{
461-
return std::reverse_iterator<Iterator>(Iterator(this, invalid_iterator));
472+
return std::reverse_iterator<Iterator<false>>(Iterator<false>(this, invalid_iterator));
462473
}
463474

464475
template<typename Value, class allocator>
465-
std::reverse_iterator<typename DoublyLinkedList<Value, allocator>::Iterator> DoublyLinkedList<Value, allocator>::rend()
476+
std::reverse_iterator<typename DoublyLinkedList<Value, allocator>::Iterator<true>> DoublyLinkedList<Value, allocator>::rend()
466477
{
467-
return std::reverse_iterator<Iterator>(Iterator(this, m_begin));
478+
return std::reverse_iterator<Iterator<true>>(Iterator<true>(this, m_begin));
468479
}
469480

470481
template<typename Value, class allocator>
471-
std::reverse_iterator<typename DoublyLinkedList<Value, allocator>::Iterator> DoublyLinkedList<Value, allocator>::crend() const
482+
std::reverse_iterator<typename DoublyLinkedList<Value, allocator>::Iterator<false>> DoublyLinkedList<Value, allocator>::crend() const
472483
{
473-
return std::reverse_iterator<Iterator>(Iterator(this, m_begin));
484+
return std::reverse_iterator<Iterator<false>>(Iterator<false>(this, m_begin));
474485
}
475486

476487
} //namespace core

‎include/nbl/core/containers/LRUCache.h

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ class LRUCacheBase
3838
LRUCacheBase(const uint32_t capacity, MapHash&& _hash, MapEquals&& _equals, disposal_func_t&& df) : m_list(capacity, std::move(df)), m_hash(std::move(_hash)), m_equals(std::move(_equals)), searchedKey(nullptr)
3939
{ }
4040

41+
// CAREFUL: searchedKey was left as nullptr because for ResizableLRUCache, it's always set by some method before being used
42+
// If you implement another LRUCache where this is not the case and the state of the copy has to match exactly, it needs to be consistent.
43+
// What I would do in such a case (either in this same class or dividing this class again into two, one that leaves the searchedKey null and
44+
// another that sets it to a consistent value) would be to iterate over every Key-Value pair in the copy, and set searchedKey to point
45+
// to the Key such that `MapEquals(Key, *other.searchedKey)`
46+
LRUCacheBase(const LRUCacheBase& other) : m_list(other.m_list), m_hash(other.m_hash), m_equals(other.m_equals), searchedKey(nullptr) {}
47+
4148
public:
4249
inline const Key& getReference(const uint32_t nodeAddr) const
4350
{
@@ -221,6 +228,19 @@ class [[deprecated]] LRUCache : protected impl::LRUCacheBase<Key,Value,MapHash,M
221228
unordered_set<uint32_t,WrapHash,WrapEquals> m_shortcut_map;
222229
};
223230

231+
namespace impl
232+
{
233+
template<typename EvictionCallback, typename Value>
234+
concept LRUCacheValueEvictionCallback = std::invocable<EvictionCallback, const Value&>;
235+
236+
template<typename EvictionCallback, typename Key, typename Value>
237+
concept LRUCacheKeyValueEvictionCallback = std::invocable<EvictionCallback, const Key&, const Value&>;
238+
239+
template<typename EvictionCallback, typename Key, typename Value>
240+
concept LRUCacheInsertEvictionCallback = LRUCacheValueEvictionCallback<EvictionCallback, Value>
241+
|| LRUCacheKeyValueEvictionCallback<EvictionCallback, Key, Value>;
242+
} //namespace impl
243+
224244
// Key-Value Least Recently Used cache
225245
// Capacity can be increased at user's will
226246
// When the cache is full inserting will remove the least used entry
@@ -278,7 +298,8 @@ class ResizableLRUCache : protected impl::LRUCacheBase<Key, Value, MapHash, MapE
278298

279299
public:
280300
// Keep it simple
281-
using Iterator = list_t::Iterator;
301+
template <bool Mutable>
302+
using Iterator = typename list_t::template Iterator<Mutable>;
282303

283304
using disposal_func_t = typename base_t::disposal_func_t;
284305
using assoc_t = typename base_t::list_value_t;
@@ -292,6 +313,16 @@ class ResizableLRUCache : protected impl::LRUCacheBase<Key, Value, MapHash, MapE
292313
}
293314
ResizableLRUCache() = delete;
294315

316+
// COPY CONST
317+
// It's not possible to copy the unordered_set memory-wise and just change hashing and KeyEquals functions unfortunately
318+
// (in the general case that wouldn't make sense but it does here due to the way the wrappers work)
319+
// Anyway, we must iterate over the old cache and copy the map over
320+
explicit ResizableLRUCache(const ResizableLRUCache& other) : base_t(other), m_capacity(other.m_capacity),
321+
m_shortcut_map(other.m_shortcut_map.cbegin(), other.m_shortcut_map.cend(), other.m_capacity >> 2, WrapHash{this}, WrapEquals{this})
322+
{
323+
m_shortcut_map.reserve(m_capacity);
324+
}
325+
295326
inline void print(core::smart_refctd_ptr<system::ILogger> logger)
296327
{
297328
logger->log("Printing LRU cache contents");
@@ -326,7 +357,7 @@ class ResizableLRUCache : protected impl::LRUCacheBase<Key, Value, MapHash, MapE
326357
return stringStream.str();
327358
}
328359

329-
template<typename K, typename V, std::invocable<const Value&> EvictionCallback> requires std::is_constructible_v<Value, V> // && (std::is_same_v<Value,V> || std::is_assignable_v<Value,V>) // is_assignable_v<int, int&> returns false :(
360+
template<typename K, typename V, typename EvictionCallback> requires std::is_constructible_v<Value, V> && impl::LRUCacheInsertEvictionCallback<EvictionCallback, Key, Value>// && (std::is_same_v<Value,V> || std::is_assignable_v<Value,V>) // is_assignable_v<int, int&> returns false :(
330361
inline Value* insert(K&& k, V&& v, EvictionCallback&& evictCallback)
331362
{
332363
bool success;
@@ -342,7 +373,15 @@ class ResizableLRUCache : protected impl::LRUCacheBase<Key, Value, MapHash, MapE
342373
const bool overflow = size() >= base_t::m_list.getCapacity();
343374
if (overflow)
344375
{
345-
evictCallback(base_t::m_list.getBack()->data.second);
376+
if constexpr (impl::LRUCacheValueEvictionCallback<EvictionCallback, Value>)
377+
{
378+
evictCallback(base_t::m_list.getBack()->data.second);
379+
}
380+
// LRUCacheKeyValueEvictionCallback
381+
else
382+
{
383+
evictCallback(base_t::m_list.getBack()->data.first, base_t::m_list.getBack()->data.second);
384+
}
346385
m_shortcut_map.erase(base_t::m_list.getLastAddress());
347386
base_t::m_list.popBack();
348387
}
@@ -446,14 +485,14 @@ class ResizableLRUCache : protected impl::LRUCacheBase<Key, Value, MapHash, MapE
446485

447486
// Iterator stuff
448487
// Normal iterator order is MRU -> LRU
449-
Iterator begin() { return base_t::m_list.begin(); }
450-
Iterator end() { return base_t::m_list.end(); }
451-
Iterator cbegin() const { return base_t::m_list.cbegin(); }
452-
Iterator cend() const { return base_t::m_list.cend(); }
453-
std::reverse_iterator<Iterator> rbegin() { return base_t::m_list.rbegin(); }
454-
std::reverse_iterator<Iterator> rend() { return base_t::m_list.rend(); }
455-
std::reverse_iterator<Iterator> crbegin() const { return base_t::m_list.crbegin(); }
456-
std::reverse_iterator<Iterator> crend() const { return base_t::m_list.crend(); }
488+
Iterator<true> begin() { return base_t::m_list.begin(); }
489+
Iterator<true> end() { return base_t::m_list.end(); }
490+
Iterator<false> cbegin() const { return base_t::m_list.cbegin(); }
491+
Iterator<false> cend() const { return base_t::m_list.cend(); }
492+
std::reverse_iterator<Iterator<true>> rbegin() { return base_t::m_list.rbegin(); }
493+
std::reverse_iterator<Iterator<true>> rend() { return base_t::m_list.rend(); }
494+
std::reverse_iterator<Iterator<false>> crbegin() const { return base_t::m_list.crbegin(); }
495+
std::reverse_iterator<Iterator<false>> crend() const { return base_t::m_list.crend(); }
457496

458497
protected:
459498
unordered_set<uint32_t, WrapHash, WrapEquals> m_shortcut_map;

0 commit comments

Comments
 (0)