Skip to content

Commit cc02757

Browse files
mgautierfrmgautierfr
committed
Correctly test that changing the cache size on live archive has effect.
1 parent fa21419 commit cc02757

File tree

7 files changed

+113
-16
lines changed

7 files changed

+113
-16
lines changed

‎include/zim/archive.h

+13-1
Original file line numberDiff line numberDiff line change
@@ -534,12 +534,18 @@ namespace zim
534534
*/
535535
std::shared_ptr<FileImpl> getImpl() const { return m_impl; }
536536

537-
/** Get the size of the cluster cache.
537+
/** Get the maximum size of the cluster cache.
538538
*
539539
* @return The maximum number of clusters stored in the cache.
540540
*/
541541
size_t get_cluster_cache_max_size() const;
542542

543+
/** Get the current size of the cluster cache.
544+
*
545+
* @return The number of clusters currently stored in the cache.
546+
*/
547+
size_t get_cluster_cache_current_size() const;
548+
543549
/** Set the size of the cluster cache.
544550
*
545551
* If the new size is lower than the number of currently stored clusters
@@ -555,6 +561,12 @@ namespace zim
555561
*/
556562
size_t get_dirent_cache_max_size() const;
557563

564+
/** Get the current size of the dirent cache.
565+
*
566+
* @return The number of dirents currently stored in the cache.
567+
*/
568+
size_t get_dirent_cache_current_size() const;
569+
558570
/** Set the size of the dirent cache.
559571
*
560572
* If the new size is lower than the number of currently stored dirents

‎src/archive.cpp

+10
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,11 @@ namespace zim
509509
return m_impl->get_cluster_cache_max_size();
510510
}
511511

512+
size_t Archive::get_cluster_cache_current_size() const
513+
{
514+
return m_impl->get_cluster_cache_current_size();
515+
}
516+
512517
void Archive::set_cluster_cache_max_size(size_t nb_clusters)
513518
{
514519
m_impl->set_cluster_cache_max_size(nb_clusters);
@@ -519,6 +524,11 @@ namespace zim
519524
return m_impl->get_dirent_cache_max_size();
520525
}
521526

527+
size_t Archive::get_dirent_cache_current_size() const
528+
{
529+
return m_impl->get_dirent_cache_current_size();
530+
}
531+
522532
void Archive::set_dirent_cache_max_size(size_t nb_dirents)
523533
{
524534
m_impl->set_dirent_cache_max_size(nb_dirents);

‎src/concurrent_cache.h

+5
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,11 @@ class ConcurrentCache
9090
return impl_.get_max_size();
9191
}
9292

93+
size_t get_current_size() const {
94+
std::unique_lock<std::mutex> l(lock_);
95+
return impl_.size();
96+
}
97+
9398
void set_max_size(size_t new_size) {
9499
std::unique_lock<std::mutex> l(lock_);
95100
return impl_.set_max_size(new_size);

‎src/dirent_accessor.h

+1
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ class LIBZIM_PRIVATE_API DirectDirentAccessor
5656
entry_index_t getDirentCount() const { return m_direntCount; }
5757

5858
size_t get_max_cache_size() const { return m_direntCache.get_max_size(); }
59+
size_t get_current_cache_size() const { return m_direntCache.size(); }
5960
void set_max_cache_size(size_t nb_dirents) const { m_direntCache.set_max_size(nb_dirents); }
6061

6162
private: // functions

‎src/fileimpl.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -794,13 +794,19 @@ bool checkTitleListing(const IndirectDirentAccessor& accessor, entry_index_type
794794
size_t FileImpl::get_cluster_cache_max_size() const {
795795
return clusterCache.get_max_size();
796796
}
797+
size_t FileImpl::get_cluster_cache_current_size() const {
798+
return clusterCache.get_current_size();
799+
}
797800
void FileImpl::set_cluster_cache_max_size(size_t nb_clusters) {
798801
clusterCache.set_max_size(nb_clusters);
799802
}
800803

801804
size_t FileImpl::get_dirent_cache_max_size() const {
802805
return mp_pathDirentAccessor->get_max_cache_size();
803806
}
807+
size_t FileImpl::get_dirent_cache_current_size() const {
808+
return mp_pathDirentAccessor->get_current_cache_size();
809+
}
804810
void FileImpl::set_dirent_cache_max_size(size_t nb_dirents) {
805811
mp_pathDirentAccessor->set_max_cache_size(nb_dirents);
806812
}

‎src/fileimpl.h

+2
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,10 @@ namespace zim
154154
bool checkIntegrity(IntegrityCheck checkType);
155155

156156
size_t get_cluster_cache_max_size() const;
157+
size_t get_cluster_cache_current_size() const;
157158
void set_cluster_cache_max_size(size_t nb_clusters);
158159
size_t get_dirent_cache_max_size() const;
160+
size_t get_dirent_cache_current_size() const;
159161
void set_dirent_cache_max_size(size_t nb_dirents);
160162
size_t get_dirent_lookup_cache_max_size() const;
161163
void set_dirent_lookup_cache_max_size(size_t nb_ranges) { m_direntLookupSize = nb_ranges; };

‎test/archive.cpp

+76-15
Original file line numberDiff line numberDiff line change
@@ -286,21 +286,32 @@ struct TestCacheConfig {
286286
size_t dirent_lookup_cache_size;
287287
};
288288

289-
#define ASSERT_ARCHIVE_EQUIVALENT(REF_ARCHIVE, TEST_ARCHIVE) \
290-
for (auto ref_entry:REF_ARCHIVE.iterEfficient()) { \
291-
auto test_entry = TEST_ARCHIVE.getEntryByPath(ref_entry.getPath()); \
292-
EXPECT_EQ(ref_entry.getPath(), test_entry.getPath()); \
293-
EXPECT_EQ(ref_entry.getTitle(), test_entry.getTitle()); \
294-
EXPECT_EQ(ref_entry.isRedirect(), test_entry.isRedirect()); \
295-
if (ref_entry.isRedirect()) { \
296-
EXPECT_EQ(ref_entry.getRedirectEntryIndex(), test_entry.getRedirectEntryIndex()); \
297-
} else { \
298-
auto ref_item = ref_entry.getItem(); \
299-
auto test_item = test_entry.getItem(); \
300-
EXPECT_EQ(ref_item.getClusterIndex(), test_item.getClusterIndex()); \
301-
EXPECT_EQ(ref_item.getBlobIndex(), test_item.getBlobIndex()); \
302-
EXPECT_EQ(ref_item.getData(), test_item.getData()); \
303-
} \
289+
290+
#define ASSERT_ARCHIVE_EQUIVALENT(REF_ARCHIVE, TEST_ARCHIVE) \
291+
ASSERT_ARCHIVE_EQUIVALENT_LIMIT(REF_ARCHIVE, TEST_ARCHIVE, REF_ARCHIVE.getEntryCount())
292+
293+
#define ASSERT_ARCHIVE_EQUIVALENT_LIMIT(REF_ARCHIVE, TEST_ARCHIVE, LIMIT) \
294+
{ \
295+
auto range = REF_ARCHIVE.iterEfficient(); \
296+
auto ref_it = range.begin(); \
297+
ASSERT_ARCHIVE_EQUIVALENT_IT_LIMIT(ref_it, range.end(), TEST_ARCHIVE, LIMIT) \
298+
}
299+
300+
301+
#define ASSERT_ARCHIVE_EQUIVALENT_IT_LIMIT(REF_IT, REF_END, TEST_ARCHIVE, LIMIT) \
302+
for (auto i = 0U; i<LIMIT && REF_IT != REF_END; i++, REF_IT++) { \
303+
auto test_entry = TEST_ARCHIVE.getEntryByPath(REF_IT->getPath()); \
304+
ASSERT_EQ(REF_IT->getPath(), test_entry.getPath()); \
305+
ASSERT_EQ(REF_IT->getTitle(), test_entry.getTitle()); \
306+
ASSERT_EQ(REF_IT->isRedirect(), test_entry.isRedirect()); \
307+
if (REF_IT->isRedirect()) { \
308+
ASSERT_EQ(REF_IT->getRedirectEntryIndex(), test_entry.getRedirectEntryIndex()); \
309+
} \
310+
auto ref_item = REF_IT->getItem(true); \
311+
auto test_item = test_entry.getItem(true); \
312+
ASSERT_EQ(ref_item.getClusterIndex(), test_item.getClusterIndex()); \
313+
ASSERT_EQ(ref_item.getBlobIndex(), test_item.getBlobIndex()); \
314+
ASSERT_EQ(ref_item.getData(), test_item.getData()); \
304315
}
305316

306317
TEST(ZimArchive, cacheDontImpactReading)
@@ -337,6 +348,56 @@ TEST(ZimArchive, cacheDontImpactReading)
337348
}
338349
}
339350

351+
352+
TEST(ZimArchive, cacheChange)
353+
{
354+
for (auto& testfile: getDataFilePath("wikibooks_be_all_nopic_2017-02.zim")) {
355+
auto ref_archive = zim::Archive(testfile.path);
356+
auto archive = zim::Archive(testfile.path);
357+
358+
archive.set_dirent_cache_max_size(30);
359+
archive.set_cluster_cache_max_size(5);
360+
361+
auto range = ref_archive.iterEfficient();
362+
auto ref_it = range.begin();
363+
ASSERT_ARCHIVE_EQUIVALENT_IT_LIMIT(ref_it, range.end(), archive, 50)
364+
EXPECT_EQ(archive.get_dirent_cache_current_size(), 30);
365+
EXPECT_EQ(archive.get_cluster_cache_current_size(), 2); // Only 2 clusters in the file
366+
367+
// Reduce cache size
368+
archive.set_dirent_cache_max_size(10);
369+
archive.set_cluster_cache_max_size(1);
370+
371+
EXPECT_EQ(archive.get_dirent_cache_current_size(), 10);
372+
EXPECT_EQ(archive.get_cluster_cache_current_size(), 1);
373+
374+
// We want to test change of cache while we are iterating on the archive.
375+
// So we don't reset the ref_it to `range.begin()`.
376+
ASSERT_ARCHIVE_EQUIVALENT_IT_LIMIT(ref_it, range.end(), archive, 50)
377+
378+
EXPECT_EQ(archive.get_dirent_cache_current_size(), 10);
379+
EXPECT_EQ(archive.get_cluster_cache_current_size(), 1);
380+
381+
// Clean cache
382+
// (More than testing the value, this is needed as we want to be sure the cache is actually populated later)
383+
archive.set_dirent_cache_max_size(0);
384+
archive.set_cluster_cache_max_size(0);
385+
386+
EXPECT_EQ(archive.get_dirent_cache_current_size(), 0);
387+
EXPECT_EQ(archive.get_cluster_cache_current_size(), 0);
388+
389+
// Increase the cache
390+
archive.set_dirent_cache_max_size(20);
391+
archive.set_cluster_cache_max_size(1);
392+
EXPECT_EQ(archive.get_dirent_cache_current_size(), 0);
393+
EXPECT_EQ(archive.get_cluster_cache_current_size(), 0);
394+
395+
ASSERT_ARCHIVE_EQUIVALENT(ref_archive, archive)
396+
EXPECT_EQ(archive.get_dirent_cache_current_size(), 20);
397+
EXPECT_EQ(archive.get_cluster_cache_current_size(), 1);
398+
}
399+
}
400+
340401
TEST(ZimArchive, openDontFallbackOnNonSplitZimArchive)
341402
{
342403
const char* fname = "wikibooks_be_all_nopic_2017-02.zim";

0 commit comments

Comments
 (0)