#30772: Template Cache "make_template_fragment_key" function speed up & switch
to
SHA256
-------------------------------------+-------------------------------------
Reporter: Daniel | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Cache system) | Version: 2.2
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by Daniel:
Old description:
> The `make_template_fragment_key` function in `django.core.cache.utils`
> has the following (minor) issues:
>
> - Using `urllib.quote` for `vary_on` args, is not needed any more - it
> was originally added to make the unhashed strings safe to send to
> memcached and similar restricted systems. But since the value is hashed,
> this is now adding nothing. (See
> https://github.com/django/django/commit/ebc1325721e43808cef4334edaffc23a43f86614
> #diff-702b69be0100a594fd6fea1e4ab2feb1).
>
> - Use of the MD5 hashing function is disallowed on certain (odd) systems,
> not being FIPS compliant. See
> (https://github.com/django/django/pull/10605).
>
> - Creates a string of all joined `vary_on` args to send to the hashing
> function, rather than using the hashlib `.update()` method.
>
> Here is a version solving these, switching to SHA256, and speeding up the
> function quite a bit:
>
> https://github.com/danthedeckie/django/tree/simplified_make_template_fragment_key
>
> And PR: https://github.com/django/django/pull/11772
>
> And here's the repo showing performance improvement:
>
> https://github.com/danthedeckie/make_template_fragment_key_test
>
> Which seems to be faster in every case.
>
> The downside of this is that the cache key is now different from before.
> The tests have been updated to the new values.
>
> Thoughts?
New description:
The `make_template_fragment_key` function in `django.core.cache.utils` has
the following (minor) issues:
- Using `urllib.quote` for `vary_on` args, is not needed any more - it was
originally added to make the unhashed strings safe to send to memcached
and similar restricted systems. But since the value is hashed, this is
now adding nothing. (See
https://github.com/django/django/commit/ebc1325721e43808cef4334edaffc23a43f86614
#diff-702b69be0100a594fd6fea1e4ab2feb1).
- Use of the MD5 hashing function is disallowed on certain (odd) systems,
not being FIPS compliant. See
(https://github.com/django/django/pull/10605).
- Creates a string of all joined `vary_on` args to send to the hashing
function, rather than using the hashlib `.update()` method.
Here is a version solving these, switching to SHA256, and speeding up the
function quite a bit:
https://github.com/danthedeckie/django/tree/simplified_make_template_fragment_key
And PR: https://github.com/django/django/pull/11772
And here's the repo showing performance improvement:
https://github.com/danthedeckie/make_template_fragment_key_test
Which seems to be faster in every case.
The downside of this is that the cache key is now different from before.
The tests have been updated to the new values.
There are other cache key generating functions used in other places which
use MD5 still - if switching to SHA256 it would make sense to me to change
those at the same time, meaning only one time invalidating keys on
upgrade.
Thoughts?
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30772#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/070.b055be4f36925107ee8d41fa99e94f38%40djangoproject.com.
