[Django] #31732: Cache function signatures
Django
-------------------------------------------------+------------------------
Reporter: Adam (Chainz) Johnson | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------------------+------------------------
I found a bit of a performance regression in Django 2.1 and 2.2, gone in
3.0. There, `SQLCompiler.get_converters()` ran `func_supports_parameter()`
for every field converter
Unfortunately the information isn't cached and `inspect.signature()` isn't
particularly fast.
Using py-spy on a client test suite, I found `func_supports_parameter()`
taking 0.25% of the total run time. This isn't much absolutely, but it was
reachable only through pathways using `Query.get_aggregation()` (2.1%) and
`Query.get_count()` (0.63%). So from those paths,
`func_supports_parameter()` took 0.25 / (2.1 + .63) = ~10% of the time to
execute aggregates, for easily cached information.
Given that function objects are immutable, I think all the use of
`inspect.signature` in `django.utils.inspect` could go through a wrapper
that caches with a `WeakKeyDictionary` or `functools.lru_cache` to avoid
re-inspecting the same signatures.
It doesn't look like there is any usage of those functions in hot paths in
Django 3.0+ but it could be useful to have this protection for any future
function argument deprecations.
--
Ticket URL: <https://code.djangoproject.com/ticket/31732>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Adam (Chainz) | Owner: nobody
Johnson |
Type: | Status: new
Cleanup/optimization |
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/31732#comment:1>
Django
-------------------------------------+-------------------------------------
Johnson |
Type: | Status: assigned
Cleanup/optimization |
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Park):
* owner: nobody => Tim Park
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/31732#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Adam (Chainz) | Owner: Tim Park
Johnson |
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/13151 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/31732#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Adam (Chainz) | Owner: Tim Park
Johnson |
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31732#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Adam (Chainz) | Owner: Tim Park
Johnson |
Cleanup/optimization |
Component: Utilities | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"d1409f51ff40f6d0ca875c4fafc4189dbe258b37" d1409f51]:
{{{
#!CommitTicketReference repository=""
revision="d1409f51ff40f6d0ca875c4fafc4189dbe258b37"
Fixed #31732 -- Cached callables signatures in django.utils.inspect
methods.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31732#comment:5>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"562898034f65e17bcdd2d951ac5236a1ec8ea690" 56289803]:
{{{
#!CommitTicketReference repository=""
revision="562898034f65e17bcdd2d951ac5236a1ec8ea690"
Refs #31732 -- Fixed django.utils.inspect caching for bound methods.
Thanks Alexandr Artemyev for the report, and Simon Charette for the
original patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31732#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Tim Park
Type: | Status: closed
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"15a85183880ebebe1b4a1e16e9b2202897d27d8e" 15a85183]:
{{{
#!CommitTicketReference repository=""
revision="15a85183880ebebe1b4a1e16e9b2202897d27d8e"
[3.2.x] Refs #31732 -- Fixed django.utils.inspect caching for bound
methods.
Thanks Alexandr Artemyev for the report, and Simon Charette for the
original patch.
Backport of 562898034f65e17bcdd2d951ac5236a1ec8ea690 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31732#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Tim Park
Type: | Status: closed
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
In [changeset:"ac72a216a7ef95e8e9cb8651b7e67320597c903b" ac72a216]:
{{{
#!CommitTicketReference repository=""
revision="ac72a216a7ef95e8e9cb8651b7e67320597c903b"
Refs #31372 -- Added django.utils.inspect tests for bound methods.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31732#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Tim Park
Type: | Status: closed
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
In [changeset:"2420fd2d5c90698454678c81f3f5a7662be2bf72" 2420fd2]:
{{{
#!CommitTicketReference repository=""
revision="2420fd2d5c90698454678c81f3f5a7662be2bf72"
[3.2.x] Refs #31372 -- Added django.utils.inspect tests for bound methods.
Backport of ac72a216a7ef95e8e9cb8651b7e67320597c903b from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31732#comment:9>