Skip to content

Commit

Permalink
Remove unnecessary ref-counting of QRunnable
Browse files Browse the repository at this point in the history
It could never be higher than 1 anyway.

Change-Id: If33c7978a4397a08e9eb091926726725d8bd3ea6
Reviewed-by: Sona Kurazyan <sona.kurazyan@qt.io>
  • Loading branch information
Allan Sandfeld Jensen committed Oct 1, 2020
1 parent 4940f6d commit ecfda98
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 32 deletions.
11 changes: 4 additions & 7 deletions src/corelib/thread/qrunnable.h
Expand Up @@ -47,21 +47,18 @@ QT_BEGIN_NAMESPACE

class Q_CORE_EXPORT QRunnable
{
int ref; // Qt6: Make this a bool, or make autoDelete() virtual.
bool m_autoDelete = true;

friend class QThreadPool;
friend class QThreadPoolPrivate;
friend class QThreadPoolThread;
Q_DISABLE_COPY(QRunnable)
public:
virtual void run() = 0;

QRunnable() : ref(0) { }
constexpr QRunnable() noexcept = default;
virtual ~QRunnable();
static QRunnable *create(std::function<void()> functionToRun);

bool autoDelete() const { return ref != -1; }
void setAutoDelete(bool _autoDelete) { ref = _autoDelete ? 0 : -1; }
bool autoDelete() const { return m_autoDelete; }
void setAutoDelete(bool autoDelete) { m_autoDelete = autoDelete; }
};

QT_END_NAMESPACE
Expand Down
28 changes: 3 additions & 25 deletions src/corelib/thread/qthreadpool.cpp
Expand Up @@ -88,9 +88,8 @@ void QThreadPoolThread::run()

do {
if (r) {
// If autoDelete() is false, r might already be deleted after run(), so check status now.
const bool del = r->autoDelete();
Q_ASSERT(!del || r->ref == 1);


// run the task
locker.unlock();
Expand Down Expand Up @@ -330,7 +329,6 @@ void QThreadPoolPrivate::clear()
while (!page->isFinished()) {
QRunnable *r = page->pop();
if (r && r->autoDelete()) {
Q_ASSERT(r->ref == 1);
locker.unlock();
delete r;
locker.relock();
Expand Down Expand Up @@ -371,10 +369,6 @@ bool QThreadPool::tryTake(QRunnable *runnable)
d->queue.removeOne(page);
delete page;
}
if (runnable->autoDelete()) {
Q_ASSERT(runnable->ref == 1);
--runnable->ref; // undo ++ref in start()
}
return true;
}
}
Expand All @@ -393,14 +387,13 @@ void QThreadPoolPrivate::stealAndRunRunnable(QRunnable *runnable)
Q_Q(QThreadPool);
if (!q->tryTake(runnable))
return;
// If autoDelete() is false, runnable might already be deleted after run(), so check status now.
const bool del = runnable->autoDelete();

runnable->run();

if (del) {
Q_ASSERT(runnable->ref == 0); // tryTake already deref'ed
if (del)
delete runnable;
}
}

/*!
Expand Down Expand Up @@ -508,10 +501,6 @@ void QThreadPool::start(QRunnable *runnable, int priority)

Q_D(QThreadPool);
QMutexLocker locker(&d->mutex);
if (runnable->autoDelete()) {
Q_ASSERT(runnable->ref == 0);
++runnable->ref;
}

if (!d->tryStart(runnable)) {
d->enqueueTask(runnable, priority);
Expand Down Expand Up @@ -558,22 +547,11 @@ bool QThreadPool::tryStart(QRunnable *runnable)
if (!runnable)
return false;

if (runnable->autoDelete()) {
Q_ASSERT(runnable->ref == 0);
++runnable->ref;
}

Q_D(QThreadPool);
QMutexLocker locker(&d->mutex);
if (d->tryStart(runnable))
return true;

// Undo the reference above as we did not start the runnable and
// take over ownership.
if (runnable->autoDelete()) {
--runnable->ref;
Q_ASSERT(runnable->ref == 0);
}
return false;
}

Expand Down

0 comments on commit ecfda98

Please sign in to comment.