From 16a338e173083954a9932a3a4005f172309c784e Mon Sep 17 00:00:00 2001 From: Jan-Marek Glogowski Date: Fri, 5 Oct 2018 17:08:49 +0200 Subject: [PATCH] Convert ImplFontCache to use o3tl::lru_map We still do our own cleanup of the LRU map, as we can't drop any fonts in use. Change-Id: I8ec5c6ce8f80893635621357e9085950e7010f5b Reviewed-on: https://gerrit.libreoffice.org/61455 Tested-by: Jenkins Reviewed-by: Jan-Marek Glogowski --- include/o3tl/lru_map.hxx | 7 +++--- o3tl/qa/test-lru_map.cxx | 20 ++++++++++++++++++ vcl/inc/impfontcache.hxx | 3 ++- vcl/source/font/fontcache.cxx | 40 +++++++++++++++++------------------ 4 files changed, 46 insertions(+), 24 deletions(-) diff --git a/include/o3tl/lru_map.hxx b/include/o3tl/lru_map.hxx index 79e81293a858..003da59551b5 100644 --- a/include/o3tl/lru_map.hxx +++ b/include/o3tl/lru_map.hxx @@ -31,7 +31,7 @@ namespace o3tl * for most of the operations with a combination unordered map and linked list. * **/ -template> +template, class KeyEqual = std::equal_to> class lru_map final { public: @@ -42,7 +42,7 @@ private: typedef typename list_t::iterator list_iterator_t; typedef typename list_t::const_iterator list_const_iterator_t; - typedef std::unordered_map map_t; + typedef std::unordered_map map_t; typedef typename map_t::iterator map_iterator_t; typedef typename map_t::const_iterator map_const_iterator_t; @@ -65,8 +65,9 @@ public: typedef list_iterator_t iterator; typedef list_const_iterator_t const_iterator; + // a size of 0 effectively disables the LRU cleanup code lru_map(size_t nMaxSize) - : mMaxSize(nMaxSize) + : mMaxSize(nMaxSize ? nMaxSize : std::min(mLruMap.max_size(), mLruList.max_size())) {} void insert(key_value_pair_t& rPair) diff --git a/o3tl/qa/test-lru_map.cxx b/o3tl/qa/test-lru_map.cxx index 7a4886d3be3e..a7f9777e2c5f 100644 --- a/o3tl/qa/test-lru_map.cxx +++ b/o3tl/qa/test-lru_map.cxx @@ -28,6 +28,7 @@ public: void testLruRemoval(); void testCustomHash(); void testRemoveIf(); + void testNoAutoCleanup(); CPPUNIT_TEST_SUITE(lru_map_test); CPPUNIT_TEST(testBaseUsage); @@ -36,6 +37,7 @@ public: CPPUNIT_TEST(testLruRemoval); CPPUNIT_TEST(testCustomHash); CPPUNIT_TEST(testRemoveIf); + CPPUNIT_TEST(testNoAutoCleanup); CPPUNIT_TEST_SUITE_END(); }; @@ -289,6 +291,24 @@ void lru_map_test::testRemoveIf() CPPUNIT_ASSERT_EQUAL(size_t(0), lru.size()); } +void lru_map_test::testNoAutoCleanup() +{ + o3tl::lru_map lru(0); + CPPUNIT_ASSERT_EQUAL(size_t(0), lru.size()); + lru.insert({0,0}); + lru.insert({1,1}); + CPPUNIT_ASSERT_EQUAL(size_t(2), lru.size()); + lru.insert({0,0}); + CPPUNIT_ASSERT_EQUAL(size_t(2), lru.size()); + + int i = 0; + for (auto &rPair : lru) + { + CPPUNIT_ASSERT_EQUAL(i, rPair.first); + ++i; + } +} + CPPUNIT_TEST_SUITE_REGISTRATION(lru_map_test); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/vcl/inc/impfontcache.hxx b/vcl/inc/impfontcache.hxx index c96994300176..ee39883f2199 100644 --- a/vcl/inc/impfontcache.hxx +++ b/vcl/inc/impfontcache.hxx @@ -69,7 +69,8 @@ private: // cache of recently used font instances struct IFSD_Equal { bool operator()( const FontSelectPattern&, const FontSelectPattern& ) const; }; struct IFSD_Hash { size_t operator()( const FontSelectPattern& ) const; }; - typedef std::unordered_map, IFSD_Hash, IFSD_Equal> FontInstanceList; + typedef o3tl::lru_map, IFSD_Hash, IFSD_Equal> FontInstanceList; + typedef FontInstanceList::key_value_pair_t FontInstanceListPair; LogicalFontInstance* mpLastHitCacheEntry; ///< keeps the last hit cache entry FontInstanceList maFontInstanceList; diff --git a/vcl/source/font/fontcache.cxx b/vcl/source/font/fontcache.cxx index 0db997fe510f..2b2e062ecea0 100644 --- a/vcl/source/font/fontcache.cxx +++ b/vcl/source/font/fontcache.cxx @@ -85,13 +85,14 @@ bool ImplFontCache::IFSD_Equal::operator()(const FontSelectPattern& rA, const Fo ImplFontCache::ImplFontCache() : mpLastHitCacheEntry( nullptr ) + , maFontInstanceList(0) // The cache limit is set by the rough number of characters needed to read your average Asian newspaper. , m_aBoundRectCache(3000) {} ImplFontCache::~ImplFontCache() { - for (auto & rLFI : maFontInstanceList) + for (const auto & rLFI : maFontInstanceList) rLFI.second->mpFontCache = nullptr; } @@ -115,7 +116,7 @@ rtl::Reference ImplFontCache::GetFontInstance( PhysicalFont pFontInstance = mpLastHitCacheEntry; else { - FontInstanceList::iterator it = maFontInstanceList.find( aFontSelData ); + FontInstanceList::const_iterator it = maFontInstanceList.find( aFontSelData ); if( it != maFontInstanceList.end() ) pFontInstance = (*it).second; } @@ -130,7 +131,7 @@ rtl::Reference ImplFontCache::GetFontInstance( PhysicalFont aFontSelData.maSearchName = pFontFamily->GetSearchName(); // check if an indirectly matching logical font instance is already cached - FontInstanceList::iterator it = maFontInstanceList.find( aFontSelData ); + FontInstanceList::const_iterator it = maFontInstanceList.find( aFontSelData ); if( it != maFontInstanceList.end() ) pFontInstance = (*it).second; } @@ -168,25 +169,24 @@ rtl::Reference ImplFontCache::GetFontInstance( PhysicalFont if (maFontInstanceList.size() >= FONTCACHE_MAX) { - // remove entries from font instance cache that are only referenced by the cache - FontInstanceList::iterator it_next = maFontInstanceList.begin(); - while( it_next != maFontInstanceList.end() ) + struct limit_exception : public std::exception {}; + try { - LogicalFontInstance* pFontEntry = (*it_next).second.get(); - if( pFontEntry->m_nCount > 1 ) - { - ++it_next; - continue; - } - m_aBoundRectCache.remove_if([&pFontEntry] (GlpyhBoundRectCachePair const& rPair) - { return rPair.first.m_pFont == pFontEntry; } ); - - maFontInstanceList.erase(it_next); - if (mpLastHitCacheEntry == pFontEntry) - mpLastHitCacheEntry = nullptr; - // just remove one entry, which will bring us back under FONTCACHE_MAX size again - break; + maFontInstanceList.remove_if([this] (FontInstanceListPair const& rFontPair) + { + if (maFontInstanceList.size() < FONTCACHE_MAX) + throw limit_exception(); + LogicalFontInstance* pFontEntry = rFontPair.second.get(); + if (pFontEntry->m_nCount > 1) + return false; + m_aBoundRectCache.remove_if([&pFontEntry] (GlpyhBoundRectCachePair const& rGlyphPair) + { return rGlyphPair.first.m_pFont == pFontEntry; }); + if (mpLastHitCacheEntry == pFontEntry) + mpLastHitCacheEntry = nullptr; + return true; + }); } + catch (limit_exception) {} } assert(pFontInstance);