-
Notifications
You must be signed in to change notification settings - Fork 47
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
option to drop module/function from key_generator functions in util.py #121
Comments
Michael Bayer (zzzeek) wrote: this is...for convenience vs. making your own function_key_generator? the issue of "long keys" is indented to be solved by key_mangler. You can make a key_mangler that just truncates the module/name part, if you dont want to go with making a GUID (I assume that's where your concern wtih mangling is).. that is, you can get what you want with your own fn_key_geneartor, you can get what you want with a key mangler . where is this lacking? |
Michael Bayer (zzzeek) wrote: how about this:
? |
jvanasco (jvanasco) wrote: re: custom key_generator. re: mangled guid re: mangled 'other' the idea for a |
Michael Bayer (zzzeek) wrote: makes sense but the whole function should be templated then |
Migrated issue, originally created by jvanasco (jvanasco)
I tracked down a performance issue to usage of
dogpile.cache.region:Region.cache_on_arguments
in a few places.The default key generation is provided key generator functions in dogpile.cache.util, with this relevant code shared by all:
My issue was the "base" of the key(
'%s:%s' % (fn.__module__, fn.__name__)
) was incredibly long in a handful of the most-used places, and the keys themselves were taking up a significant chunk of the memory allocated to Redis. Dropping the key length reclaimed a lot of space.While a
key_mangler
would solve this, I needed to keep the keys unmangled. The easiest solution was to just reimplement the stock key generator to just use thenamespace
argument as the key.This seems like a relatively useful approach for many people, so I wanted to suggest porting it upstream:
I think the relevant changes would be:
The text was updated successfully, but these errors were encountered: