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

Fix: notify expiration only if the latest expire time is changed when getItem or SetWithTTL #56

Merged
merged 1 commit into from
Oct 15, 2021

Conversation

chenyahui
Copy link
Contributor

@chenyahui chenyahui commented Oct 15, 2021

If the latest expire time is changed, no need to notify expiration.

Benchmark Result

BenchmarkName Before After
BenchmarkCacheSetWithoutTTL-8 1140 ns/op 297.1 ns/op
BenchmarkCacheSetWithGlobalTTL-8 1508 ns/op 772.2 ns/op
BenchmarkCacheSetWithTTL-8 1648 ns/op 808.6 ns/op

According the benchmark result, it has 2~3 times performance improvement


item.touch()
cache.priorityQueue.update(item)
if item.ttl == 0 {
Copy link
Contributor

@ReneKroon ReneKroon Oct 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bit implicit from line 90 (implying cache.ttl > 0 when you get here), but ok

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if cache.ttl = 0, item.ttl will also remain 0. This is what we want, but with less code.

@ReneKroon ReneKroon merged commit 2463849 into jellydator:master Oct 15, 2021
@chenyahui
Copy link
Contributor Author

By the way, can we get a release version later at your convenience? I need recent commits for my project.

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