From daf52a2610b2dbe84c00bd4b5628fb03f2fde5b5 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Sun, 8 Sep 2019 14:00:08 +0800 Subject: [PATCH 1/3] Avoid double lookups --- src/base/settingsstorage.cpp | 15 ++++++++------- src/base/settingvalue.h | 3 +++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/base/settingsstorage.cpp b/src/base/settingsstorage.cpp index e87f5562c..5ee51c266 100644 --- a/src/base/settingsstorage.cpp +++ b/src/base/settingsstorage.cpp @@ -202,17 +202,19 @@ bool SettingsStorage::save() QVariant SettingsStorage::loadValue(const QString &key, const QVariant &defaultValue) const { - QReadLocker locker(&m_lock); + const QReadLocker locker(&m_lock); return m_data.value(mapKey(key), defaultValue); } void SettingsStorage::storeValue(const QString &key, const QVariant &value) { const QString realKey = mapKey(key); - QWriteLocker locker(&m_lock); - if (m_data.value(realKey) != value) { + const QWriteLocker locker(&m_lock); + + QVariant ¤tValue = m_data[realKey]; + if (currentValue != value) { m_dirty = true; - m_data.insert(realKey, value); + currentValue = value; m_timer.start(); } } @@ -220,10 +222,9 @@ void SettingsStorage::storeValue(const QString &key, const QVariant &value) void SettingsStorage::removeValue(const QString &key) { const QString realKey = mapKey(key); - QWriteLocker locker(&m_lock); - if (m_data.contains(realKey)) { + const QWriteLocker locker(&m_lock); + if (m_data.remove(realKey) > 0) { m_dirty = true; - m_data.remove(realKey); m_timer.start(); } } diff --git a/src/base/settingvalue.h b/src/base/settingvalue.h index 64998e2d4..ad863e10a 100644 --- a/src/base/settingvalue.h +++ b/src/base/settingvalue.h @@ -68,6 +68,9 @@ public: CachedSettingValue &operator=(const T &newValue) { + if (m_value == newValue) + return *this; + m_value = newValue; storeValue(m_value); return *this; From e7e5ee1ea2bdc7b37864c7572293573ffd343c89 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Sun, 8 Sep 2019 14:38:14 +0800 Subject: [PATCH 2/3] Add const to TransactionalSettings class functions --- src/base/settingsstorage.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/base/settingsstorage.cpp b/src/base/settingsstorage.cpp index 5ee51c266..4548335d9 100644 --- a/src/base/settingsstorage.cpp +++ b/src/base/settingsstorage.cpp @@ -51,16 +51,16 @@ namespace { } - QVariantHash read(); - bool write(const QVariantHash &data); + QVariantHash read() const; + bool write(const QVariantHash &data) const; private: // we return actual file names used by QSettings because // there is no other way to get that name except // actually create a QSettings object. // if serialization operation was not successful we return empty string - QString deserialize(const QString &name, QVariantHash &data); - QString serialize(const QString &name, const QVariantHash &data); + QString deserialize(const QString &name, QVariantHash &data) const; + QString serialize(const QString &name, const QVariantHash &data) const; const QString m_name; }; @@ -187,10 +187,10 @@ SettingsStorage *SettingsStorage::instance() bool SettingsStorage::save() { if (!m_dirty) return false; // Obtaining the lock is expensive, let's check early - QWriteLocker locker(&m_lock); + const QWriteLocker locker(&m_lock); if (!m_dirty) return false; // something might have changed while we were getting the lock - TransactionalSettings settings(QLatin1String("qBittorrent")); + const TransactionalSettings settings(QLatin1String("qBittorrent")); if (settings.write(m_data)) { m_dirty = false; return true; @@ -229,7 +229,7 @@ void SettingsStorage::removeValue(const QString &key) } } -QVariantHash TransactionalSettings::read() +QVariantHash TransactionalSettings::read() const { QVariantHash res; @@ -257,7 +257,7 @@ QVariantHash TransactionalSettings::read() return res; } -bool TransactionalSettings::write(const QVariantHash &data) +bool TransactionalSettings::write(const QVariantHash &data) const { // QSettings deletes the file before writing it out. This can result in problems // if the disk is full or a power outage occurs. Those events might occur @@ -278,7 +278,7 @@ bool TransactionalSettings::write(const QVariantHash &data) return QFile::rename(newPath, finalPath); } -QString TransactionalSettings::deserialize(const QString &name, QVariantHash &data) +QString TransactionalSettings::deserialize(const QString &name, QVariantHash &data) const { SettingsPtr settings = Profile::instance().applicationSettings(name); @@ -294,7 +294,7 @@ QString TransactionalSettings::deserialize(const QString &name, QVariantHash &da return settings->fileName(); } -QString TransactionalSettings::serialize(const QString &name, const QVariantHash &data) +QString TransactionalSettings::serialize(const QString &name, const QVariantHash &data) const { SettingsPtr settings = Profile::instance().applicationSettings(name); for (auto i = data.begin(); i != data.end(); ++i) From 0a959bcbe7693f78a81fb4481bf54a0bf77f60dc Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Sun, 8 Sep 2019 14:40:18 +0800 Subject: [PATCH 3/3] Clean up SettingsStorage::save() Also it should return `true` when `m_dirty` is `false`. --- src/base/settingsstorage.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/base/settingsstorage.cpp b/src/base/settingsstorage.cpp index 4548335d9..85fdaad4f 100644 --- a/src/base/settingsstorage.cpp +++ b/src/base/settingsstorage.cpp @@ -186,18 +186,18 @@ SettingsStorage *SettingsStorage::instance() bool SettingsStorage::save() { - if (!m_dirty) return false; // Obtaining the lock is expensive, let's check early - const QWriteLocker locker(&m_lock); - if (!m_dirty) return false; // something might have changed while we were getting the lock + if (!m_dirty) return true; // Obtaining the lock is expensive, let's check early + const QWriteLocker locker(&m_lock); // to guard for `m_dirty` + if (!m_dirty) return true; // something might have changed while we were getting the lock const TransactionalSettings settings(QLatin1String("qBittorrent")); - if (settings.write(m_data)) { - m_dirty = false; - return true; + if (!settings.write(m_data)) { + m_timer.start(); + return false; } - m_timer.start(); - return false; + m_dirty = false; + return true; } QVariant SettingsStorage::loadValue(const QString &key, const QVariant &defaultValue) const