3DPrinting/PrusaSlicer-NonPlainar
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Fix of libslic3r "Mutable priority queue - first pop" test failure #8276

Browse source 
Improved readability by introducing invalid_id() getter.
Made the ResetIndexWhenRemoved flag active in both debug and release mode,
it used to be made active by Vojtech for release mode only for unknown
reason.
This commit is contained in:
Vojtech Bubnik 2022-05-16 11:34:06 +02:00
parent fd0579d4a2
commit 15a082b80b
2 changed files with 25 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

49
src/libslic3r/MutablePriorityQueue.hpp
View file

@ -41,6 +41,7 @@ public:
bool empty() const { return m_heap.empty(); }
T& operator[](std::size_t idx) noexcept { return m_heap[idx]; }
const T& operator[](std::size_t idx) const noexcept { return m_heap[idx]; }
static constexpr size_t invalid_id() { return std::numeric_limits<size_t>::max(); }
using iterator = typename std::vector<T>::iterator;
using const_iterator = typename std::vector<T>::const_iterator;
@ -69,14 +70,10 @@ MutablePriorityQueue<T, IndexSetter, LessPredicate, ResetIndexWhenRemoved> make_
template<class T, class LessPredicate, class IndexSetter, const bool ResetIndexWhenRemoved>
inline void MutablePriorityQueue<T, LessPredicate, IndexSetter, ResetIndexWhenRemoved>::clear()
{
#ifdef NDEBUG
// Only mark as removed from the queue in release mode, if configured so.
if (ResetIndexWhenRemoved)
#endif /* NDEBUG */
{
if constexpr (ResetIndexWhenRemoved) {
for (size_t idx = 0; idx < m_heap.size(); ++ idx)
// Mark as removed from the queue.
m_index_setter(m_heap[idx], std::numeric_limits<size_t>::max());
m_index_setter(m_heap[idx], this->invalid_id());
}
m_heap.clear();
}
@ -103,13 +100,9 @@ template<class T, class LessPredicate, class IndexSetter, const bool ResetIndexW
inline void MutablePriorityQueue<T, LessPredicate, IndexSetter, ResetIndexWhenRemoved>::pop()
{
assert(! m_heap.empty());
#ifdef NDEBUG
// Only mark as removed from the queue in release mode, if configured so.
if (ResetIndexWhenRemoved)
#endif /* NDEBUG */
{
if constexpr (ResetIndexWhenRemoved) {
// Mark as removed from the queue.
m_index_setter(m_heap.front(), std::numeric_limits<size_t>::max());
m_index_setter(m_heap.front(), this->invalid_id());
}
if (m_heap.size() > 1) {
m_heap.front() = m_heap.back();
@ -124,13 +117,10 @@ template<class T, class LessPredicate, class IndexSetter, const bool ResetIndexW
inline void MutablePriorityQueue<T, LessPredicate, IndexSetter, ResetIndexWhenRemoved>::remove(size_t idx)
{
assert(idx < m_heap.size());
#ifdef NDEBUG
// Only mark as removed from the queue in release mode, if configured so.
if (ResetIndexWhenRemoved)
#endif /* NDEBUG */
{
if constexpr (ResetIndexWhenRemoved) {
// Mark as removed from the queue.
m_index_setter(m_heap[idx], std::numeric_limits<size_t>::max());
m_index_setter(m_heap[idx], this->invalid_id());
}
if (idx + 1 == m_heap.size()) {
m_heap.pop_back();
@ -291,6 +281,7 @@ public:
bool empty() const { return m_heap.empty(); }
T& operator[](std::size_t idx) noexcept { assert(! address::is_padding(idx)); return m_heap[idx]; }
const T& operator[](std::size_t idx) const noexcept { assert(! address::is_padding(idx)); return m_heap[idx]; }
static constexpr size_t invalid_id() { return std::numeric_limits<size_t>::max(); }
protected:
void update_heap_up(size_t top, size_t bottom);
@ -320,15 +311,11 @@ MutableSkipHeapPriorityQueue<T, IndexSetter, LessPredicate, BlockSize, ResetInde
template<class T, class LessPredicate, class IndexSetter, std::size_t blocking, const bool ResetIndexWhenRemoved>
inline void MutableSkipHeapPriorityQueue<T, LessPredicate, IndexSetter, blocking, ResetIndexWhenRemoved>::clear()
{
#ifdef NDEBUG
// Only mark as removed from the queue in release mode, if configured so.
if (ResetIndexWhenRemoved)
#endif /* NDEBUG */
{
if constexpr (ResetIndexWhenRemoved) {
for (size_t idx = 0; idx < m_heap.size(); ++ idx)
// Mark as removed from the queue.
if (! address::is_padding(idx))
m_index_setter(m_heap[idx], std::numeric_limits<size_t>::max());
m_index_setter(m_heap[idx], this->invalid_id());
}
m_heap.clear();
}
@ -359,13 +346,9 @@ template<class T, class LessPredicate, class IndexSetter, std::size_t blocking,
inline void MutableSkipHeapPriorityQueue<T, LessPredicate, IndexSetter, blocking, ResetIndexWhenRemoved>::pop()
{
assert(! m_heap.empty());
#ifdef NDEBUG
// Only mark as removed from the queue in release mode, if configured so.
if (ResetIndexWhenRemoved)
#endif /* NDEBUG */
{
if constexpr (ResetIndexWhenRemoved) {
// Mark as removed from the queue.
m_index_setter(m_heap[1], std::numeric_limits<size_t>::max());
m_index_setter(m_heap[1], this->invalid_id());
}
// Zero'th element is padding, thus non-empty queue must have at least two elements.
if (m_heap.size() > 2) {
@ -382,13 +365,9 @@ inline void MutableSkipHeapPriorityQueue<T, LessPredicate, IndexSetter, blocking
{
assert(idx < m_heap.size());
assert(! address::is_padding(idx));
#ifdef NDEBUG
// Only mark as removed from the queue in release mode, if configured so.
if (ResetIndexWhenRemoved)
#endif /* NDEBUG */
{
if constexpr (ResetIndexWhenRemoved) {
// Mark as removed from the queue.
m_index_setter(m_heap[idx], std::numeric_limits<size_t>::max());
m_index_setter(m_heap[idx], this->invalid_id());
}
if (idx + 1 == m_heap.size()) {
this->pop_back();

25
tests/libslic3r/test_mutable_priority_queue.cpp
View file

@ -346,25 +346,22 @@ TEST_CASE("Mutable priority queue - first pop", "[MutableSkipHeapPriorityQueue]"
size_t id;
float val;
};
size_t count = 50000;
std::vector<size_t> idxs(count, {0});
std::vector<bool> dels(count, false);
static constexpr const size_t count = 50000;
auto q = make_miniheap_mutable_priority_queue<MyValue, 16, true>(
[&](MyValue &v, size_t idx) {
idxs[v.id] = idx;
},
[](MyValue &v, size_t idx) { v.id = idx; },
[](MyValue &l, MyValue &r) { return l.val < r.val; });
using QueueType = decltype(q);
q.reserve(count);
for (size_t id = 0; id < count; id++) {
MyValue mv{ id, rand() / 100.f };
q.push(mv);
}
MyValue it = q.top(); // copy
q.pop();
for (size_t id = 0; id < count; ++ id)
q.push({ id, rand() / 100.f });
MyValue v = q.top(); // copy
// is valid id (no initial value default value)
CHECK(it.id != 0);
CHECK(QueueType::address::is_padding(0));
CHECK(!QueueType::address::is_padding(1));
q.pop();
CHECK(v.id == 1);
// is first item in queue valid value
CHECK(idxs[0] != std::numeric_limits<size_t>::max());
CHECK(q.top().id == 1);
}
TEST_CASE("Mutable priority queue complex", "[MutableSkipHeapPriorityQueue]")