Skip to content

Limit the cluster cache by memory instead of number of clusters #956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 10 commits into from
Closed

Limit the cluster cache by memory instead of number of clusters #956

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7ab76f9
Make ConcurentCache inherit lru_cache.
mgautierfr Mar 3, 2025
a21ee25
Make caches open to a cost of each item different than 1
mgautierfr Feb 7, 2025
71a496f
Add `getMemorySize` on `IStreamReader` and `Reader`.
mgautierfr Feb 11, 2025
1add1d3
Limit the cluster cache by memory instead of numbers of items.
mgautierfr Feb 11, 2025
dc82b88
Drop all zim clusters from cache at zim file destruction.
mgautierfr Feb 14, 2025
a5032d2
Test archive against a preloaded content.
mgautierfr Feb 17, 2025
61c863e
Make the cluster cache global.
mgautierfr Feb 14, 2025
8c91f85
Reset global cache size before each archive test.
mgautierfr Feb 14, 2025
9b9e229
Ensure cluster cache is properly clean in case of exception in constr…
mgautierfr Feb 14, 2025
f896c0f
Check if shared_future has been drop while we compute the value.
mgautierfr Mar 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions src/concurrent_cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ namespace zim
available.
*/
template <typename Key, typename Value>
class ConcurrentCache
class ConcurrentCache: private lru_cache<Key, std::shared_future<Value>>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The explanation in the commit description is not a good one. Private inheritance is not an "is a" relation. To the best of my understanding, this change is made so that ConcurrentCache may have access to the protected API of lru_cache.

{
private: // types
typedef std::shared_future<Value> ValuePlaceholder;
typedef lru_cache<Key, ValuePlaceholder> Impl;

public: // types
explicit ConcurrentCache(size_t maxEntries)
: impl_(maxEntries)
: Impl(maxEntries)
{}

// Gets the entry corresponding to the given key. If the entry is not in the
Expand All @@ -65,7 +65,7 @@ class ConcurrentCache
{
std::promise<Value> valuePromise;
std::unique_lock<std::mutex> l(lock_);
const auto x = impl_.getOrPut(key, valuePromise.get_future().share());
const auto x = Impl::getOrPut(key, valuePromise.get_future().share());
l.unlock();
if ( x.miss() ) {
try {
Expand All @@ -82,26 +82,25 @@ class ConcurrentCache
bool drop(const Key& key)
{
std::unique_lock<std::mutex> l(lock_);
return impl_.drop(key);
return Impl::drop(key);
}

size_t getMaxSize() const {
std::unique_lock<std::mutex> l(lock_);
return impl_.getMaxSize();
return Impl::getMaxSize();
}

size_t getCurrentSize() const {
std::unique_lock<std::mutex> l(lock_);
return impl_.size();
return Impl::size();
}

void setMaxSize(size_t newSize) {
std::unique_lock<std::mutex> l(lock_);
return impl_.setMaxSize(newSize);
return Impl::setMaxSize(newSize);
}

private: // data
Impl impl_;
mutable std::mutex lock_;
};

Expand Down