From d89d9c2f751a8dc1bfd2895f08756b7dde5aa77f Mon Sep 17 00:00:00 2001 From: Ivan Sorokin Date: Sat, 11 Oct 2014 02:37:25 +0400 Subject: [PATCH 1/3] Fewer calls to torrent_handle::info_hash() --- src/qtlibtorrent/torrentmodel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qtlibtorrent/torrentmodel.cpp b/src/qtlibtorrent/torrentmodel.cpp index 3dca808c7..419361ec1 100644 --- a/src/qtlibtorrent/torrentmodel.cpp +++ b/src/qtlibtorrent/torrentmodel.cpp @@ -549,7 +549,7 @@ void TorrentModel::stateUpdated(const std::vector &s for (statuses_t::const_iterator i = statuses.begin(), end = statuses.end(); i != end; ++i) { libtorrent::torrent_status const& status = *i; - const int row = torrentRow(misc::toQString(status.handle.info_hash())); + const int row = torrentRow(misc::toQString(status.info_hash)); if (row >= 0) m_torrents[row]->refreshStatus(status); } From 333978f1ff6b9f71b97239eac52b901240e76aea Mon Sep 17 00:00:00 2001 From: Ivan Sorokin Date: Sun, 12 Oct 2014 11:26:19 +0400 Subject: [PATCH 2/3] Use std::vector instead of std::deque in QAlertDispatcher As we never use {push,pop}_front std::vector works here perfectly. Also reserve memory for std::vector out of lock. This could be considered as an optimization, but in reality this is just using right container in right place. According to my measurements total speedup is under 0.2%. --- src/qtlibtorrent/alertdispatcher.cpp | 9 +++++++-- src/qtlibtorrent/alertdispatcher.h | 6 +++--- src/qtlibtorrent/qbtsession.cpp | 6 +++--- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/qtlibtorrent/alertdispatcher.cpp b/src/qtlibtorrent/alertdispatcher.cpp index b9d3a7a53..1bcfed64e 100644 --- a/src/qtlibtorrent/alertdispatcher.cpp +++ b/src/qtlibtorrent/alertdispatcher.cpp @@ -33,12 +33,15 @@ #include #include +const size_t DEFAULT_ALERTS_CAPACITY = 32; + QAlertDispatcher::QAlertDispatcher(libtorrent::session *session, QObject* parent) : QObject(parent) , m_session(session) , current_tag(new QAtomicPointer(this)) , event_posted(false) { + alerts.reserve(DEFAULT_ALERTS_CAPACITY); m_session->set_alert_dispatch(boost::bind(&QAlertDispatcher::dispatch, current_tag, _1)); } @@ -60,16 +63,18 @@ QAlertDispatcher::~QAlertDispatcher() { m_session->set_alert_dispatch(dispatch_function_t()); } -void QAlertDispatcher::getPendingAlertsNoWait(std::deque& out) { +void QAlertDispatcher::getPendingAlertsNoWait(std::vector& out) { Q_ASSERT(out.empty()); + out.reserve(DEFAULT_ALERTS_CAPACITY); QMutexLocker lock(&alerts_mutex); alerts.swap(out); event_posted = false; } -void QAlertDispatcher::getPendingAlerts(std::deque& out, unsigned long time) { +void QAlertDispatcher::getPendingAlerts(std::vector& out, unsigned long time) { Q_ASSERT(out.empty()); + out.reserve(DEFAULT_ALERTS_CAPACITY); QMutexLocker lock(&alerts_mutex); diff --git a/src/qtlibtorrent/alertdispatcher.h b/src/qtlibtorrent/alertdispatcher.h index a107bf4e1..b9d097f24 100644 --- a/src/qtlibtorrent/alertdispatcher.h +++ b/src/qtlibtorrent/alertdispatcher.h @@ -46,8 +46,8 @@ public: QAlertDispatcher(libtorrent::session *session, QObject* parent); ~QAlertDispatcher(); - void getPendingAlertsNoWait(std::deque&); - void getPendingAlerts(std::deque&, unsigned long time = ULONG_MAX); + void getPendingAlertsNoWait(std::vector&); + void getPendingAlerts(std::vector&, unsigned long time = ULONG_MAX); signals: void alertsReceived(); @@ -64,7 +64,7 @@ private: libtorrent::session *m_session; QMutex alerts_mutex; QWaitCondition alerts_condvar; - std::deque alerts; + std::vector alerts; QSharedPointer > current_tag; bool event_posted; }; diff --git a/src/qtlibtorrent/qbtsession.cpp b/src/qtlibtorrent/qbtsession.cpp index 818cff613..f8378fc5a 100755 --- a/src/qtlibtorrent/qbtsession.cpp +++ b/src/qtlibtorrent/qbtsession.cpp @@ -1649,7 +1649,7 @@ void QBtSession::saveFastResumeData() { } catch(libtorrent::invalid_handle&) {} } while (num_resume_data > 0) { - std::deque alerts; + std::vector alerts; m_alertDispatcher->getPendingAlerts(alerts, 30*1000); if (alerts.empty()) { std::cerr << " aborting with " << num_resume_data << " outstanding " @@ -1657,7 +1657,7 @@ void QBtSession::saveFastResumeData() { break; } - for (std::deque::const_iterator i = alerts.begin(), end = alerts.end(); i != end; ++i) + for (std::vector::const_iterator i = alerts.begin(), end = alerts.end(); i != end; ++i) { alert const* a = *i; // Saving fastresume data can fail @@ -2147,7 +2147,7 @@ void QBtSession::sendNotificationEmail(const QTorrentHandle &h) { // Read alerts sent by the Bittorrent session void QBtSession::readAlerts() { - typedef std::deque alerts_t; + typedef std::vector alerts_t; alerts_t alerts; m_alertDispatcher->getPendingAlertsNoWait(alerts); From b995a9d75e0bc1b9e632e99b37574e16f39efa1c Mon Sep 17 00:00:00 2001 From: Ivan Sorokin Date: Sun, 12 Oct 2014 12:01:30 +0400 Subject: [PATCH 3/3] Fix race condition in QAlertDispatcher It was possible that QAlertDispatcher::dispatch() could access (lock) mutex that was destroyed by main thread. Fix this by moving mutex into a tag. --- src/qtlibtorrent/alertdispatcher.cpp | 43 ++++++++++++++-------------- src/qtlibtorrent/alertdispatcher.h | 7 +++-- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/qtlibtorrent/alertdispatcher.cpp b/src/qtlibtorrent/alertdispatcher.cpp index 1bcfed64e..ccfd3308c 100644 --- a/src/qtlibtorrent/alertdispatcher.cpp +++ b/src/qtlibtorrent/alertdispatcher.cpp @@ -35,10 +35,21 @@ const size_t DEFAULT_ALERTS_CAPACITY = 32; +struct QAlertDispatcher::Tag { + Tag(QAlertDispatcher* dispatcher); + + QAlertDispatcher* dispatcher; + QMutex alerts_mutex; +}; + +QAlertDispatcher::Tag::Tag(QAlertDispatcher* dispatcher) + : dispatcher(dispatcher) +{} + QAlertDispatcher::QAlertDispatcher(libtorrent::session *session, QObject* parent) : QObject(parent) , m_session(session) - , current_tag(new QAtomicPointer(this)) + , current_tag(new Tag(this)) , event_posted(false) { alerts.reserve(DEFAULT_ALERTS_CAPACITY); @@ -54,8 +65,8 @@ QAlertDispatcher::~QAlertDispatcher() { // with invalid tag it simply discard an alert. { - QMutexLocker lock(&alerts_mutex); - *current_tag = 0; + QMutexLocker lock(¤t_tag->alerts_mutex); + current_tag->dispatcher = 0; current_tag.clear(); } @@ -67,7 +78,7 @@ void QAlertDispatcher::getPendingAlertsNoWait(std::vector& o Q_ASSERT(out.empty()); out.reserve(DEFAULT_ALERTS_CAPACITY); - QMutexLocker lock(&alerts_mutex); + QMutexLocker lock(¤t_tag->alerts_mutex); alerts.swap(out); event_posted = false; } @@ -76,34 +87,22 @@ void QAlertDispatcher::getPendingAlerts(std::vector& out, un Q_ASSERT(out.empty()); out.reserve(DEFAULT_ALERTS_CAPACITY); - QMutexLocker lock(&alerts_mutex); + QMutexLocker lock(¤t_tag->alerts_mutex); while (alerts.empty()) - alerts_condvar.wait(&alerts_mutex, time); + alerts_condvar.wait(¤t_tag->alerts_mutex, time); alerts.swap(out); event_posted = false; } -void QAlertDispatcher::dispatch(QSharedPointer > tag, +void QAlertDispatcher::dispatch(QSharedPointer tag, std::auto_ptr alert_ptr) { -#if (QT_VERSION >= QT_VERSION_CHECK(5, 0, 0)) - QAlertDispatcher* that = tag->loadAcquire(); -#else - QAlertDispatcher* that = *tag; -#endif + QMutexLocker lock(&(tag->alerts_mutex)); + QAlertDispatcher* that = tag->dispatcher; if (!that) return; - QMutexLocker lock(&(that->alerts_mutex)); - -#if (QT_VERSION >= QT_VERSION_CHECK(5, 0, 0)) - if (!tag->load()) -#else - if (!*tag) -#endif - return; - bool was_empty = that->alerts.empty(); that->alerts.push_back(alert_ptr.get()); @@ -127,7 +126,7 @@ void QAlertDispatcher::enqueueToMainThread() { void QAlertDispatcher::deliverSignal() { emit alertsReceived(); - QMutexLocker lock(&alerts_mutex); + QMutexLocker lock(¤t_tag->alerts_mutex); event_posted = false; if (!alerts.empty()) diff --git a/src/qtlibtorrent/alertdispatcher.h b/src/qtlibtorrent/alertdispatcher.h index b9d097f24..aa2a9ec1f 100644 --- a/src/qtlibtorrent/alertdispatcher.h +++ b/src/qtlibtorrent/alertdispatcher.h @@ -42,6 +42,8 @@ class QAlertDispatcher : public QObject { Q_OBJECT Q_DISABLE_COPY(QAlertDispatcher) + struct Tag; + public: QAlertDispatcher(libtorrent::session *session, QObject* parent); ~QAlertDispatcher(); @@ -53,7 +55,7 @@ signals: void alertsReceived(); private: - static void dispatch(QSharedPointer >, + static void dispatch(QSharedPointer, std::auto_ptr); void enqueueToMainThread(); @@ -62,10 +64,9 @@ private slots: private: libtorrent::session *m_session; - QMutex alerts_mutex; QWaitCondition alerts_condvar; std::vector alerts; - QSharedPointer > current_tag; + QSharedPointer current_tag; bool event_posted; };