[Django] #35230: Cache ForeignObjectRel.get_accessor_name()

11 views
Skip to first unread message

Django

unread,
Feb 18, 2024, 7:26:52 AM2/18/24
to django-...@googlegroups.com
#35230: Cache ForeignObjectRel.get_accessor_name()
-------------------------------------+-------------------------------------
Reporter: Adam | Owner: nobody
Johnson |
Type: | Status: assigned
Cleanup/optimization |
Component: Database | Version: dev
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
This method computes the name for the accessor descriptor, used in many
places. Its results are stable, unless the optional `model` parameter is
provided, used only in forms.

Adding a cached property `accessor_name` for the default value can speed
up many code paths. In particular, I profiled the system checks for a
project and found it was taking ~2% of the runtime, which can be basically
eliminated with caching. Results:

Before: 6625 calls taking 2ms
After: 9 calls taking ~0ms
--
Ticket URL: <https://code.djangoproject.com/ticket/35230>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Feb 18, 2024, 1:13:23 PM2/18/24
to django-...@googlegroups.com
#35230: Cache ForeignObjectRel.get_accessor_name()
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: nobody
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
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 Simon Charette):

* stage: Unreviewed => Accepted

Comment:

This is worth doing but if anyone runs into issues with this change future
when running data migrations the root cause is likely tickets such as
#27737 until #29898 is completed. It is well known that operating from
rendered models can cause all of sorts of issues with regard to caching of
relationship attributes, it should be a reason to avoid using rendered
models though and not to avoid caching relationship attributes.

It will be interesting to see if this has a noticeable effect on
[https://django.github.io/django-asv/ the benchmarks].
--
Ticket URL: <https://code.djangoproject.com/ticket/35230#comment:1>

Django

unread,
Feb 18, 2024, 3:55:37 PM2/18/24
to django-...@googlegroups.com
#35230: Cache ForeignObjectRel.get_accessor_name()
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: nobody
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
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
-------------------------------------+-------------------------------------
Comment (by Adam Johnson):

Thanks for the note Simon.

Yes, it will be interesting to see the benchmark results. For a quick
check, I ran a bunch of model tests with `./runtests.py --parallel 1
"*models*` under [https://github.com/sharkdp/hyperfine hyperfine] and got
the following results.

Before:

```
Time (mean ± σ): 2.745 s ± 0.031 s [User: 2.628 s, System: 0.100
s]
Range (min … max): 2.697 s … 2.796 s 10 runs
```

After:

```
Time (mean ± σ): 2.727 s ± 0.021 s [User: 2.611 s, System: 0.099
s]
Range (min … max): 2.699 s … 2.764 s 10 runs
```

The mean is lower but a statistical T test calculates this as not
statistically significant.
--
Ticket URL: <https://code.djangoproject.com/ticket/35230#comment:2>

Django

unread,
Feb 18, 2024, 11:46:57 PM2/18/24
to django-...@googlegroups.com
#35230: Cache ForeignObjectRel.get_accessor_name()
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* owner: nobody => Adam Johnson
* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35230#comment:3>

Django

unread,
Feb 18, 2024, 11:47:22 PM2/18/24
to django-...@googlegroups.com
#35230: Cache ForeignObjectRel.get_accessor_name()
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Adam
Type: | Johnson
Cleanup/optimization | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by GitHub <noreply@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"5e80390add100e0c7a1ac8e51739f94c5d706ea3" 5e80390a]:
{{{#!CommitTicketReference repository=""
revision="5e80390add100e0c7a1ac8e51739f94c5d706ea3"
Fixed #35230 -- Added cached ForeignObjectRel.accessor_name.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35230#comment:4>
Reply all
Reply to author
Forward
0 new messages