Ticket #17: instance-caching-5.patch

django/core/serializers/python.py

@@ -88 +88 @@
             # Handle all other fields
             else:
                 data[field.name] = field.to_python(field_value)
-
+        data["disable_inst_caching"] = True
         yield base.DeserializedObject(Model(**data), m2m_data)
 
 def _get_model(model_identifier):

django/core/serializers/xml_serializer.py

@@ -176 +176 @@
                 else:
                     value = field.to_python(getInnerText(field_node).strip())
                 data[field.name] = value
-
+        data["disable_inst_caching"] = True
         # Return a DeserializedObject so that the m2m data has a place to live.
         return base.DeserializedObject(Model(**data), m2m_data)
 
@@ -234 +234 @@
         else:
            pass
     return u"".join(inner_text)
-

django/db/models/base.py

@@ -15 +15 @@
 from django.utils.encoding import smart_str, force_unicode
 from django.conf import settings
 from itertools import izip
+from weakref import WeakValueDictionary
 import types
 import sys
 import os
@@ -77 +78 @@
         # registered version.
         return get_model(new_class._meta.app_label, name, False)
 
+    def __call__(cls, *args, **kwargs):
+        if not kwargs.pop('disable_inst_caching', False) and not cls.disable_inst_caching:
+            key = cls._get_cache_key(args, kwargs)
+            if key is not None:
+                obj = cls.__instance_cache__.get(key)
+                if obj is None:
+                    obj = super(ModelBase, cls).__call__(*args, **kwargs)
+                    cls.__instance_cache__[key] = obj
+            else:
+                obj = super(ModelBase, cls).__call__(*args, **kwargs)
+        else:
+            obj = super(ModelBase, cls).__call__(*args, **kwargs)
+        return obj
+
+
 class Model(object):
     __metaclass__ = ModelBase
 
@@ -97 +113 @@
     def __ne__(self, other):
         return not self.__eq__(other)
 
+    def _get_cache_key(cls, args, kwargs):
+        # this should be calculated *once*, but isn't atm
+        pk_position = cls._meta.fields.index(cls._meta.pk)
+        if len(args) > pk_position:
+            return args[pk_position]
+        pk = cls._meta.pk
+        if pk.name in kwargs:
+            return kwargs[pk.name]
+        elif pk.attname in kwargs:
+            return kwargs[pk.attname]
+        return None
+    _get_cache_key = classmethod(_get_cache_key)
+
+    def get_cached_instance(cls, id):
+        return cls.__instance_cache__.get(id)
+    get_cached_instance = classmethod(get_cached_instance)
+
     def __init__(self, *args, **kwargs):
         dispatcher.send(signal=signals.pre_init, sender=self.__class__, args=args, kwargs=kwargs)
 
@@ -197 +230 @@
         if hasattr(cls, 'get_absolute_url'):
             cls.get_absolute_url = curry(get_absolute_url, opts, cls.get_absolute_url)
 
+        cls.__instance_cache__ = WeakValueDictionary()
+        cls.disable_inst_caching = getattr(cls, 'disable_inst_caching', False) or not cls._meta.has_auto_field
+
         dispatcher.send(signal=signals.class_prepared, sender=cls)
 
     _prepare = classmethod(_prepare)
@@ -255 +291 @@
                 setattr(self, self._meta.pk.attname, backend.get_last_insert_id(cursor, self._meta.db_table, self._meta.pk.column))
         transaction.commit_unless_managed()
 
+        # if we're a new instance that hasn't been written in; save ourself.
+        if not self.disable_inst_caching:
+            self.__instance_cache__[self._get_pk_val()] = self
+
         # Run any post-save hooks.
         dispatcher.send(signal=signals.post_save, sender=self.__class__, instance=self)
 
@@ -315 +355 @@
         self._collect_sub_objects(seen_objs)
 
         # Actually delete the objects
+        if not self.disable_inst_caching:
+            pk = self._get_pk_val()
+            if pk is not None:
+                self.__instance_cache__.pop(pk, None)
         delete_objects(seen_objs)
 
     delete.alters_data = True

django/db/models/fields/related.py

@@ -165 +165 @@
                 if self.field.null:
                     return None
                 raise self.field.rel.to.DoesNotExist
-            other_field = self.field.rel.get_related_field()
-            if other_field.rel:
-                params = {'%s__pk' % self.field.rel.field_name: val}
-            else:
-                params = {'%s__exact' % self.field.rel.field_name: val}
-            rel_obj = self.field.rel.to._default_manager.get(**params)
+            rel_obj = self.field.rel.to.get_cached_instance(val)
+            if rel_obj is None:
+                other_field = self.field.rel.get_related_field()
+                if other_field.rel:
+                    params = {'%s__pk' % self.field.rel.field_name: val}
+                else:
+                    params = {'%s__exact' % self.field.rel.field_name: val}
+                rel_obj = self.field.rel.to._default_manager.get(**params)
             setattr(instance, cache_name, rel_obj)
             return rel_obj
 

django/db/models/query.py

@@ -1117 +1117 @@
     for cls in ordered_classes:
         seen_objs[cls] = seen_objs[cls].items()
         seen_objs[cls].sort()
+        clean_inst_cache = cls.__instance_cache__.pop
 
         # Pre notify all instances to be deleted
         for pk_val, instance in seen_objs[cls]:
             dispatcher.send(signal=signals.pre_delete, sender=cls, instance=instance)
 
         pk_list = [pk for pk,instance in seen_objs[cls]]
+        # we wipe the cache now; it's *possible* some form of a __get__ lookup may reintroduce an item after
+        # the fact with the same pk (extremely unlikely)
+        for x in pk_list:
+            clean_inst_cache(x, None)
+
         for related in cls._meta.get_all_related_many_to_many_objects():
             if not isinstance(related.field, generic.GenericRelation):
-                for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
+                for offset in xrange(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
                     cursor.execute("DELETE FROM %s WHERE %s IN (%s)" % \
                         (qn(related.field.m2m_db_table()),
                             qn(related.field.m2m_reverse_name()),
-                            ','.join(['%s' for pk in pk_list[offset:offset+GET_ITERATOR_CHUNK_SIZE]])),
-                        pk_list[offset:offset+GET_ITERATOR_CHUNK_SIZE])
+                            ','.join(['%s' for pk in pk_list[offset:offset+GET_ITERATOR_CHUNK_SIZE]])), 
+                            pk_list[offset:offset+GET_ITERATOR_CHUNK_SIZE])
+
         for f in cls._meta.many_to_many:
             if isinstance(f, generic.GenericRelation):
                 from django.contrib.contenttypes.models import ContentType
@@ -1139 +1146 @@
             else:
                 query_extra = ''
                 args_extra = []
