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.

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

Implement dynamic cache entry expiration #50

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nicolaic
Copy link

I've implemented the functionality to dynamically set a specific expiration time, when a cache entry is created. At mentioned in issue #18 this feature can be very useful in some scenarios, when you know some value expires at a specific time.

I've currently only implemented the ability to set an expiration time when an entry is created, as that seemed to be the most common and useful case, but perhaps being able to update the expiration time could be useful as well.

There is currently a bug in the implementation, that the entry is not actually evicted from the cache, but just returns a null value, when retrieving the cache because it is expired. I could add a third queue, that tracks entries with a set expiration, but figured it was easier to discuss the options first.

@nicolaic nicolaic requested a review from ychescale9 as a code owner February 26, 2025 08:35
@nicolaic
Copy link
Author

nicolaic commented Feb 26, 2025

I've created a separate branch where I've added a queue for entries that expire at a set time.

This is better than before, where no entries would get evicted, though this is still not optimal, because of how the queue processing is implemented, if an earlier entry has an expiration time set later than new entries, the newer entries won't be evicted, because it break early, when a entry has not expired. Ideally we'd want something like a priority queue, but I could not find any such collection in the Kotlin collections standard library.

Link to commit: nicolaic@d44ab1f

@ychescale9
Copy link
Member

For KMP priority queue this thread might be relevant.

@nicolaic
Copy link
Author

I can definitely look into a KMP implementation of a priority queue, if we want to go that route. That could possibly also replace these reordering sets/queues. Speaking of which, what is the purpose of these exactly? I can see they're from an external library, which if possible would be nice to get to eliminate third party dependencies and be completely standalone.

Thoughts?

@ychescale9
Copy link
Member

Yes a properly implemented priority queue can probably replace the ReorderingIsoMutableSet which is a rudimentary implementation of a mutable collection that always keeps the inserted item at the end.

The IsoXxx collections from stately are thread-safe, reasonably performant collection implementations for Kotlin Native. For JVM I'm still using the java.util.concurrent.ConcurrentHashMap rather than the IsoMutableMap for keeping the cache entries for better performance. If we want to implement a KMP priority queue I think piggybacking on the Java PriorityQueue for JVM might be the way to go for the same reason.

@nicolaic
Copy link
Author

I'll get started on that. I'll take some heavy inspiration from the Java PriorityQueue,PriorityBlockingQueue and other implementations, to implement a pure Kotlin thread-safe priority queue for our task at hand. I will also consider implementing another data structure for the "reordering sets", as those need to be re-balanced on access/write, where I don't think a priority queue is the best choice.

I'll get this done over the next few days and create a new branch so you can review that. Do you have any acceptance criteria regarding tests for these new thread-safe collections?

@ychescale9
Copy link
Member

Sounds good. We have some integration tests covering multi-threaded scenarios so I wouldn't stress too much about replicating all features in the Java priority queue and testing it properly (unless you want to write a standalone KMP priority queue library).

If you're keen to dive deeper I would look into Kotlinx-lincheck.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants