Opened 13 months ago
Closed 13 months ago
#33937 closed Cleanup/optimization (fixed)
Optimize m2m serialization to avoid loading full model instances
Reported by: | Gordon Wrigley | Owned by: | mark evans |
---|---|---|---|
Component: | Core (Serialization) | Version: | 4.0 |
Severity: | Normal | Keywords: | performance |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
When not using natural keys, the handle_m2m_field
function(source) loads the full object for every entry in the m2m model, when it only needs the pks. The pk's can even be obtained from the m2m intermediate table, without joining the target table.
In my case the table we are m2m'ing to has files in it, so that's a weighty fetch.
We are using django-reversion which stores a serialized version of each save.
On the workload that flagged this up enabling reversion incurs a 300x performance hit (from half a second to 2.5 minutes) and it's almost entirely because of this.
Change History (9)
comment:1 Changed 13 months ago by
Description: | modified (diff) |
---|---|
Summary: | Serialize is loading full objects when serializing m2m fields. → Optimize m2m serialization to avoid loading full model instances |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Cleanup/optimization |
comment:2 follow-up: 3 Changed 13 months ago by
comment:3 Changed 13 months ago by
Replying to Mariusz Felisiak:
When not using natural keys, the handle_m2m_field function(source) loads the full object for every entry in the m2m model, when it only needs the pks.
What about natural keys? They can contain any combination of fields (also non-unique in general).
It loads the whole objects for the natural key case as well, but I don't know if that is avoidable, whereas the PK case seems like it should be easily avoidable (and is also the one I care about :P )
comment:4 Changed 13 months ago by
Owner: | changed from nobody to mark evans |
---|---|
Status: | new → assigned |
Going to take this on as my first Django ticket. Should have a patch shortly.
comment:5 Changed 13 months ago by
Has patch: | set |
---|
comment:6 Changed 13 months ago by
I've submitted a patch here: https://github.com/django/django/pull/16028
comment:7 Changed 13 months ago by
Patch needs improvement: | set |
---|
comment:8 Changed 13 months ago by
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
What about natural keys? They can contain any combination of fields (also non-unique in general).