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

Serialization does not support non-default by functions #51

Open
justinmimbs opened this issue Feb 18, 2025 · 3 comments · May be fixed by #52
Open

Serialization does not support non-default by functions #51

justinmimbs opened this issue Feb 18, 2025 · 3 comments · May be fixed by #52

Comments

@justinmimbs
Copy link

The serialization extension assumes that currentsize is always measured by the number of items in the LRU.

If we use another by function to measure size, then we error trying to serialize (which is good because it looks like deserialize depends on this assumption).

using LRUCache
using Serialization

lru = LRU{String,Vector{UInt8}}(; maxsize = 1024, by = sizeof)
lru["asdf"] = rand(UInt8, 512)

# not the same
@assert lru.currentsize == 512
@assert length(lru) == 1

serialize(IOBuffer(), lru)
# ERROR: AssertionError: lru.currentsize == length(lru)

Would a PR contribution be welcome to help here?
@Jutho
Copy link
Collaborator

Jutho commented Feb 18, 2025

Yes definitely. It seems the @assert statement in the serialize method should not be there, and that removing that would resolve the issue. Maybe @jarbus who implemented the serialization extension can comment on why the @assert statement is there? Is that a leftover from debugging?

@jarbus
Copy link
Contributor

jarbus commented Feb 18, 2025

I have no idea why it's there either! Most likely leftover from debugging, we can probably remove it.

@justinmimbs justinmimbs linked a pull request Feb 19, 2025 that will close this issue
Open
@justinmimbs
Copy link
Author

Thank you both for the fast response! I opened a small PR with a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update serialization extension to support custom cache-size metric justinmimbs/LRUCache.jl
3 participants
@Jutho @justinmimbs @jarbus