diff --git a/map/traffic_manager.cpp b/map/traffic_manager.cpp index 796e30cd4..f89525740 100644 --- a/map/traffic_manager.cpp +++ b/map/traffic_manager.cpp @@ -54,6 +54,8 @@ auto constexpr kStorageUpdateInterval = minutes(1); auto constexpr kTrafficXMLFileName = "traffic.xml"; } // namespace +// TODO no longer needed +#ifdef traffic_dead_code TrafficManager::CacheEntry::CacheEntry() : m_isLoaded(false) // TODO no longer needed @@ -77,6 +79,7 @@ TrafficManager::CacheEntry::CacheEntry(time_point const & requestT , m_isWaitingForResponse(true) , m_lastAvailability(traffic::TrafficInfo::Availability::Unknown) {} +#endif TrafficManager::TrafficManager(DataSource & dataSource, CountryInfoGetterFn countryInfoGetter, const CountryParentNameGetterFn &countryParentNameGetter, @@ -193,7 +196,6 @@ void TrafficManager::Clear() LOG(LINFO, ("Messages in cache:", m_messageCache.size())); LOG(LINFO, ("Feeds in queue:", m_feedQueue.size())); LOG(LINFO, ("MWMs with coloring:", m_allMwmColoring.size())); - LOG(LINFO, ("MWM cache size:", m_mwmCache.size())); LOG(LINFO, ("Clearing...")); // TODO no longer needed #ifdef traffic_dead_code @@ -201,7 +203,10 @@ void TrafficManager::Clear() #endif m_messageCache.clear(); m_feedQueue.clear(); +// TODO no longer needed +#ifdef traffic_dead_code m_mwmCache.clear(); +#endif // TODO figure out which of the ones below we still need m_lastDrapeMwmsByRect.clear(); @@ -214,7 +219,6 @@ void TrafficManager::Clear() LOG(LINFO, ("Messages in cache:", m_messageCache.size())); LOG(LINFO, ("Feeds in queue:", m_feedQueue.size())); LOG(LINFO, ("MWMs with coloring:", m_allMwmColoring.size())); - LOG(LINFO, ("MWM cache size:", m_mwmCache.size())); } OnTrafficDataUpdate(); } @@ -318,7 +322,7 @@ void TrafficManager::OnChangeRoutingSessionState(routing::SessionState previous, { m_activeMwmsChanged = true; std::swap(mwms, m_activeRoutingMwms); - RequestTrafficData(); + RequestTrafficSubscription(); } } } @@ -405,7 +409,7 @@ void TrafficManager::UpdateActiveMwms(m2::RectD const & rect, if (mwm.IsAlive()) activeMwms.insert(mwm); } - RequestTrafficData(); + RequestTrafficSubscription(); } } @@ -835,40 +839,7 @@ bool TrafficManager::WaitForRequest() return true; } -void TrafficManager::RequestTrafficData(MwmSet::MwmId const & mwmId, bool force) -{ - bool needRequesting = false; - auto const currentTime = steady_clock::now(); - auto const it = m_mwmCache.find(mwmId); - if (it == m_mwmCache.end()) - { - needRequesting = true; - m_mwmCache.insert(std::make_pair(mwmId, CacheEntry(currentTime))); - } - else - { - auto const passedSeconds = currentTime - it->second.m_lastRequestTime; - if (passedSeconds >= kUpdateInterval || force) - { - needRequesting = true; - it->second.m_isWaitingForResponse = true; - it->second.m_lastRequestTime = currentTime; - } - if (!force) - it->second.m_lastActiveTime = currentTime; - } - - if (needRequesting) - { -// TODO no longer needed -#ifdef traffic_dead_code - m_requestedMwms.push_back(mwmId); -#endif - m_condition.notify_one(); - } -} - -void TrafficManager::RequestTrafficData() +void TrafficManager::RequestTrafficSubscription() { if ((m_activeDrapeMwms.empty() && m_activePositionMwms.empty() && m_activeRoutingMwms.empty()) || !IsEnabled() || IsInvalidState() || m_isPaused) @@ -878,9 +849,17 @@ void TrafficManager::RequestTrafficData() ForEachActiveMwm([this](MwmSet::MwmId const & mwmId) { ASSERT(mwmId.IsAlive(), ()); - RequestTrafficData(mwmId, false /* force */); +// TODO no longer needed +#ifdef traffic_dead_code + m_mwmCache.insert(mwmId); +#endif }); + m_condition.notify_one(); + +// TODO no longer needed +#ifdef traffic_dead_code UpdateState(); +#endif } // TODO no longer needed @@ -982,13 +961,12 @@ void TrafficManager::OnTrafficDataUpdate() /* * Much of this code is copied and pasted together from old MWM code, with some minor adaptations: * - * ForEachActiveMwm and the assertion (not the rest of the body) is from RequestTrafficData(); - * modification: cycle over all MWMs (active or not). + * ForEachActiveMwm and the assertion (not the rest of the body) is from RequestTrafficData() + * (now RequestTrafficSubscription()), modification: cycle over all MWMs (active or not). * trafficCache lookup is original code. * TrafficInfo construction is taken fron ThreadRoutine(), with modifications (different constructor). - * Updating m_mwmCache is from RequestTrafficData(MwmSet::MwmId const &, bool), with modifications. * The remainder of the loop is from OnTrafficDataResponse(traffic::TrafficInfo &&), with some modifications - * (deciding whether to notify a component and managing timestamps is original code). + * (removed CacheEntry logic; deciding whether to notify a component and managing timestamps is original code). * Existing coloring deletion (if there is no new coloring) is original code. */ ForEachMwm([this, notifyDrape, notifyObserver](std::shared_ptr info) { @@ -1007,42 +985,34 @@ void TrafficManager::OnTrafficDataUpdate() LOG(LINFO, ("Setting new coloring for", mwmId, "with", coloring.size(), "entries")); traffic::TrafficInfo info(mwmId, std::move(coloring)); - m_mwmCache.try_emplace(mwmId, CacheEntry(steady_clock::now())); +// TODO no longer needed +#ifdef traffic_dead_code + UpdateState(); +#endif - auto it = m_mwmCache.find(mwmId); - if (it != m_mwmCache.end()) + if (notifyDrape) { - it->second.m_isLoaded = true; - it->second.m_lastResponseTime = steady_clock::now(); - it->second.m_isWaitingForResponse = false; - it->second.m_lastAvailability = info.GetAvailability(); + /* + * TODO calling ClearTrafficCache before UpdateTraffic is a workaround for a bug in the + * Drape engine: some segments found in the old coloring but not in the new one may get + * left behind. This was not a problem for MapsWithMe as the set of segments never + * changed, but is an issue wherever the segment set is dynamic. Workaround is to clear + * before sending an update. Ultimately, the processing logic for UpdateTraffic needs to + * be fixed, but the code is hard to read (involves multiple messages getting thrown back + * and forth between threads). + */ + m_drapeEngine.SafeCall(&df::DrapeEngine::ClearTrafficCache, + static_cast(mwmId)); + m_drapeEngine.SafeCall(&df::DrapeEngine::UpdateTraffic, + static_cast(info)); + m_lastDrapeUpdate = steady_clock::now(); + } - UpdateState(); - - if (notifyDrape) - { - /* - * TODO calling ClearTrafficCache before UpdateTraffic is a workaround for a bug in the - * Drape engine: some segments found in the old coloring but not in the new one may get - * left behind. This was not a problem for MapsWithMe as the set of segments never - * changed, but is an issue wherever the segment set is dynamic. Workaround is to clear - * before sending an update. Ultimately, the processing logic for UpdateTraffic needs to - * be fixed, but the code is hard to read (involves multiple messages getting thrown back - * and forth between threads). - */ - m_drapeEngine.SafeCall(&df::DrapeEngine::ClearTrafficCache, - static_cast(mwmId)); - m_drapeEngine.SafeCall(&df::DrapeEngine::UpdateTraffic, - static_cast(info)); - m_lastDrapeUpdate = steady_clock::now(); - } - - if (notifyObserver) - { - // Update traffic colors for routing. - m_routingSession.OnTrafficInfoAdded(std::move(info)); - m_lastObserverUpdate = steady_clock::now(); - } + if (notifyObserver) + { + // Update traffic colors for routing. + m_routingSession.OnTrafficInfoAdded(std::move(info)); + m_lastObserverUpdate = steady_clock::now(); } } else @@ -1181,6 +1151,8 @@ bool TrafficManager::IsInvalidState() const return m_state == TrafficState::NetworkError; } +// TODO no longer needed +#ifdef traffic_dead_code void TrafficManager::UpdateState() { if (!IsEnabled() || IsInvalidState()) @@ -1238,6 +1210,7 @@ void TrafficManager::UpdateState() else ChangeState(TrafficState::Enabled); } +#endif void TrafficManager::ChangeState(TrafficState newState) { diff --git a/map/traffic_manager.hpp b/map/traffic_manager.hpp index 2118c2fb2..267259e34 100644 --- a/map/traffic_manager.hpp +++ b/map/traffic_manager.hpp @@ -46,6 +46,7 @@ public: /** * @brief Global state of traffic information. */ + // TODO apart from `Disabled` and `Enabled`, all states are obsolete enum class TrafficState { /** Traffic is disabled, no traffic data will be retrieved or considered for routing. */ @@ -294,6 +295,8 @@ public: void Clear(); private: +// TODO no longer needed +#ifdef traffic_dead_code /** * @brief Holds information about pending or previous traffic requests pertaining to an MWM. */ @@ -307,13 +310,10 @@ private: */ bool m_isLoaded; -// TODO no longer needed -#ifdef traffic_dead_code /** * @brief The amount of memory occupied by the coloring for this MWM. */ size_t m_dataSize; -#endif /** * @brief When the last update request occurred, not including forced updates. @@ -352,6 +352,7 @@ private: traffic::TrafficInfo::Availability m_lastAvailability; }; +#endif /** * @brief Ensures every TraFF source has a subscription covering all currently active MWMs. @@ -466,7 +467,7 @@ private: * If the MWM is no longer active, this method returns immediately after that. * * If the retry limit has not been reached, the MWM is re-inserted into the list by calling - * `RequestTrafficData(MwmSet::MwmId, bool)` with `force` set to true. Otherwise, the retry count + * `RequestTrafficSubscription(MwmSet::MwmId, bool)` with `force` set to true. Otherwise, the retry count * is reset and the state updated accordingly. * * @param info @@ -498,12 +499,9 @@ private: // This is a group of methods that haven't their own synchronization inside. /** - * @brief Requests a refresh of traffic data for all currently active MWMs. + * @brief Requests a refresh of traffic subscriptions to match all currently active MWMs. * - * This method is the entry point for periodic traffic data refresh operations. It cycles through - * all active MWMs and calls `RequestTrafficData(MwmSet::MwmId, bool)` on each `MwmId`, - * scheduling a refresh if needed. The actual network operation is performed asynchronously on a - * separate thread. + * The actual call to the TraFF sources is performed asynchronously on a separate thread. * * The method does nothing if the `TrafficManager` instance is disabled, paused, in an invalid * state (`NetworkError`) or if neither the rendering engine nor the routing engine have any @@ -511,24 +509,9 @@ private: * * This method is unsynchronized; the caller must lock `m_mutex` prior to calling it. */ - void RequestTrafficData(); + void RequestTrafficSubscription(); - /** - * @brief Requests a refresh of traffic data for a single MWM. - * - * This method first checks if traffic data for the given MWM needs to be refreshed, which is the - * case if no traffic data has ever been fetched for the given MWM, the update interval has - * expired or `force` is true. In that case, the method inserts the `mwmId` into the list of MWMs - * for which to update traffic and wakes up the worker thread. - * - * This method is unsynchronized; the caller must lock `m_mutex` prior to calling it. - * - * @param mwmId Describes the MWM for which traffic data is to be refreshed. - * @param force If true, a refresh is requested even if the update interval has not expired. - */ - void RequestTrafficData(MwmSet::MwmId const & mwmId, bool force); - - // TODO no longer needed +// TODO no longer needed #ifdef traffic_dead_code /** * @brief Removes traffic data for one specific MWM from the cache. @@ -543,7 +526,6 @@ private: */ void ClearCache(MwmSet::MwmId const & mwmId); void ShrinkCacheToAllowableSize(); -#endif /** * @brief Updates the state of the traffic manager based on the state of all MWMs used by the renderer. @@ -558,6 +540,7 @@ private: * `TrafficState::NoData`, `TrafficState::Outdated`, `TrafficState::Enabled`. */ void UpdateState(); +#endif void ChangeState(TrafficState newState); bool IsInvalidState() const; @@ -638,9 +621,9 @@ private: #ifdef traffic_dead_code size_t m_maxCacheSizeBytes; size_t m_currentCacheSizeBytes = 0; -#endif std::map m_mwmCache; +#endif /** * @brief The TraFF sources from which we get traffic information. @@ -666,9 +649,9 @@ private: * * Methods which use only the set: * - * * RequestTrafficData(), exits if empty, otherwise cycles through the set. + * * RequestTrafficSubscription(), exits if empty, otherwise cycles through the set. * * OnTrafficRequestFailed(), determines if an MWM is still active and the request should be retried. - * * UniteActiveMwms(), build the list of active MWMs (used by RequestTrafficData() or to shrink the cache). + * * UniteActiveMwms(), build the list of active MWMs (used by RequestTrafficSubscription() or to shrink the cache). * * UpdateState(), cycles through the set to determine the state of traffic requests (renderer only). * * Methods which use both, but in a different way: