Skip to content
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

Jump to bottom

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

Open
wants to merge 9 commits into
base: cache_config
Choose a base branch
from
Open

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

wants to merge 9 commits into from

Conversation

mgautierfr
Copy link
Collaborator

  • Allow the cache to be limited by memory usage instead of number of items
  • Make the cluster cache global.

Fix #947

@mgautierfr mgautierfr force-pushed the memory_cache branch 3 times, most recently from 75e78c6 to 5429143 Compare February 14, 2025 16:33
veloman-yunkan
veloman-yunkan reviewed Feb 15, 2025
View reviewed changes
Copy link
Collaborator

@veloman-yunkan veloman-yunkan left a comment

Choose a reason for hiding this comment

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

This looks like a serious change and I skimmed through the PR very quickly without even trying to comment on it but with the only purpose of getting a rough idea of where it will eventually take us. I will come back to it next week.

@mgautierfr mgautierfr force-pushed the cache_config branch 3 times, most recently from 55616af to f3ea99f Compare February 17, 2025 15:59
@mgautierfr mgautierfr mentioned this pull request Feb 17, 2025
Open
@mgautierfr
Make lru_cache open to a cost of each item different than 1
5d0a98a 
This is the first step to a cache limited by memory usage.
@mgautierfr
Make concurrent_cache open to complex limitation
216cfce
@mgautierfr
Add IStreamReader::getSize
96e3db6 
We need to know the size of our input reader. Especially for compression
stream where we need to know the size of the compression state.
@mgautierfr
Limit the cluster cache by memory instead of numbers of items.
2d4c2b3
@mgautierfr
Drop all zim clusters from cache at zim file destruction.
3b88a81 
This is useless now as we destroy the cache itself just after but it will
be useful when cache will be global.
@mgautierfr
Test archive against a preloaded content.
be24ef7 
Cluster cache will be global. So we cannot test content againt a
ref_archive as getting the data from it will also be impacted by cache
configuration.

So we "preload" all the content first and then to the test.
@mgautierfr
Make the cluster cache global.
f41bb7f
@mgautierfr
Reset global cache size before each archive test.
28cd5e2
@mgautierfr
Ensure cluster cache is properly clean in case of exception in constr…
10134b0 
…uctor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@veloman-yunkan veloman-yunkan veloman-yunkan left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mgautierfr @veloman-yunkan