-
Notifications
You must be signed in to change notification settings - Fork 227
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.
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
Improve management of folder's caches of articles #1810
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- makes behavior consistent between minimal cache and complete cache - makes things simpler - the advantage in memory management of the specific treatment of these articles can be compensated by smarter eviction policy from cache
folderarrayOfArticles:filterString: is used and it retrieves information directly from the database, when a smart folder or a string filter is required. Try to preserve the article status by complementing database data with the information from the folders caches whenever it is possible.
To preserve article status, we preserve content cache as much as it is possible, except when we see unexpected cache behavior. As cache elements are preserved, we have to be careful to not create duplicate keys.
Inside an article, the elements which are not parts of the minimal cache are the most prone to being discarded.
Bordering writes into NSCache with beginContentAccess / endContentAccess calls seems to fix occurrences of articles being unexpectedly evicted from folder's cache. This also eliminates the need for the workaround which was introduced in commit 49b31e1, because keys were disappearing from the cachedGuids array.
This makes the "Last Refresh" filter to behave more like it did before pull request ViennaRSS#1770 (cf. commit bb06760) Significant difference though: "Last Refresh" refers to the last time a particular feed was refreshed and got new articles, while previously it referred to the last time "Refresh All Subscriptions" was triggered.
Eitot
previously approved these changes
Sep 13, 2024
Apply process optimization similar to the one used for marking all articles of a feed read.
Eitot
approved these changes
Sep 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After PR #1770 was merged, I noticed that the "Last Refresh" filter did not always have the expected behavior.
Investigation led to various discoveries regarding the use of NSCache and parts of the code where article's
status
was lost.This PR solves these issues and gives back a functional "Last Refresh" filter, with a difference though: "Last Refresh" now refers to the last time each feed was refreshed and got new articles, while previously
it referred to the last time "Refresh All Subscriptions" was triggered.