Skip to content

Commit

Permalink
Correctly test that changing the cache size on live archive has effect.
Browse files Browse the repository at this point in the history
  • Loading branch information
mgautierfr committed Feb 12, 2025
1 parent 83883ff commit fe9ee9d
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 1 deletion.
14 changes: 13 additions & 1 deletion include/zim/archive.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
10 changes: 10 additions & 0 deletions src/archive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions src/concurrent_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/dirent_accessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions src/fileimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -795,13 +795,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);
}
Expand Down
2 changes: 2 additions & 0 deletions src/fileimpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; };
Expand Down
64 changes: 64 additions & 0 deletions test/archive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,70 @@ TEST(ZimArchive, cacheDontImpactReading)
}
}


TEST(ZimArchive, cacheChange)
{

Check notice on line 341 in test/archive.cpp

View check run for this annotation

codefactor.io / CodeFactor

test/archive.cpp#L341

Redundant blank line at the start of a code block should be deleted. (whitespace/blank_line)
for (auto& testfile: getDataFilePath("wikibooks_be_all_nopic_2017-02.zim")) {
auto archive = zim::Archive(testfile.path);

archive.set_dirent_cache_max_size(30);
archive.set_cluster_cache_max_size(5);

auto range = archive.iterEfficient();
auto it = range.begin();
for (auto i = 0; i<50 && it != range.end(); i++, it++) {
// Be sure to search by path to populate the dirent cache
auto entry = archive.getEntryByPath(it->getPath());
auto item = entry.getItem(true);
auto data = item.getData();
}
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);

for (auto i = 0; i<50 && it != range.end(); i++, it++) {
// Be sure to search by path to populate the dirent cache
auto entry = archive.getEntryByPath(it->getPath());
auto item = entry.getItem(true);
auto data = item.getData();
}

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);

for (auto it = range.begin(); it != range.end(); it++) {
// Be sure to search by path to populate the dirent cache
auto entry = archive.getEntryByPath(it->getPath());
auto item = entry.getItem(true);
auto data = item.getData();
}

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";
Expand Down

0 comments on commit fe9ee9d

Please sign in to comment.