Skip to content

Add take_dropped to lazy_lru as a template parameter#3

Open
tomerfiliba wants to merge 3 commits into
behzadnouri:masterfrom
sweet-security:master
Open

Add take_dropped to lazy_lru as a template parameter#3
tomerfiliba wants to merge 3 commits into
behzadnouri:masterfrom
sweet-security:master

Conversation

@tomerfiliba

@tomerfiliba tomerfiliba commented Mar 4, 2025

Copy link
Copy Markdown

Hi @behzadnouri, I really like lazy_lru but I need a way to obtain the dropped elements. The "main" LruCache impl allows you to get the dropped value upon insert, but your crate doesn't. I added put_with_evicted() which does the trick

@tomerfiliba tomerfiliba force-pushed the master branch 2 times, most recently from 0a201ee to b1b6e47 Compare March 4, 2025 13:38
@behzadnouri

Copy link
Copy Markdown
Owner

I will take a look, but on a cursory initial look:

  • Can you please provide some context when/why you would need the evicted cached entries?
  • Have you checked if this code has any impact on performance? there are already some benchmarks in the crate.

@tomerfiliba

Copy link
Copy Markdown
Author

i'm using the LRU for dedup purposes, to reduce the number of events sent. once an event gets dropped from the LRU i want to pick it up and send it. the events that remain in the LRU are drained every interval.

@kskalski

Copy link
Copy Markdown

The performance concern here is valid, this implementation will allocate a vector every time the eviction happens, which will make the impact of latency spikes (puts that have to evict as opposed to those staying within capacity) higher.

I think it could return an impl Iterator instead, such that the existing entries are split into those that stay and those that are returned to the caller.

As for use-cases for such API, there could be many - we gather stats in the cache value or we need to do some some extra work / finalization for dropped values. In general if the implementation of some API removes a series of elements, it is nice to allow obtaining those elements back somewhere through that or supplementing API.

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.

3 participants