-            for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
+            for offset in xrange(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
                 cursor.execute(("DELETE FROM %s WHERE %s IN (%s)" % \
                     (qn(f.m2m_db_table()), qn(f.m2m_column_name()),
                     ','.join(['%s' for pk in pk_list[offset:offset+GET_ITERATOR_CHUNK_SIZE]]))) + query_extra,
                     pk_list[offset:offset+GET_ITERATOR_CHUNK_SIZE] + args_extra)
+
         for field in cls._meta.fields:
             if field.rel and field.null and field.rel.to in seen_objs:
-                for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
+                for offset in xrange(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
                     cursor.execute("UPDATE %s SET %s=NULL WHERE %s IN (%s)" % \
                         (qn(cls._meta.db_table), qn(field.column), qn(cls._meta.pk.column),
                             ','.join(['%s' for pk in pk_list[offset:offset+GET_ITERATOR_CHUNK_SIZE]])),
@@ -1156 +1164 @@
     for cls in ordered_classes:
         seen_objs[cls].reverse()
         pk_list = [pk for pk,instance in seen_objs[cls]]
-        for offset in range(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
+        clean_inst_cache = cls.__instance_cache__.pop
+        for x in pk_list:
+            clean_inst_cache(x, None)
+        for offset in xrange(0, len(pk_list), GET_ITERATOR_CHUNK_SIZE):
             cursor.execute("DELETE FROM %s WHERE %s IN (%s)" % \
                 (qn(cls._meta.db_table), qn(cls._meta.pk.column),
                 ','.join(['%s' for pk in pk_list[offset:offset+GET_ITERATOR_CHUNK_SIZE]])),

tests/modeltests/basic/models.py

@@ -361 +361 @@
 __test__['API_TESTS'] += """
 
 # You can manually specify the primary key when creating a new object.
->>> a101 = Article(id=101, headline='Article 101', pub_date=datetime(2005, 7, 31, 12, 30, 45))
+>>> a101 = Article(id=101, headline=u'Article 101', pub_date=datetime(2005, 7, 31, 12, 30, 45))
 >>> a101.save()
 >>> a101 = Article.objects.get(pk=101)
 >>> a101.headline

tests/modeltests/custom_columns/models.py

@@ -40 +40 @@
 
 __test__ = {'API_TESTS':"""
 # Create a Author.
->>> a = Author(first_name='John', last_name='Smith')
+>>> a = Author(first_name=u'John', last_name=u'Smith')
 >>> a.save()
 
 >>> a.id

tests/modeltests/generic_relations/models.py

@@ -87 +87 @@
 
 # Recall that the Mineral class doesn't have an explicit GenericRelation
 # defined. That's OK, because you can create TaggedItems explicitly.
->>> tag1 = TaggedItem(content_object=quartz, tag="shiny")
->>> tag2 = TaggedItem(content_object=quartz, tag="clearish")
+>>> tag1 = TaggedItem(content_object=quartz, tag=u"shiny")
+>>> tag2 = TaggedItem(content_object=quartz, tag=u"clearish")
 >>> tag1.save()
 >>> tag2.save()
 

tests/modeltests/many_to_one/models.py

@@ -27 +27 @@
 
 __test__ = {'API_TESTS':"""
 # Create a few Reporters.
->>> r = Reporter(first_name='John', last_name='Smith', email='john@example.com')
+>>> r = Reporter(first_name=u'John', last_name=u'Smith', email='john@example.com')
 >>> r.save()
 
->>> r2 = Reporter(first_name='Paul', last_name='Jones', email='paul@example.com')
+>>> r2 = Reporter(first_name=u'Paul', last_name=u'Jones', email='paul@example.com')
 >>> r2.save()
 
 # Create an Article.

tests/modeltests/select_related/models.py

@@ -107 +107 @@
 1
 
 # select_related() also of course applies to entire lists, not just items.
-# Without select_related()
+# Without select_related() (note instance caching still reduces this from 9 to 5)
 >>> db.reset_queries()
 >>> world = Species.objects.all()
 >>> [o.genus.family for o in world]
 [<Family: Drosophilidae>, <Family: Hominidae>, <Family: Fabaceae>, <Family: Amanitacae>]
 >>> len(db.connection.queries)
-9
+5
 
 # With select_related():
 >>> db.reset_queries()
@@ -129 +129 @@
 >>> pea.genus.family.order.klass.phylum.kingdom.domain
 <Domain: Eukaryota>
 
-# Notice: one few query than above because of depth=1
+# notice: instance caching saves the day; would be 7 without.
 >>> len(db.connection.queries)
-7
+1
 
 >>> db.reset_queries()
 >>> pea = Species.objects.select_related(depth=5).get(name="sativum")
 >>> pea.genus.family.order.klass.phylum.kingdom.domain
 <Domain: Eukaryota>
 >>> len(db.connection.queries)
-3
+1
 
 >>> db.reset_queries()
 >>> world = Species.objects.all().select_related(depth=2)
 >>> [o.genus.family.order for o in world]
 [<Order: Diptera>, <Order: Primates>, <Order: Fabales>, <Order: Agaricales>]
 >>> len(db.connection.queries)
-5
+1
 
 # Reset DEBUG to where we found it.
 >>> settings.DEBUG = False