go-generics-cache icon indicating copy to clipboard operation
go-generics-cache copied to clipboard

Improved performance/memory usage for items with an expiry

Open hypirion opened this issue 2 years ago • 4 comments

Hi! Thanks for a good cache library that provides generics – it's really handy.

I've used this a bit for certain things, and found out that adding an expiry to items adds a new goroutine:

https://github.com/Code-Hex/go-generics-cache/blob/a0612befe7b73dba691fcf989637c0d303fc2b84/cache.go#L170-L180

A lot of goroutines is pretty expensive, both in terms of switching and in terms of memory usage (each is 2kb in size).

Would you be open to a PR for a more performant implementation? github.com/patrickmn/go-cache uses a janitor, and I suppose this library could implement something like that -- falling back to the current behaviour if a cleanup interval isn't provided.

hypirion avatar May 31 '22 10:05 hypirion

@hypirion Thanks for feedback!

I understand your opinion, but I have one concern. That is the data will remain on memory unless the next Get method is called.

I'm hoping this can be solved, so please, give me some ideas!

Thanks

Code-Hex avatar May 31 '22 11:05 Code-Hex

And, I don't believe each goroutine size is 2kb. The size meaning for initial stack size.

See https://tpaschalis.github.io/goroutines-size/

Code-Hex avatar May 31 '22 11:05 Code-Hex

@hypirion Thanks for feedback!

I understand your opinion, but I have one concern. That is the data will remain on memory unless the next Get method is called.

I'm hoping this can be solved, so please, give me some ideas!

Oh, right. https://github.com/patrickmn/go-cache uses one janitor goroutine per cache, which at some user-specified interval removes all expired items from the cache:

https://github.com/patrickmn/go-cache/blob/46f407853014144407b6c2ec7ccc76bf67958d93/cache.go#L1071-L1126

There is a bit more overhead when getting values, as you have to check whether they have expired or not. Perhaps one of those things prevent this from being doable, as I haven't looked too closely at the performance implications of this.

And true, as outlined and tested in https://tpaschalis.github.io/goroutines-size/, the goroutine stack size is minimum 2kb, but could be more -- although I don't think the cleanup routines would go much beyond that.

I'd happy to take a look and make a PR if I find a workable solution that doesn't add too much complexity. If I were to do that I'd still fall back on the original implementation with goroutines though, for backwards compatibility/behaviour.

hypirion avatar May 31 '22 23:05 hypirion

@hypirion Thanks. I'm looking forward to the PR you create!

If you have no time, I can make PR for it. So please let me know.

Code-Hex avatar Jun 02 '22 09:06 Code-Hex

I find myself in the same situation. Thousands of ExpirationWatcher goroutines. I think the idea mentioned above is the right direction: one routine taking care of collecting all expired items.

GlennRicaud avatar Aug 12 '22 12:08 GlennRicaud

improved, pls check the code

mingcheng avatar Aug 22 '22 10:08 mingcheng

I've just published the latest version. Thank you for your contributions!

@mingcheng

https://github.com/Code-Hex/go-generics-cache/releases/tag/v1.1.0

Code-Hex avatar Aug 27 '22 07:08 Code-Hex

Thanks for the contribution here! 😁

hypirion avatar Oct 17 '22 13:10 hypirion