[Libreoffice-commits] core.git: vcl/inc vcl/source vcl/unx

[Jan-Marek Glogowski (via logerrit)]

 vcl/inc/unx/glyphcache.hxx            |   13 +++++++++++++
 vcl/source/font/fontcache.cxx         |    9 +++++++++
 vcl/unx/generic/glyphs/glyphcache.cxx |   21 ++++++++++++++++++++-
 3 files changed, 42 insertions(+), 1 deletion(-)

New commits:
commit e6aac0b637d583d3cfb893276f813ff5aa1ade17
Author:     Jan-Marek Glogowski <jan-marek.glogowski at extern.cib.de>
AuthorDate: Sat Nov 16 02:21:26 2019 +0000
Commit:     Jan-Marek Glogowski <glogow at fbihome.de>
CommitDate: Tue Nov 19 03:13:06 2019 +0100

    tdf#128434 try garbage collect ImplFontCache fonts
    
    Instead of changing the harfbuzz caching, for this use case it's
    enough to queue all per-OutputDevice fonts for garbage collection
    (GC), which are managed by the OutputDevices ImplFontCache. So
    just try to GC all the fonts in the ImplFontCache destructor.
    
    There is no point keeping these LogicalFontInstances alive, after
    the OutputDevice font cache is destroyed, as these fonts aren't
    shared and can't be accessed later. But the main problem is still
    some correct accounting of harfbuzz resources and eventual even
    the Freetype ones, so this cleanup would not really be needed.
    
    AFAI can tell, this plugs the remaining per-document leaks of the
    PDF generation, except for a 72 byte basic listener leak from:
    
    basic::ImplRepository::impl_createManagerForModel(...)
        basicmanagerrepository.cxx:480
    
    Change-Id: I3155be59a2bdcd85e01f6f048b990db04d88c94f
    Reviewed-on: https://gerrit.libreoffice.org/82968
    Tested-by: Jenkins
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>

diff --git a/vcl/inc/unx/glyphcache.hxx b/vcl/inc/unx/glyphcache.hxx
index 797890a2a345..f369952faac4 100644
--- a/vcl/inc/unx/glyphcache.hxx
+++ b/vcl/inc/unx/glyphcache.hxx
@@ -90,6 +90,19 @@ public:
 
     FreetypeFont*           CacheFont(LogicalFontInstance* pFontInstance);
     void                    UncacheFont( FreetypeFont& );
+
+    /** Try to GarbageCollect an explicit logical font
+     *
+     * This should just be called from the ~ImplFontCache destructor, which holds the mapping of the
+     * FontSelectPattern to the LogicalFontInstance per OutputDevice. All other users should just
+     * call CacheFont and UncacheFont correctly. When the ImplFontCache is destroyed with its
+     * OutputDevice, we can safely garbage collection its unused entries, as these can't be reused.
+     *
+     * It's always safe to call this, as it just ignores the used bytes when considering a font for
+     * garbage collection, which normally keeps unreferenced fonts alive.
+     **/
+    void TryGarbageCollectFont(LogicalFontInstance*);
+
     void                    ClearFontCache();
     void                    ClearFontOptions();
 
diff --git a/vcl/source/font/fontcache.cxx b/vcl/source/font/fontcache.cxx
index 67217acd0472..f3f87d36616b 100644
--- a/vcl/source/font/fontcache.cxx
+++ b/vcl/source/font/fontcache.cxx
@@ -24,6 +24,10 @@
 #include <PhysicalFontFamily.hxx>
 #include <sal/log.hxx>
 
+#if !(defined(_WIN32) || defined(MACOSX) || defined(IOS))
+#include <unx/glyphcache.hxx>
+#endif
+
 size_t ImplFontCache::IFSD_Hash::operator()( const FontSelectPattern& rFSD ) const
 {
     return rFSD.hashCode();
@@ -91,7 +95,12 @@ ImplFontCache::ImplFontCache()
 ImplFontCache::~ImplFontCache()
 {
     for (const auto & rLFI : maFontInstanceList)
+    {
+#if !(defined(_WIN32) || defined(MACOSX) || defined(IOS))
+        GlyphCache::GetInstance().TryGarbageCollectFont(rLFI.second.get());
+#endif
         rLFI.second->mpFontCache = nullptr;
+    }
 }
 
 rtl::Reference<LogicalFontInstance> ImplFontCache::GetFontInstance( PhysicalFontCollection const * pFontList,
diff --git a/vcl/unx/generic/glyphs/glyphcache.cxx b/vcl/unx/generic/glyphs/glyphcache.cxx
index 34543b7731a0..9640ea70adcf 100644
--- a/vcl/unx/generic/glyphs/glyphcache.cxx
+++ b/vcl/unx/generic/glyphs/glyphcache.cxx
@@ -199,6 +199,21 @@ void GlyphCache::UncacheFont( FreetypeFont& rFreetypeFont )
     }
 }
 
+void GlyphCache::TryGarbageCollectFont(LogicalFontInstance *pFontInstance)
+{
+    if (maFontList.empty() || !pFontInstance)
+        return;
+    FreetypeFontInstance* pFFI = dynamic_cast<FreetypeFontInstance*>(pFontInstance);
+    if (!pFFI)
+        return;
+    FreetypeFont* pFreetypeFont = pFFI->GetFreetypeFont();
+    if (pFreetypeFont && (pFreetypeFont->GetRefCount() <= 0))
+    {
+        mpCurrentGCFont = pFreetypeFont;
+        GarbageCollect();
+    }
+}
+
 void GlyphCache::GarbageCollect()
 {
     // when current GC font has been destroyed get another one
@@ -236,7 +251,11 @@ void GlyphCache::GarbageCollect()
         if( pFreetypeFont == mpCurrentGCFont )
             mpCurrentGCFont = nullptr;
 
-        maFontList.erase(pFreetypeFont->GetFontInstance());
+#ifndef NDEBUG
+        int nErased =
+#endif
+            maFontList.erase(pFreetypeFont->GetFontInstance());
+        assert(1 == nErased);
     }
 }