Skip to content

Commit 55cba8a

Browse files
committed
fix(priorityqueue): avoid unnecessary copies
1 parent 795de95 commit 55cba8a

9 files changed

Lines changed: 105 additions & 13 deletions

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ set(TEV_SOURCES
188188
include/tev/Ipc.h src/Ipc.cpp
189189
include/tev/Lazy.h src/Lazy.cpp
190190
include/tev/MultiGraph.h src/MultiGraph.cpp
191+
include/tev/PriorityQueue.h src/PriorityQueue.cpp
191192
include/tev/SharedQueue.h src/SharedQueue.cpp
192193
include/tev/Task.h src/Task.cpp
193194
include/tev/ThreadPool.h src/ThreadPool.cpp

include/tev/BackgroundImagesLoader.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
#include <tev/Common.h>
2222
#include <tev/Image.h>
23+
#include <tev/PriorityQueue.h>
2324
#include <tev/SharedQueue.h>
2425
#include <tev/imageio/ImageLoader.h>
2526

@@ -76,7 +77,7 @@ class BackgroundImagesLoader {
7677
private:
7778
SharedQueue<ImageAddition> mLoadedImages;
7879

79-
std::priority_queue<ImageAddition, std::vector<ImageAddition>, ImageAddition::Comparator> mPendingLoadedImages;
80+
PriorityQueue<ImageAddition, ImageAddition::Comparator> mPendingLoadedImages;
8081
mutable std::mutex mPendingLoadedImagesMutex;
8182

8283
std::atomic<int> mLoadCounter{0};

include/tev/PriorityQueue.h

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* tev -- the EDR viewer
3+
*
4+
* Copyright (C) 2025 Thomas Müller <contact@tom94.net>
5+
*
6+
* This program is free software: you can redistribute it and/or modify
7+
* it under the terms of the GNU General Public License as published by
8+
* the Free Software Foundation, either version 3 of the License.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
19+
#pragma once
20+
21+
#include <algorithm>
22+
#include <stdexcept>
23+
#include <utility>
24+
#include <vector>
25+
26+
namespace tev {
27+
28+
template <typename T, typename Cmp = std::less<T>> class PriorityQueue {
29+
std::vector<T> mData;
30+
Cmp mCmp;
31+
32+
public:
33+
explicit PriorityQueue(Cmp cmp = Cmp{}) : mCmp{cmp} {}
34+
35+
template <typename Iter> PriorityQueue(Iter first, Iter last, Cmp cmp = Cmp{}) : mData{first, last}, mCmp{cmp} {
36+
std::make_heap(mData.begin(), mData.end(), mCmp);
37+
}
38+
39+
void push(const T& val) {
40+
mData.push_back(val);
41+
std::push_heap(mData.begin(), mData.end(), mCmp);
42+
}
43+
44+
void push(T&& val) {
45+
mData.push_back(std::move(val));
46+
std::push_heap(mData.begin(), mData.end(), mCmp);
47+
}
48+
49+
T pop() {
50+
if (mData.empty()) {
51+
throw std::runtime_error{"pop from empty queue"};
52+
}
53+
54+
std::pop_heap(mData.begin(), mData.end(), mCmp);
55+
T val = std::move(mData.back());
56+
mData.pop_back();
57+
return val;
58+
}
59+
60+
const T& top() const & { return mData.front(); }
61+
T& top() & { return mData.front(); }
62+
63+
bool empty() const { return mData.empty(); }
64+
std::size_t size() const { return mData.size(); }
65+
};
66+
67+
} // namespace tev

include/tev/SharedQueue.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,18 @@ template <typename T> class SharedQueue {
3939
return mRawQueue.size();
4040
}
4141

42-
void push(T newElem) {
42+
void push(const T& newElem) {
4343
std::lock_guard lock{mMutex};
4444
mRawQueue.push_back(newElem);
4545
mDataCondition.notify_one();
4646
}
4747

48+
void push(T&& newElem) {
49+
std::lock_guard lock{mMutex};
50+
mRawQueue.push_back(std::move(newElem));
51+
mDataCondition.notify_one();
52+
}
53+
4854
T waitAndPop() {
4955
std::unique_lock lock{mMutex};
5056

include/tev/ThreadPool.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#pragma once
2020

2121
#include <tev/Common.h>
22+
#include <tev/PriorityQueue.h>
2223
#include <tev/Task.h>
2324

2425
#include <concurrentqueue/concurrentqueue.h> // Needs to be included before lightweightsemaphore.h
@@ -28,7 +29,6 @@
2829
#include <concepts>
2930
#include <functional>
3031
#include <future>
31-
#include <queue>
3232
#include <thread>
3333
#include <vector>
3434

@@ -183,7 +183,7 @@ class ThreadPool {
183183
};
184184
};
185185

186-
std::priority_queue<QueuedTask, std::vector<QueuedTask>, QueuedTask::Comparator> mTaskQueue;
186+
PriorityQueue<QueuedTask, QueuedTask::Comparator> mTaskQueue;
187187
std::recursive_mutex mTaskQueueMutex;
188188

189189
// Significantly faster than C++20's std::semaphore implementation on some platforms

src/BackgroundImagesLoader.cpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,14 +126,8 @@ bool BackgroundImagesLoader::publishSortedLoads() {
126126
const lock_guard lock{mPendingLoadedImagesMutex};
127127
bool pushed = false;
128128
while (!mPendingLoadedImages.empty() && mPendingLoadedImages.top().loadId == mLoadCounter) {
129-
// null image pointers indicate failed loads. These shouldn't be pushed.
130-
if (!mPendingLoadedImages.top().images.empty()) {
131-
mLoadedImages.push(mPendingLoadedImages.top());
132-
}
133-
134-
mPendingLoadedImages.pop();
129+
mLoadedImages.push(mPendingLoadedImages.pop());
135130
pushed = true;
136-
137131
++mLoadCounter;
138132
}
139133

src/ImageViewer.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,6 +1304,7 @@ void ImageViewer::draw_contents() {
13041304
} else {
13051305
addImage(image, shallSelect);
13061306
}
1307+
13071308
first = false;
13081309
}
13091310
}

src/PriorityQueue.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* tev -- the EDR viewer
3+
*
4+
* Copyright (C) 2025 Thomas Müller <contact@tom94.net>
5+
*
6+
* This program is free software: you can redistribute it and/or modify
7+
* it under the terms of the GNU General Public License as published by
8+
* the Free Software Foundation, either version 3 of the License.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
19+
#include <tev/PriorityQueue.h>
20+
21+
namespace tev {
22+
23+
}

src/ThreadPool.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,7 @@ void ThreadPool::startThreads(size_t num) {
6464
const scoped_lock taskQueueLock{mTaskQueueMutex};
6565
TEV_ASSERT(!mTaskQueue.empty(), "Task queue was empty after semaphore wait.");
6666

67-
task = std::move(mTaskQueue.top());
68-
mTaskQueue.pop();
67+
task = mTaskQueue.pop();
6968
}
7069

7170
task.fun();

0 commit comments

Comments
 (0)