From 5fbb74832882d5f60d222d4ef07c6b4aba94800a Mon Sep 17 00:00:00 2001 From: Dan Paulat Date: Sat, 8 Apr 2023 01:06:56 -0500 Subject: [PATCH] Use weak_ptr to hold product manager records - When selecting a product that's expired, it successfully refreshes the data, but doesn't display unless selected again - When old data is downloaded, the refresh timer starts at 15 seconds, even if the newest data says the timer should be longer - Selecting a product should update the recent lists --- .../scwx/qt/manager/radar_product_manager.cpp | 127 ++++++++++++++---- wxdata/include/scwx/util/map.hpp | 30 ++++- 2 files changed, 124 insertions(+), 33 deletions(-) diff --git a/scwx-qt/source/scwx/qt/manager/radar_product_manager.cpp b/scwx-qt/source/scwx/qt/manager/radar_product_manager.cpp index 33b12b58..41741e99 100644 --- a/scwx-qt/source/scwx/qt/manager/radar_product_manager.cpp +++ b/scwx-qt/source/scwx/qt/manager/radar_product_manager.cpp @@ -37,8 +37,10 @@ static const auto logger_ = scwx::util::Logger::Create(logPrefix_); typedef std::function()> CreateNexradFileFunction; typedef std::map> + std::weak_ptr> RadarProductRecordMap; +typedef std::list> + RadarProductRecordList; static constexpr uint32_t NUM_RADIAL_GATES_0_5_DEGREE = common::MAX_0_5_DEGREE_RADIALS * common::MAX_DATA_MOMENT_GATES; @@ -124,7 +126,9 @@ public: coordinates0_5Degree_ {}, coordinates1Degree_ {}, level2ProductRecords_ {}, + level2ProductRecentRecords_ {}, level3ProductRecordsMap_ {}, + level3ProductRecentRecordsMap_ {}, level2ProductRecordMutex_ {}, level3ProductRecordMutex_ {}, level2ProviderManager_ {std::make_shared( @@ -179,6 +183,8 @@ public: std::chrono::system_clock::time_point time); std::shared_ptr StoreRadarProductRecord(std::shared_ptr record); + void UpdateRecentRecords(RadarProductRecordList& recentList, + std::shared_ptr record); void LoadProviderData(std::chrono::system_clock::time_point time, std::shared_ptr providerManager, @@ -201,10 +207,12 @@ public: std::vector coordinates0_5Degree_; std::vector coordinates1Degree_; - RadarProductRecordMap level2ProductRecords_; + RadarProductRecordMap level2ProductRecords_; + RadarProductRecordList level2ProductRecentRecords_; std::unordered_map level3ProductRecordsMap_; - + std::unordered_map + level3ProductRecentRecordsMap_; std::shared_mutex level2ProductRecordMutex_; std::shared_mutex level3ProductRecordMutex_; @@ -299,8 +307,9 @@ void RadarProductManager::DumpRecords() for (auto& record : radarProductManager->p->level2ProductRecords_) { - logger_->info(" {}", - scwx::util::TimeString(record.first)); + logger_->info(" {}{}", + scwx::util::TimeString(record.first), + record.second.expired() ? " (expired)" : ""); } } @@ -318,8 +327,10 @@ void RadarProductManager::DumpRecords() for (auto& record : recordMap.second) { - logger_->info(" {}", - scwx::util::TimeString(record.first)); + logger_->info(" {}{}", + scwx::util::TimeString(record.first), + record.second.expired() ? " (expired)" : + ""); } } } @@ -674,10 +685,13 @@ void RadarProductManagerImpl::LoadProviderData( auto it = recordMap.find(time); if (it != recordMap.cend()) { - logger_->debug( - "Data previously loaded, loading from data cache"); + existingRecord = it->second.lock(); - existingRecord = it->second; + if (existingRecord != nullptr) + { + logger_->debug( + "Data previously loaded, loading from data cache"); + } } } @@ -844,22 +858,28 @@ RadarProductManagerImpl::GetLevel2ProductRecord( std::chrono::system_clock::time_point time) { std::shared_ptr record; + RadarProductRecordMap::const_pointer recordPtr {nullptr}; if (!level2ProductRecords_.empty() && time == std::chrono::system_clock::time_point {}) { // If a default-initialized time point is given, return the latest record - record = level2ProductRecords_.rbegin()->second; + recordPtr = &(*level2ProductRecords_.rbegin()); } else { // TODO: Round to minutes - record = scwx::util::GetBoundedElementValue(level2ProductRecords_, time); + recordPtr = + scwx::util::GetBoundedElementPointer(level2ProductRecords_, time); + } - // Does the record contain the time we are looking for? - if (record != nullptr && (time < record->level2_file()->start_time())) + if (recordPtr != nullptr) + { + record = recordPtr->second.lock(); + if (record == nullptr) { - record = nullptr; + // Product is expired, reload it + self_->LoadLevel2Data(recordPtr->first, nullptr); } } @@ -871,6 +891,7 @@ RadarProductManagerImpl::GetLevel3ProductRecord( const std::string& product, std::chrono::system_clock::time_point time) { std::shared_ptr record = nullptr; + RadarProductRecordMap::const_pointer recordPtr {nullptr}; std::unique_lock lock {level3ProductRecordMutex_}; @@ -882,11 +903,24 @@ RadarProductManagerImpl::GetLevel3ProductRecord( { // If a default-initialized time point is given, return the latest // record - record = it->second.rbegin()->second; + recordPtr = &(*it->second.rbegin()); } else { - record = scwx::util::GetBoundedElementValue(it->second, time); + recordPtr = scwx::util::GetBoundedElementPointer(it->second, time); + } + } + + // Lock is no longer needed + lock.unlock(); + + if (recordPtr != nullptr) + { + record = recordPtr->second.lock(); + if (record == nullptr) + { + // Product is expired, reload it + self_->LoadLevel3Data(product, recordPtr->first, nullptr); } } @@ -899,7 +933,7 @@ RadarProductManagerImpl::StoreRadarProductRecord( { logger_->debug("StoreRadarProductRecord()"); - std::shared_ptr storedRecord = record; + std::shared_ptr storedRecord = nullptr; auto timeInSeconds = std::chrono::time_point_castdebug( - "Level 2 product previously loaded, loading from cache"); + storedRecord = it->second.lock(); - storedRecord = it->second; + if (storedRecord != nullptr) + { + logger_->debug( + "Level 2 product previously loaded, loading from cache"); + } } - else + + if (storedRecord == nullptr) { + storedRecord = record; level2ProductRecords_[timeInSeconds] = record; } + + UpdateRecentRecords(level2ProductRecentRecords_, storedRecord); } else if (record->radar_product_group() == common::RadarProductGroup::Level3) { @@ -931,20 +972,54 @@ RadarProductManagerImpl::StoreRadarProductRecord( auto it = productMap.find(timeInSeconds); if (it != productMap.cend()) { - logger_->debug( - "Level 3 product previously loaded, loading from cache"); + storedRecord = it->second.lock(); - storedRecord = it->second; + if (storedRecord != nullptr) + { + logger_->debug( + "Level 3 product previously loaded, loading from cache"); + } } - else + + if (storedRecord == nullptr) { + storedRecord = record; productMap[timeInSeconds] = record; } + + UpdateRecentRecords( + level3ProductRecentRecordsMap_[record->radar_product()], storedRecord); } return storedRecord; } +void RadarProductManagerImpl::UpdateRecentRecords( + RadarProductRecordList& recentList, + std::shared_ptr record) +{ + static constexpr std::size_t kRecentListMaxSize_ {2u}; + + auto it = std::find(recentList.cbegin(), recentList.cend(), record); + if (it != recentList.cbegin() && it != recentList.cend()) + { + // If the record exists beyond the front of the list, remove it + recentList.erase(it); + } + + if (recentList.size() == 0 || it != recentList.cbegin()) + { + // Add the record to the front of the list, unless it's already there + recentList.push_front(record); + } + + while (recentList.size() > kRecentListMaxSize_) + { + // Remove from the end of the list while it's too big + recentList.pop_back(); + } +} + std::tuple, float, std::vector> diff --git a/wxdata/include/scwx/util/map.hpp b/wxdata/include/scwx/util/map.hpp index 9aa2b6f0..3392b5d7 100644 --- a/wxdata/include/scwx/util/map.hpp +++ b/wxdata/include/scwx/util/map.hpp @@ -8,10 +8,10 @@ namespace scwx namespace util { -template> -ReturnType GetBoundedElement(std::map& map, Key key) +template::const_pointer> +ReturnType GetBoundedElementPointer(std::map& map, const Key& key) { - ReturnType element; + ReturnType elementPtr {nullptr}; // Find the first element greater than the key requested auto it = map.upper_bound(key); @@ -24,26 +24,42 @@ ReturnType GetBoundedElement(std::map& map, Key key) { // Get the element immediately preceding, this the element we are // looking for - element = (--it)->second; + elementPtr = &(*(--it)); } else { // The current element is a good substitute - element = it->second; + elementPtr = &(*it); } } else if (map.size() > 0) { // An element with a key greater was not found. If it exists, it must be // the last element. - element = map.rbegin()->second; + elementPtr = &(*map.rbegin()); + } + + return elementPtr; +} + +template> +ReturnType GetBoundedElement(std::map& map, const Key& key) +{ + ReturnType element; + + typename std::map::pointer elementPtr = + GetBoundedElementPointer::pointer>(map, + key); + if (elementPtr != nullptr) + { + element = elementPtr->second; } return element; } template -inline T GetBoundedElementValue(std::map& map, Key key) +inline T GetBoundedElementValue(std::map& map, const Key& key) { return GetBoundedElement(map, key); }