Skip to content

Conversation

@Edenzzzz
Copy link
Contributor

@Edenzzzz Edenzzzz commented Apr 19, 2025

Motivation

Following #4913, seems that we only need to evict tokens_required - avail_size(). cc @xiezhq-hermann

Modifications

Checklist

@xiezhq-hermann
Copy link
Collaborator

xiezhq-hermann commented Apr 19, 2025

technically it is true, but the eviction has always been kind of generous to prevent too frequent eviction, i.e., not to evict if not necessary, and to evict sufficient once it happens. I have no objection to this change but just think it might be not necessary.
if we are making this change, would mind examine eviction of other places to align with this style?

@Edenzzzz
Copy link
Contributor Author

Edenzzzz commented Apr 19, 2025

I see, I think you may be right, collecting, heapifying and iterating through the leaves incurs some overhead, so we should evict generously

@Edenzzzz Edenzzzz closed this Apr 19, 2025
@Edenzzzz Edenzzzz deleted the fix_hi_cache branch April 24, 2025 16:34
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