Correctly test that changing the cache size on live archive has effect.

openzim · Feb 17, 2025 · 55616af · 55616af
1 parent 91afabf
commit 55616af
Show file tree

Hide file tree

Showing 7 changed files with 111 additions and 16 deletions.
diff --git a/include/zim/archive.h b/include/zim/archive.h
@@ -534,12 +534,18 @@ namespace zim
        */
       std::shared_ptr<FileImpl> getImpl() const { return m_impl; }
 
-      /** Get the size of the cluster cache.
+      /** Get the maximum size of the cluster cache.
        *
        * @return The maximum number of clusters stored in  the cache.
        */
       size_t get_cluster_cache_max_size() const;
 
+      /** Get the current size of the cluster cache.
+       *
+       * @return The number of clusters currently stored in  the cache.
+       */
+      size_t get_cluster_cache_current_size() const;
+
       /** Set the size of the cluster cache.
        *
        * If the new size is lower than the number of currently stored clusters
@@ -555,6 +561,12 @@ namespace zim
        */
       size_t get_dirent_cache_max_size() const;
 
+      /** Get the current size of the dirent cache.
+       *
+       * @return The number of dirents currently stored in  the cache.
+       */
+      size_t get_dirent_cache_current_size() const;
+
       /** Set the size of the dirent cache.
        *
        * If the new size is lower than the number of currently stored dirents

diff --git a/src/archive.cpp b/src/archive.cpp
@@ -509,6 +509,11 @@ namespace zim
     return m_impl->get_cluster_cache_max_size();
   }
 
+  size_t Archive::get_cluster_cache_current_size() const
+  {
+    return m_impl->get_cluster_cache_current_size();
+  }
+
   void Archive::set_cluster_cache_max_size(size_t nb_clusters)
   {
     m_impl->set_cluster_cache_max_size(nb_clusters);
@@ -519,6 +524,11 @@ namespace zim
     return m_impl->get_dirent_cache_max_size();
   }
 
+  size_t Archive::get_dirent_cache_current_size() const
+  {
+    return m_impl->get_dirent_cache_current_size();
+  }
+
   void Archive::set_dirent_cache_max_size(size_t nb_dirents)
   {
     m_impl->set_dirent_cache_max_size(nb_dirents);

diff --git a/src/concurrent_cache.h b/src/concurrent_cache.h
@@ -90,6 +90,11 @@ class ConcurrentCache
     return impl_.get_max_size();
   }
 
+  size_t get_current_size() const {
+    std::unique_lock<std::mutex> l(lock_);
+    return impl_.size();
+  }
+
   void set_max_size(size_t new_size) {
     std::unique_lock<std::mutex> l(lock_);
     return impl_.set_max_size(new_size);

diff --git a/src/dirent_accessor.h b/src/dirent_accessor.h
@@ -56,6 +56,7 @@ class LIBZIM_PRIVATE_API DirectDirentAccessor
   entry_index_t getDirentCount() const  {  return m_direntCount; }
 
   size_t get_max_cache_size() const { return m_direntCache.get_max_size(); }
+  size_t get_current_cache_size() const { return m_direntCache.size(); }
   void set_max_cache_size(size_t nb_dirents) const { m_direntCache.set_max_size(nb_dirents); }
 
 private: // functions

diff --git a/src/fileimpl.cpp b/src/fileimpl.cpp
@@ -794,13 +794,19 @@ bool checkTitleListing(const IndirectDirentAccessor& accessor, entry_index_type
   size_t FileImpl::get_cluster_cache_max_size() const {
     return clusterCache.get_max_size();
   }
+  size_t FileImpl::get_cluster_cache_current_size() const {
+    return clusterCache.get_current_size();
+  }
   void FileImpl::set_cluster_cache_max_size(size_t nb_clusters) {
     clusterCache.set_max_size(nb_clusters);
   }
 
   size_t FileImpl::get_dirent_cache_max_size() const {
     return mp_pathDirentAccessor->get_max_cache_size();
   }
+  size_t FileImpl::get_dirent_cache_current_size() const {
+    return mp_pathDirentAccessor->get_current_cache_size();
+  }
   void FileImpl::set_dirent_cache_max_size(size_t nb_dirents) {
     mp_pathDirentAccessor->set_max_cache_size(nb_dirents);
   }

diff --git a/src/fileimpl.h b/src/fileimpl.h
@@ -154,8 +154,10 @@ namespace zim
       bool checkIntegrity(IntegrityCheck checkType);
 
       size_t get_cluster_cache_max_size() const;
+      size_t get_cluster_cache_current_size() const;
       void set_cluster_cache_max_size(size_t nb_clusters);
       size_t get_dirent_cache_max_size() const;
+      size_t get_dirent_cache_current_size() const;
       void set_dirent_cache_max_size(size_t nb_dirents);
       size_t get_dirent_lookup_cache_max_size() const;
       void set_dirent_lookup_cache_max_size(size_t nb_ranges) { m_direntLookupSize = nb_ranges; };

diff --git a/test/archive.cpp b/test/archive.cpp
@@ -286,21 +286,32 @@ struct TestCacheConfig {
   size_t dirent_lookup_cache_size;
 };
 
-#define ASSERT_ARCHIVE_EQUIVALENT(REF_ARCHIVE, TEST_ARCHIVE)                  \
-  for (auto ref_entry:REF_ARCHIVE.iterEfficient()) {                          \
-    auto test_entry = TEST_ARCHIVE.getEntryByPath(ref_entry.getPath());       \
-    EXPECT_EQ(ref_entry.getPath(), test_entry.getPath());                     \
-    EXPECT_EQ(ref_entry.getTitle(), test_entry.getTitle());                   \
-    EXPECT_EQ(ref_entry.isRedirect(), test_entry.isRedirect());               \
-    if (ref_entry.isRedirect()) {                                             \
-      EXPECT_EQ(ref_entry.getRedirectEntryIndex(), test_entry.getRedirectEntryIndex()); \
-    } else {                                                                  \
-      auto ref_item = ref_entry.getItem();                                    \
-      auto test_item = test_entry.getItem();                                  \
-      EXPECT_EQ(ref_item.getClusterIndex(), test_item.getClusterIndex());     \
-      EXPECT_EQ(ref_item.getBlobIndex(), test_item.getBlobIndex());           \
-      EXPECT_EQ(ref_item.getData(), test_item.getData());                     \
-    }                                                                         \
+
+#define ASSERT_ARCHIVE_EQUIVALENT(REF_ARCHIVE, TEST_ARCHIVE)  \
+  ASSERT_ARCHIVE_EQUIVALENT_LIMIT(REF_ARCHIVE, TEST_ARCHIVE, REF_ARCHIVE.getEntryCount())
+
+#define ASSERT_ARCHIVE_EQUIVALENT_LIMIT(REF_ARCHIVE, TEST_ARCHIVE, LIMIT)             \
+  {                                                                                   \
+    auto range = REF_ARCHIVE.iterEfficient();                                         \
+    auto ref_it = range.begin();                                                      \
+    ASSERT_ARCHIVE_EQUIVALENT_IT_LIMIT(ref_it, range.end(), TEST_ARCHIVE, LIMIT)      \
+  }
+
+
+#define ASSERT_ARCHIVE_EQUIVALENT_IT_LIMIT(REF_IT, REF_END, TEST_ARCHIVE, LIMIT)      \
+  for (auto i = 0U; i<LIMIT && REF_IT != REF_END; i++, REF_IT++) {                    \
+    auto test_entry = TEST_ARCHIVE.getEntryByPath(REF_IT->getPath());                 \
+    ASSERT_EQ(REF_IT->getPath(), test_entry.getPath());                               \
+    ASSERT_EQ(REF_IT->getTitle(), test_entry.getTitle());                             \
+    ASSERT_EQ(REF_IT->isRedirect(), test_entry.isRedirect());                         \
+    if (REF_IT->isRedirect()) {                                                       \
+      ASSERT_EQ(REF_IT->getRedirectEntryIndex(), test_entry.getRedirectEntryIndex()); \
+    }                                                                                 \
+    auto ref_item = REF_IT->getItem(true);                                            \
+    auto test_item = test_entry.getItem(true);                                        \
+    ASSERT_EQ(ref_item.getClusterIndex(), test_item.getClusterIndex());               \
+    ASSERT_EQ(ref_item.getBlobIndex(), test_item.getBlobIndex());                     \
+    ASSERT_EQ(ref_item.getData(), test_item.getData());                               \
   }
 
 TEST(ZimArchive, cacheDontImpactReading)
@@ -337,6 +348,54 @@ TEST(ZimArchive, cacheDontImpactReading)
   }
 }
 
+
+TEST(ZimArchive, cacheChange)
+{
+  for (auto& testfile: getDataFilePath("wikibooks_be_all_nopic_2017-02.zim")) {
+    auto ref_archive = zim::Archive(testfile.path);
+    auto archive = zim::Archive(testfile.path);
+
+    archive.set_dirent_cache_max_size(30);
+    archive.set_cluster_cache_max_size(5);
+
+    auto range = ref_archive.iterEfficient();
+    auto ref_it = range.begin();
+    ASSERT_ARCHIVE_EQUIVALENT_IT_LIMIT(ref_it, range.end(), archive, 50)
+    EXPECT_EQ(archive.get_dirent_cache_current_size(), 30);
+    EXPECT_EQ(archive.get_cluster_cache_current_size(), 2); // Only 2 clusters in the file
+
+    // Reduce cache size
+    archive.set_dirent_cache_max_size(10);
+    archive.set_cluster_cache_max_size(1);
+
+    EXPECT_EQ(archive.get_dirent_cache_current_size(), 10);
+    EXPECT_EQ(archive.get_cluster_cache_current_size(), 1);
+
+    ASSERT_ARCHIVE_EQUIVALENT_IT_LIMIT(ref_it, range.end(), archive, 50)
+
+    EXPECT_EQ(archive.get_dirent_cache_current_size(), 10);
+    EXPECT_EQ(archive.get_cluster_cache_current_size(), 1);
+
+    // Clean cache
+    // (More than testing the value, this is needed as we want to be sure the cache is actually populated later)
+    archive.set_dirent_cache_max_size(0);
+    archive.set_cluster_cache_max_size(0);
+
+    EXPECT_EQ(archive.get_dirent_cache_current_size(), 0);
+    EXPECT_EQ(archive.get_cluster_cache_current_size(), 0);
+
+    // Increase the cache
+    archive.set_dirent_cache_max_size(20);
+    archive.set_cluster_cache_max_size(1);
+    EXPECT_EQ(archive.get_dirent_cache_current_size(), 0);
+    EXPECT_EQ(archive.get_cluster_cache_current_size(), 0);
+
+    ASSERT_ARCHIVE_EQUIVALENT(ref_archive, archive)
+    EXPECT_EQ(archive.get_dirent_cache_current_size(), 20);
+    EXPECT_EQ(archive.get_cluster_cache_current_size(), 1);
+  }
+}
+
 TEST(ZimArchive, openDontFallbackOnNonSplitZimArchive)
 {
   const char* fname = "wikibooks_be_all_nopic_2017-02.zim";