sc/inc/document.hxx | 5 +- sc/inc/interpretercontext.hxx | 4 -- sc/inc/rangecache.hxx | 5 -- sc/source/core/data/documen2.cxx | 49 +++++++++++++++-------------- sc/source/core/tool/interpretercontext.cxx | 12 ------- sc/source/core/tool/rangecache.cxx | 4 -- 6 files changed, 33 insertions(+), 46 deletions(-)
New commits: commit 1ac3b4ae83856eebe6bc329b7a92575b6b1d84be Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Mon Jun 13 20:46:08 2022 +0200 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Thu Jun 16 15:12:42 2022 +0200 share ScSortedRangeCache between threads It's the same data for all threads, they access it as read-only, so it doesn't make sense for each thread to build its own copy. Change-Id: Ia1559c61d976bcce78661cae7e030bb8430aed7c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/135794 Tested-by: Jenkins Reviewed-by: Luboš Luňák <l.lu...@collabora.com> diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx index 7bd27c6b95fa..294d7deadecc 100644 --- a/sc/inc/document.hxx +++ b/sc/inc/document.hxx @@ -48,7 +48,7 @@ #include <cassert> #include <memory> #include <map> -#include <mutex> +#include <shared_mutex> #include <optional> #include <set> #include <unordered_map> @@ -471,7 +471,8 @@ private: mutable ScInterpreterContext maInterpreterContext; - std::mutex mScLookupMutex; // protection for thread-unsafe parts of handling ScLookup + std::shared_mutex mScLookupMutex; // protection for thread-unsafe parts of handling ScLookup + std::unique_ptr<ScSortedRangeCacheMap> mxScSortedRangeCache; // cache for unsorted lookups static const sal_uInt16 nSrcVer; // file version (load/save) sal_uInt16 nFormulaTrackCount; diff --git a/sc/inc/interpretercontext.hxx b/sc/inc/interpretercontext.hxx index 9f424b1a4f42..4ecb1ed9de1a 100644 --- a/sc/inc/interpretercontext.hxx +++ b/sc/inc/interpretercontext.hxx @@ -23,7 +23,6 @@ class FormulaToken; class ScDocument; class SvNumberFormatter; struct ScLookupCacheMap; -struct ScSortedRangeCacheMap; class ScInterpreter; enum class SvNumFormatType : sal_Int16; @@ -58,7 +57,6 @@ struct ScInterpreterContext std::vector<formula::FormulaToken*> maTokens; std::vector<DelayedSetNumberFormat> maDelayedSetNumberFormat; std::unique_ptr<ScLookupCacheMap> mxScLookupCache; // cache for lookups like VLOOKUP and MATCH - std::unique_ptr<ScSortedRangeCacheMap> mxScSortedRangeCache; // cache for unsorted lookups // Allocation cache for "aConditions" array in ScInterpreter::IterateParameterIfs() // This is populated/used only when formula-group threading is enabled. std::vector<sal_uInt8> maConditions; @@ -85,7 +83,6 @@ private: void SetDocAndFormatter(const ScDocument& rDoc, SvNumberFormatter* pFormatter); void Cleanup(); void ClearLookupCache(); - void ClearSortedRangeCache(); void initFormatTable(); SvNumberFormatter* mpFormatter; mutable NFIndexAndFmtType maNFTypeCache; @@ -139,7 +136,6 @@ class ScInterpreterContextPool public: // Only to be used to clear lookup cache in all pool elements static void ClearLookupCaches(); - static void ClearSortedRangeCaches(); }; class ScThreadedInterpreterContextGetterGuard diff --git a/sc/inc/rangecache.hxx b/sc/inc/rangecache.hxx index fd5b12cbd8de..048d9b9ba046 100644 --- a/sc/inc/rangecache.hxx +++ b/sc/inc/rangecache.hxx @@ -45,7 +45,7 @@ class ScSortedRangeCache final : public SvtListener public: /// MUST be new'd because Notify() deletes. ScSortedRangeCache(ScDocument* pDoc, const ScRange& rRange, const ScQueryParam& param, - ScSortedRangeCacheMap& cacheMap, ScInterpreterContext* context); + ScInterpreterContext* context); /// Remove from document structure and delete (!) cache on modify hint. virtual void Notify(const SfxHint& rHint) override; @@ -53,8 +53,6 @@ public: const ScRange& getRange() const { return maRange; } bool isDescending() const { return mDescending; } - ScSortedRangeCacheMap& getCacheMap() const { return mCacheMap; } - enum class ValueType { Values, @@ -102,7 +100,6 @@ private: std::vector<size_t> mRowToIndex; // indexed by 'SCROW - maRange.aStart.Row()' ScRange maRange; ScDocument* mpDoc; - ScSortedRangeCacheMap& mCacheMap; bool mDescending; ValueType mValues; diff --git a/sc/source/core/data/documen2.cxx b/sc/source/core/data/documen2.cxx index bcdbd9b586e9..ed090885ed3b 100644 --- a/sc/source/core/data/documen2.cxx +++ b/sc/source/core/data/documen2.cxx @@ -137,6 +137,7 @@ ScDocument::ScDocument( ScDocumentMode eMode, SfxObjectShell* pDocShell ) : nMacroInterpretLevel(0), nInterpreterTableOpLevel(0), maInterpreterContext( *this, nullptr ), + mxScSortedRangeCache(new ScSortedRangeCacheMap), nFormulaTrackCount(0), eHardRecalcState(HardRecalcState::OFF), nVisibleTab( 0 ), @@ -1198,25 +1199,31 @@ ScLookupCache & ScDocument::GetLookupCache( const ScRange & rRange, ScInterprete ScSortedRangeCache& ScDocument::GetSortedRangeCache( const ScRange & rRange, const ScQueryParam& param, ScInterpreterContext* pContext ) { - ScSortedRangeCache* pCache = nullptr; - if (!pContext->mxScSortedRangeCache) - pContext->mxScSortedRangeCache.reset(new ScSortedRangeCacheMap); - ScSortedRangeCacheMap* pCacheMap = pContext->mxScSortedRangeCache.get(); - // insert with temporary value to avoid doing two lookups - auto [findIt, bInserted] = pCacheMap->aCacheMap.emplace(ScSortedRangeCache::makeHashKey(rRange, param), nullptr); + assert(mxScSortedRangeCache); + ScSortedRangeCache::HashKey key = ScSortedRangeCache::makeHashKey(rRange, param); + // This should be created just once for one range, and repeated calls should reuse it, even + // between threads (it doesn't make sense to use ScInterpreterContext and have all threads + // build their own copy of the same data). So first try read-only access, which should + // in most cases be enough. + { + std::shared_lock guard(mScLookupMutex); + auto findIt = mxScSortedRangeCache->aCacheMap.find(key); + if( findIt != mxScSortedRangeCache->aCacheMap.end()) + return *findIt->second; + } + // Avoid recursive calls because of some cells in the range being dirty and triggering + // interpreting, which may call into this again. Threaded calculation makes sure + // no cells are dirty. + if(!IsThreadedGroupCalcInProgress()) + InterpretCellsIfNeeded(rRange); + std::unique_lock guard(mScLookupMutex); + auto [findIt, bInserted] = mxScSortedRangeCache->aCacheMap.emplace(key, nullptr); if (bInserted) { - findIt->second = std::make_unique<ScSortedRangeCache>(this, rRange, param, *pCacheMap, pContext); - pCache = findIt->second.get(); - // The StartListeningArea() call is not thread-safe, as all threads - // would access the same SvtBroadcaster. - std::unique_lock guard( mScLookupMutex ); - StartListeningArea(rRange, false, pCache); + findIt->second = std::make_unique<ScSortedRangeCache>(this, rRange, param, pContext); + StartListeningArea(rRange, false, findIt->second.get()); } - else - pCache = (*findIt).second.get(); - - return *pCache; + return *findIt->second; } void ScDocument::RemoveLookupCache( ScLookupCache & rCache ) @@ -1244,12 +1251,11 @@ void ScDocument::RemoveSortedRangeCache( ScSortedRangeCache & rCache ) // a result of user input or recalc). If it turns out this can be the case, locking is needed // here and also in ScSortedRangeCache::Notify(). assert(!IsThreadedGroupCalcInProgress()); - auto & cacheMap = rCache.getCacheMap(); - auto it(cacheMap.aCacheMap.find(rCache.getHashKey())); - if (it != cacheMap.aCacheMap.end()) + auto it(mxScSortedRangeCache->aCacheMap.find(rCache.getHashKey())); + if (it != mxScSortedRangeCache->aCacheMap.end()) { ScSortedRangeCache* pCache = (*it).second.release(); - cacheMap.aCacheMap.erase(it); + mxScSortedRangeCache->aCacheMap.erase(it); assert(!IsThreadedGroupCalcInProgress()); // EndListeningArea() is not thread-safe EndListeningArea(pCache->getRange(), false, &rCache); return; @@ -1261,10 +1267,9 @@ void ScDocument::ClearLookupCaches() { assert(!IsThreadedGroupCalcInProgress()); GetNonThreadedContext().mxScLookupCache.reset(); - GetNonThreadedContext().mxScSortedRangeCache.reset(); + mxScSortedRangeCache->aCacheMap.clear(); // Clear lookup cache in all interpreter-contexts in the (threaded/non-threaded) pools. ScInterpreterContextPool::ClearLookupCaches(); - ScInterpreterContextPool::ClearSortedRangeCaches(); } bool ScDocument::IsCellInChangeTrack(const ScAddress &cell,Color *pColCellBorder) diff --git a/sc/source/core/tool/interpretercontext.cxx b/sc/source/core/tool/interpretercontext.cxx index c35b6a602fd2..0df4a8407c9c 100644 --- a/sc/source/core/tool/interpretercontext.cxx +++ b/sc/source/core/tool/interpretercontext.cxx @@ -64,7 +64,7 @@ void ScInterpreterContext::initFormatTable() void ScInterpreterContext::Cleanup() { - // Do not disturb mxScLookupCache/mxScSortedRangeCache + // Do not disturb mxScLookupCache. maConditions.clear(); maDelayedSetNumberFormat.clear(); ResetTokens(); @@ -72,8 +72,6 @@ void ScInterpreterContext::Cleanup() void ScInterpreterContext::ClearLookupCache() { mxScLookupCache.reset(); } -void ScInterpreterContext::ClearSortedRangeCache() { mxScSortedRangeCache.reset(); } - SvNumFormatType ScInterpreterContext::GetNumberFormatType(sal_uInt32 nFIndex) const { if (!mpDoc->IsThreadedGroupCalcInProgress()) @@ -167,14 +165,6 @@ void ScInterpreterContextPool::ClearLookupCaches() rPtr->ClearLookupCache(); } -void ScInterpreterContextPool::ClearSortedRangeCaches() -{ - for (auto& rPtr : aThreadedInterpreterPool.maPool) - rPtr->ClearSortedRangeCache(); - for (auto& rPtr : aNonThreadedInterpreterPool.maPool) - rPtr->ClearSortedRangeCache(); -} - /* ScThreadedInterpreterContextGetterGuard */ ScThreadedInterpreterContextGetterGuard::ScThreadedInterpreterContextGetterGuard( diff --git a/sc/source/core/tool/rangecache.cxx b/sc/source/core/tool/rangecache.cxx index 79bebcfa8fe6..6a80ca786082 100644 --- a/sc/source/core/tool/rangecache.cxx +++ b/sc/source/core/tool/rangecache.cxx @@ -52,11 +52,9 @@ static ScSortedRangeCache::ValueType toValueType(const ScQueryParam& param) } ScSortedRangeCache::ScSortedRangeCache(ScDocument* pDoc, const ScRange& rRange, - const ScQueryParam& param, ScSortedRangeCacheMap& cacheMap, - ScInterpreterContext* context) + const ScQueryParam& param, ScInterpreterContext* context) : maRange(rRange) , mpDoc(pDoc) - , mCacheMap(cacheMap) , mDescending(needsDescending(param)) , mValues(toValueType(param)) {