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

TTL = Infinity #9

Closed
marcopeg opened this issue Jul 26, 2022 · 4 comments · Fixed by #10
Closed

TTL = Infinity #9

marcopeg opened this issue Jul 26, 2022 · 4 comments · Fixed by #10

Comments

@marcopeg
Copy link

Hello and thanks for this package.

I'd like to propose that TTL could be set to Infinity pretty much same as for the max config param.
This would let setting some items as "pinned" into a TTL cache mechanism.

Thanks again,
Marco

@isaacs
Copy link
Owner

isaacs commented Jul 26, 2022

Idk, kinda sounds like a footgun. If you want an item that doesn't have a TTL, better off using a Map or plain old object instead of a cache that keeps things sorted by expiration.

More to the point, it would require a different data structure internally to do that. This module is able to efficiently prune by taking advantage of the fact that V8 keeps numeric object keys sorted. If a value's TTL is Infinity, then its expiration is also Infinity, which would be cast to the string 'Infinity', and then it'd need a bunch of special case logic to detect non-numeric expiration values. Not really a fit, sorry.

@isaacs isaacs closed this as completed Jul 26, 2022
@isaacs
Copy link
Owner

isaacs commented Aug 2, 2022

Actually this might not be so bad. It does reduce performance a bit to have the string 'Infinity' in the TTL object, which I'm guessing is owing to it no longer being an object with strictly numeric keys. But it does the right thing without much added complexity. Adding the footgun!

@isaacs isaacs reopened this Aug 2, 2022
isaacs added a commit that referenced this issue Aug 2, 2022
@isaacs
Copy link
Owner

isaacs commented Aug 2, 2022

Actually I can do it without having 'Infinity' in the ttl object, it's faster to have the extra check and not take the deopt.

isaacs added a commit that referenced this issue Aug 2, 2022
@marcopeg
Copy link
Author

marcopeg commented Aug 3, 2022 via email

@isaacs isaacs closed this as completed in 1fe7733 Aug 3, 2022
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 a pull request may close this issue.

2 participants