From 27786d709d911572b2439769bab50df3d2517648 Mon Sep 17 00:00:00 2001 From: Florent Rougon Date: Wed, 16 Aug 2017 19:34:23 +0200 Subject: [PATCH] EmbeddedResourceManager: better API with respect to rehashing Add a rehash() method to EmbeddedResourceManager::Impl to update 'poolSearchList'. Introduce a 'dirty' flag so that EmbeddedResourceManager::Impl::rehash() is automatically called whenever needed. It is not necessary anymore to call EmbeddedResourceManager::selectLocale() after adding resources: changing the EmbeddedResourceManager's locale or adding resources are both operations that set the dirty flag. Whenever someone tries to fetch a resource for the selected locale and the dirty flag is set, a rehash is triggered before the actual fetching so as to ensure it is correct. --- .../EmbeddedResourceManager.cxx | 32 ++++++++++++------- .../EmbeddedResourceManager.hxx | 12 +++---- .../EmbeddedResourceManager_private.hxx | 16 +++++++--- .../embedded_resources_test.cxx | 4 +-- 4 files changed, 40 insertions(+), 24 deletions(-) diff --git a/simgear/embedded_resources/EmbeddedResourceManager.cxx b/simgear/embedded_resources/EmbeddedResourceManager.cxx index 52690a5b..450550b2 100644 --- a/simgear/embedded_resources/EmbeddedResourceManager.cxx +++ b/simgear/embedded_resources/EmbeddedResourceManager.cxx @@ -46,8 +46,20 @@ static unique_ptr staticInstance; // * EmbeddedResourceManager::Impl * // *************************************************************************** EmbeddedResourceManager::Impl::Impl() + : dirty(true) { } +void +EmbeddedResourceManager::Impl::rehash() +{ + // Update the list of resource pools to search when looking up a resource. + // This allows to optimize resource lookup: no need to parse, split and hash + // the same locale string every time to find the corresponding resource + // pools. + poolSearchList = listOfResourcePoolsToSearch(selectedLocale); + dirty = false; +} + string EmbeddedResourceManager::Impl::getLocale() const { @@ -59,11 +71,7 @@ EmbeddedResourceManager::Impl::selectLocale(const std::string& locale) { string previousLocale = std::move(selectedLocale); selectedLocale = locale; - // Update the list of resource pools to search when looking up a resource. - // This allows to optimize resource lookup: no need to parse, split and hash - // the same locale string every time to find the corresponding resource - // pools. - poolSearchList = listOfResourcePoolsToSearch(selectedLocale); + dirty = true; return previousLocale; } @@ -157,6 +165,8 @@ EmbeddedResourceManager::Impl::addResource( "Virtual path already in use for " + localeDescr + " in the EmbeddedResourceManager: '" + virtualPath + "'"); } + + dirty = true; } // *************************************************************************** @@ -203,10 +213,10 @@ EmbeddedResourceManager::addResource( shared_ptr EmbeddedResourceManager::getResourceOrNullPtr(const string& virtualPath) const { - // Failure would indicate that either no resource has been added, or - // selectLocale() hasn't been called. Remember that selectLocale() must be - // called after all resources have been added. - assert(!p->poolSearchList.empty()); + if (p->dirty) { + p->rehash(); // update p->poolSearchList + } + // Use the selected locale return p->lookupResourceInPools(virtualPath, p->poolSearchList); } @@ -216,8 +226,8 @@ EmbeddedResourceManager::getResourceOrNullPtr(const string& virtualPath, const string& locale) const { // In this overload, we don't use the cached list of pools - // (p->poolSearchList), therefore this can be used to find a resource for - // any locale without any need to call selectLocale(). + // (p->poolSearchList), therefore there is no need to check the 'dirty' flag + // or to rehash(). return p->lookupResourceInPools(virtualPath, p->listOfResourcePoolsToSearch(locale)); } diff --git a/simgear/embedded_resources/EmbeddedResourceManager.hxx b/simgear/embedded_resources/EmbeddedResourceManager.hxx index d6658414..eb40475c 100644 --- a/simgear/embedded_resources/EmbeddedResourceManager.hxx +++ b/simgear/embedded_resources/EmbeddedResourceManager.hxx @@ -61,14 +61,12 @@ public: // Select the locale for which resources will be returned in the future, for // the getResourceOrNullPtr(), getResource(), getString(), getStreambuf() // and getIStream() overloads that don't have a 'locale' parameter. - // Return the previously-selected locale. + // May be called several times. Return the previously-selected locale. // - // This method *must* be called after all resources have been added. It can - // be called several times (typically, when the user changes the language - // for the whole application). If you just want to fetch one or two - // resources in a particular “locale” (language), it is simpler to use an - // overload of one of the getResourceOrNullPtr(), getResource(), ..., - // getIStream() methods that has a 'locale' parameter. + // If you just want to fetch one or two resources in a particular “locale” + // (language), it is simpler to use an overload of one of the + // getResourceOrNullPtr(), getResource(), ..., getIStream() methods that has + // a 'locale' parameter. std::string selectLocale(const std::string& locale); // Add a resource for the specified locale to the embedded resource manager. diff --git a/simgear/embedded_resources/EmbeddedResourceManager_private.hxx b/simgear/embedded_resources/EmbeddedResourceManager_private.hxx index dfacc7c9..80ef6aef 100644 --- a/simgear/embedded_resources/EmbeddedResourceManager_private.hxx +++ b/simgear/embedded_resources/EmbeddedResourceManager_private.hxx @@ -68,6 +68,10 @@ public: const std::string& virtualPath, const std::vector< std::shared_ptr >& poolSearchList); + // Recompute p->poolSearchList. This method is automatically called whenever + // needed (lazily), so it doesn't need to be part of the public interface. + void rehash(); + // Implement the corresponding EmbeddedResourceManager public methods std::string getLocale() const; std::string selectLocale(const std::string& locale); @@ -78,11 +82,15 @@ public: const std::string& locale); std::string selectedLocale; - // After each call to selectLocale(), we update this member to contain - // precisely the (ordered) list of pools to search for a resource in the - // selected “locale”. This allows relatively cheap resource lookups, - // assuming the desired “locale” doesn't change all the time. + // Each call to rehash() updates this member to contain precisely the + // (ordered) list of pools to search for a resource in the selected + // “locale”. This allows relatively cheap resource lookups, assuming the + // desired “locale” doesn't change all the time. std::vector< std::shared_ptr > poolSearchList; + // Indicate whether 'poolSearchList' must be updated (i.e., resources have + // been added or the selected locale was changed without rehash() being + // called afterwards). + bool dirty; // Maps each “locale name” to the corresponding resource pool. std::unordered_map< std::string, diff --git a/simgear/embedded_resources/embedded_resources_test.cxx b/simgear/embedded_resources/embedded_resources_test.cxx index 878cfa10..47bcd342 100644 --- a/simgear/embedded_resources/embedded_resources_test.cxx +++ b/simgear/embedded_resources/embedded_resources_test.cxx @@ -158,8 +158,8 @@ void initResources() new RawEmbeddedResource(res2frArray, sizeof(res2frArray) - 1)); resMgr->addResource("/path/to/resource2", std::move(res2fr), "fr"); - // This method must be called after all resources have been added. Here, we - // select the default locale (typically, English). + // Explicitly select the default locale (typically, English). This is for + // clarity, but isn't required. resMgr->selectLocale(""); }