Simplify the perthread_info memory management by making the imagecach…

…e itself own (and free) the memory of all per thread info structs, regardless of if they are created implicitly or explicitly.

Signed-off-by: Chris Kulla <[email protected]>

src/libtexture/imagecache.cpp

@@ -67,14 +67,8 @@ static ustring s_constantalpha("constantalpha");
 
 static thread_local tsl::robin_map<const ImageCacheImpl*, std::string> imcache_error_messages;
 
-struct ImageCachePerThreadInfoDeleter {
-    void operator()(ImageCachePerThreadInfo* info) const {
-        ImageCacheImpl::cleanup_perthread_info(info);
-    }
-};
-using ImageCachePerThreadInfoUniquePtr = std::unique_ptr<ImageCachePerThreadInfo, ImageCachePerThreadInfoDeleter>;
 static std::atomic_int64_t imagecache_id_atomic = 0; // constantly increasing, so we can avoid
-static thread_local tsl::robin_map<uint64_t, ImageCachePerThreadInfoUniquePtr> imagecache_per_thread_infos;
+static thread_local tsl::robin_map<uint64_t, ImageCachePerThreadInfo*> imagecache_per_thread_infos;
 
 
 // Functor to compare filenames
@@ -1719,7 +1713,12 @@ ImageCacheImpl::init()
 ImageCacheImpl::~ImageCacheImpl()
 {
     printstats();
-    erase_perthread_info();
+    // All the per_thread_infos get destroyed here, regardless of if they were created implicitly
+    // or manually by the caller
+    {
+        spin_lock lock(m_perthread_info_mutex);
+        m_all_perthread_info.clear();
+    }
 }
 
 
@@ -1729,7 +1728,7 @@ ImageCacheImpl::mergestats(ImageCacheStatistics& stats) const
 {
     stats.init();
     spin_lock lock(m_perthread_info_mutex);
-    for (auto p : m_all_perthread_info)
+    for (auto& p : m_all_perthread_info)
         if (p)
             stats.merge(p->m_stats);
 }
@@ -3813,7 +3812,7 @@ ImageCacheImpl::create_thread_info()
     ImageCachePerThreadInfo* p = new ImageCachePerThreadInfo;
     // printf ("New perthread %p\n", (void *)p);
     spin_lock lock(m_perthread_info_mutex);
-    m_all_perthread_info.push_back(p);
+    m_all_perthread_info.emplace_back(p);
     return p;
 }
 
@@ -3824,14 +3823,16 @@ ImageCacheImpl::destroy_thread_info(ImageCachePerThreadInfo* thread_info)
 {
     if (!thread_info)
         return;
+    // the ImageCache owns the thread_infos associated with it,
+    // so all we need to do is find the entry and reset the unique pointer
+    // to fully destroy the object
     spin_lock lock(m_perthread_info_mutex);
-    for (size_t i = 0; i < m_all_perthread_info.size(); ++i) {
-        if (m_all_perthread_info[i] == thread_info) {
-            m_all_perthread_info[i] = nullptr;
+    for (auto& p : m_all_perthread_info) {
+        if (p.get() == thread_info) {
+            p.reset();
             break;
         }
     }
-    delete thread_info;
 }
 
 
@@ -3841,16 +3842,15 @@ ImageCacheImpl::get_perthread_info(ImageCachePerThreadInfo* p)
 {
     if (!p) {
         // user has not provided an ImageCachePerThreadInfo yet
-        ImageCachePerThreadInfoUniquePtr& ptr = imagecache_per_thread_infos[imagecache_id];
-        p = ptr.get();
+        ImageCachePerThreadInfo*& ptr = imagecache_per_thread_infos[imagecache_id];
+        p = ptr;
         if (!p)
         {
             // this thread doesn't have a ImageCachePerThreadInfo for this ImageCacheImpl yet
-            p = new ImageCachePerThreadInfo;
-            ptr.reset(p);
+            ptr = p = new ImageCachePerThreadInfo;
             // printf ("New perthread %p\n", (void *)p);
             spin_lock lock(m_perthread_info_mutex);
-            m_all_perthread_info.push_back(p);
+            m_all_perthread_info.emplace_back(p);
         }
     }
     if (p->purge) {  // has somebody requested a tile purge?
@@ -3864,41 +3864,6 @@ ImageCacheImpl::get_perthread_info(ImageCachePerThreadInfo* p)
     return p;
 }
 
-
-
-void
-ImageCacheImpl::erase_perthread_info()
-{
-    spin_lock lock(m_perthread_info_mutex);
-    for (size_t i = 0; i < m_all_perthread_info.size(); ++i) {
-        ImageCachePerThreadInfo* p = m_all_perthread_info[i];
-        if (p) {
-            // Clear the microcache.
-            p->tile     = nullptr;
-            p->lasttile = nullptr;
-            p->m_thread_files.clear();
-            m_all_perthread_info[i] = nullptr;
-        }
-    }
-}
-
-
-
-void
-ImageCacheImpl::cleanup_perthread_info(ImageCachePerThreadInfo* p)
-{
-    spin_lock lock(m_perthread_info_mutex);
-    if (p) {
-        // Clear the microcache.
-        p->tile     = nullptr;
-        p->lasttile = nullptr;
-        // This method should only be called via ImageCachePerThreadInfoDeleter
-        delete p;
-    }
-}
-
-
-
 void
 ImageCacheImpl::purge_perthread_microcaches()
 {

src/libtexture/imagecache_pvt.h

@@ -1103,19 +1103,6 @@ class ImageCacheImpl final : public ImageCache {
     Perthread* create_thread_info() override;
     void destroy_thread_info(Perthread* thread_info) override;
 
-    /// Called when the IC is destroyed.  We have a list of all the
-    /// perthread pointers -- go through and delete the ones for which we
-    /// hold the only remaining pointer.
-    void erase_perthread_info();
-
-    /// This is called when the thread terminates.  If p->m_imagecache
-    /// is non-NULL, there's still an imagecache alive that might want
-    /// the per-thread info (say, for statistics, though it's safe to
-    /// clear its tile microcache), so don't delete the perthread info
-    /// (it will be owned thereafter by the IC).  If there is no IC still
-    /// depending on it (signalled by m_imagecache == NULL), delete it.
-    static void cleanup_perthread_info(Perthread* thread_info);
-
     /// Ensure that the max_memory_bytes is at least newsize bytes.
     /// Override the previous value if necessary, with thread-safety.
     void set_min_cache_size(long long newsize);
@@ -1163,7 +1150,7 @@ class ImageCacheImpl final : public ImageCache {
     void clear_fingerprints();
 
     uint64_t imagecache_id;
-    std::vector<ImageCachePerThreadInfo*> m_all_perthread_info;
+    std::vector<std::unique_ptr<ImageCachePerThreadInfo>> m_all_perthread_info;
     static spin_mutex m_perthread_info_mutex;  ///< Thread safety for perthread
     int m_max_open_files;
     atomic_ll m_max_memory_bytes;