#34211: Performance Regression After Upgrade to Django 3.2
-------------------------------------+-------------------------------------
Reporter: polarmt | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: performance, | Triage Stage:
regression | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
I see three ways to address this issue
1. Revert 4122d9d3f1983eea612f236e941d937bd8589a0d
2. Optimize `ForeignKeyDeferredAttribute.__set__` to reduce its overheard
on model initialization
3. Introduce `Field.initialize_value(instance: Model, value: Any)` that
`Model.__init__` would use. It would default to `setattr(instance,
self.attname, value)` and could be overridden in `ForeignKey` to do
`self.__dict__[self.attname] = value`
------
**Revert 4122d9d3f1983eea612f236e941d937bd8589a0d**
Pros
- Avoid performance regression
Cons
- Re-introduce confusing cache invalidation logic
------
**Optimize `ForeignKeyDeferredAttribute.__set__`**
We'd need to test it out but I'd be curious to see how better the
following performs
{{{#!python
diff --git a/django/db/models/fields/related_descriptors.py
b/django/db/models/fields/related_descriptors.py
index 422b08e6ca..e3ba443b13 100644
--- a/django/db/models/fields/related_descriptors.py
+++ b/django/db/models/fields/related_descriptors.py
@@ -84,9 +84,10 @@ class Child(Model):
class ForeignKeyDeferredAttribute(DeferredAttribute):
def __set__(self, instance, value):
- if instance.__dict__.get(self.field.attname) != value and
self.field.is_cached(
- instance
- ):
+ setvalue = instance.__dict__.setdefault(self.field.attname,
value)
+ if setvalue == value:
+ return
+ if self.field.is_cached(instance):
self.field.delete_cached_value(instance)
instance.__dict__[self.field.attname] = value
}}}
Pros
- Keep #28147 fixed with a (possibly) more performant initialization
approach
Cons
- The initialization optimization is at the cost of an update tax which
might be ok since we more often initialize a large set of model instances
than update their attributes
-----
Introduce `Field.initialize_value(instance: Model, value: Any)` that would
also need to be tested but given the `_setattr = setattr` caching I think
this might be a dead end since `field.initialize` would incur an extra
attribute lookup as well as a function call **for all fields** not only
`ForeignKey` and its subclasses
-----
My take on it is that we should investigate the feasibility and impact of
2. but I'd be 0 on reverting 122d9d3f1983eea612f236e941d937bd8589a0d given
materializing thousands of model instances instead of using
`QuerySet.values` is always going to be slow.
--
Ticket URL: <https://code.djangoproject.com/ticket/34211#comment:6>
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/010701850e019a59-adcaee2d-1a39-44c8-a4a5-e7b8b4c45b6d-000000%40eu-central-1.amazonses.com.
