From c6a038aaa8083c928179ec57ae043a21e5bd768b Mon Sep 17 00:00:00 2001 From: Mathew Sutcliffe Date: Thu, 10 Mar 2016 00:09:14 +0000 Subject: [PATCH] refs #601 Simplify syncLoad and rename to synchronize. Now it doesn't return anything, it just causes the next async get() to be synchronized with the latest loaded value. It does this by hooking into the queue introduced in the previous commit. --- src/blackmisc/datacache.cpp | 54 +++++++++++++++++++++--------------- src/blackmisc/datacache.h | 32 +++++++++++---------- src/blackmisc/valuecache.cpp | 8 ++++++ src/blackmisc/valuecache.h | 8 ++++-- 4 files changed, 64 insertions(+), 38 deletions(-) diff --git a/src/blackmisc/datacache.cpp b/src/blackmisc/datacache.cpp index ea8bd5c2a..7bf99e513 100644 --- a/src/blackmisc/datacache.cpp +++ b/src/blackmisc/datacache.cpp @@ -92,19 +92,15 @@ namespace BlackMisc return enumerateFiles(persistentStore()); } - std::future CDataCache::syncLoad(QObject *pageOwner, const QString &key) + bool CDataCache::synchronize(const QString &key) { - auto future = m_revision.promiseLoadedValue(pageOwner, key, getTimestampSync(key)); + auto future = m_revision.promiseLoadedValue(key, getTimestampSync(key)); if (future.valid()) { - return future; - } - else // value is not awaiting load, so immediately return the current value - { - std::promise p; - p.set_value(getValueSync(key)); - return p.get_future(); + future.wait(); + return true; } + return false; } void CDataCache::setTimeToLive(const QString &key, int ttl) @@ -186,6 +182,25 @@ namespace BlackMisc } } + void CDataPageQueue::setQueuedValueFromCache(const QString &key) + { + QMutexLocker lock(&m_mutex); + + decltype(m_queue) filtered; + for (auto &pair : m_queue) + { + if (pair.first.contains(key)) + { + filtered.push_back({ pair.first.takeByKey(key), pair.second }); + } + } + lock.unlock(); + for (const auto &pair : filtered) + { + m_page->setValuesFromCache(pair.first, pair.second); + } + } + CDataCacheSerializer::CDataCacheSerializer(CDataCache *owner, const QString &revisionFileName) : CContinuousWorker(owner), m_cache(owner), @@ -236,19 +251,14 @@ namespace BlackMisc } } - void CDataCacheSerializer::deliverPromises(std::vector>> i_promises) + void CDataCacheSerializer::deliverPromises(std::vector> i_promises) { - auto changes = m_deferredChanges; auto promises = std::make_shared(std::move(i_promises)); // \todo use C++14 lambda init-capture - QTimer::singleShot(0, Qt::PreciseTimer, this, [this, changes, promises] + QTimer::singleShot(0, Qt::PreciseTimer, this, [this, promises] { - for (auto &tuple : *promises) + for (auto &promise : *promises) { - QString key; - std::promise promise; - std::tie(std::ignore, key, promise) = std::move(tuple); - - promise.set_value(changes.value(key).first); + promise.set_value(); } }); } @@ -391,21 +401,21 @@ namespace BlackMisc return (m_updateInProgress || beginUpdate({{ key, timestamp }}, false)) && m_timestamps.contains(key); } - std::future CDataCacheRevision::promiseLoadedValue(QObject *pageOwner, const QString &key, qint64 currentTimestamp) + std::future CDataCacheRevision::promiseLoadedValue(const QString &key, qint64 currentTimestamp) { QMutexLocker lock(&m_mutex); if (isNewerValueAvailable(key, currentTimestamp)) { - std::promise promise; + std::promise promise; auto future = promise.get_future(); - m_promises.emplace_back(pageOwner, key, std::move(promise)); + m_promises.push_back(std::move(promise)); return future; } return {}; } - std::vector>> CDataCacheRevision::loadedValuePromises() + std::vector> CDataCacheRevision::loadedValuePromises() { QMutexLocker lock(&m_mutex); diff --git a/src/blackmisc/datacache.h b/src/blackmisc/datacache.h index 3a8a9b487..a37dea840 100644 --- a/src/blackmisc/datacache.h +++ b/src/blackmisc/datacache.h @@ -43,6 +43,9 @@ namespace BlackMisc //! Synchronize with changes queued by queueValuesFromCache, if the mutex is not currently locked. void trySetQueuedValuesFromCache(); + //! Synchronize with one specific change in the queue, leave the rest for later. + void setQueuedValueFromCache(const QString &key); + private: CValuePage *m_page = nullptr; QList> m_queue; @@ -96,10 +99,10 @@ namespace BlackMisc bool isNewerValueAvailable(const QString &key, qint64 timestamp); //! Return a future which will be made ready when the value is loaded. Future is invalid if value is not loading. - std::future promiseLoadedValue(QObject *pageOwner, const QString &key, qint64 currentTimestamp); + std::future promiseLoadedValue(const QString &key, qint64 currentTimestamp); //! Returns (by move) the container of promises to load values. - std::vector>> loadedValuePromises(); + std::vector> loadedValuePromises(); //! Set TTL value that will be written to the revision file. void setTimeToLive(const QString &key, int ttl); @@ -117,7 +120,7 @@ namespace BlackMisc QUuid m_uuid; QMap m_timestamps; QMap m_timesToLive; - std::vector>> m_promises; + std::vector> m_promises; static QJsonObject toJson(const QMap ×tamps); static QMap fromJson(const QJsonObject ×tamps); @@ -152,7 +155,7 @@ namespace BlackMisc private: const QString &persistentStore() const; void applyDeferredChanges(); - void deliverPromises(std::vector>>); + void deliverPromises(std::vector>); CDataCache *const m_cache = nullptr; QUuid m_revision; @@ -182,8 +185,8 @@ namespace BlackMisc //! Return all files where data may be stored. QStringList enumerateStore() const; - //! Method used for implementing CData::syncLoad. - std::future syncLoad(QObject *pageOwner, const QString &key); + //! Method used for implementing CData::synchronize. + bool synchronize(const QString &key); //! Method used for implementing TTL. void setTimeToLive(const QString &key, int ttl); @@ -225,8 +228,7 @@ namespace BlackMisc //! Must be a void, non-const member function of the owner. template CData(T *owner, NotifySlot slot = nullptr) : - CData::CCached(CDataCache::instance(), Trait::key(), Trait::isValid, Trait::defaultValue(), owner, slot), - m_owner(owner) + CData::CCached(CDataCache::instance(), Trait::key(), Trait::isValid, Trait::defaultValue(), owner, slot) { if (Trait::timeToLive() >= 0) { CDataCache::instance()->setTimeToLive(Trait::key(), Trait::timeToLive()); } } @@ -243,15 +245,17 @@ namespace BlackMisc //! Don't change the value, but write a new timestamp, to extend the life of the value. void renewTimestamp(qint64 timestamp) { return CDataCache::instance()->renewTimestamp(this->getKey(), timestamp); } - //! Return a future providing the value. If the value is still loading, the future will wait for it. - //! If the value is not present, the variant is null. Bypasses async get and inhibits notification slot. - std::future syncLoad() { return CDataCache::instance()->syncLoad(m_owner, this->getKey()); } + //! If the value is currently being loaded, wait for it to finish loading, and call the notification slot, if any. + void synchronize() + { + auto *queue = this->m_page.template findChild(); + Q_ASSERT(queue); + CDataCache::instance()->synchronize(this->getKey()); + queue->setQueuedValueFromCache(this->getKey()); + } //! Data cache doesn't support setAndSave (because set() already causes save anyway). CStatusMessage setAndSave(const typename Trait::type &value, qint64 timestamp = 0) = delete; - - private: - QObject *m_owner = nullptr; }; /*! diff --git a/src/blackmisc/valuecache.cpp b/src/blackmisc/valuecache.cpp index 056602173..90240e1cb 100644 --- a/src/blackmisc/valuecache.cpp +++ b/src/blackmisc/valuecache.cpp @@ -74,6 +74,14 @@ namespace BlackMisc return result; } + CValueCachePacket CValueCachePacket::takeByKey(const QString &key) + { + auto copy = *this; + remove(key); + copy.removeByKeyIf([ = ](const QString &key2) { return key2 != key; }); + return copy; + } + void CValueCachePacket::registerMetadata() { MetaType::registerMetadata(); diff --git a/src/blackmisc/valuecache.h b/src/blackmisc/valuecache.h index 98abee759..e27fe84cf 100644 --- a/src/blackmisc/valuecache.h +++ b/src/blackmisc/valuecache.h @@ -60,6 +60,9 @@ namespace BlackMisc //! Discard values and return as map of timestamps. QMap toTimestampMap() const; + //! Remove value matching the given key, and return it in a separate packet. + CValueCachePacket takeByKey(const QString &key); + //! \copydoc BlackMisc::Mixin::MetaType::registerMetadata static void registerMetadata(); @@ -330,8 +333,9 @@ namespace BlackMisc QVariant getVariantCopy() const { return m_page.getValueCopy(m_element).getQVariant(); } bool isValid() const { return m_page.isValid(m_element, qMetaTypeId()); } - Private::CValuePage &m_page; - Private::CValuePage::Element &m_element; + protected: + Private::CValuePage &m_page; //!< \private + Private::CValuePage::Element &m_element; //!< \private }; /*!