Ticket #17: 17.diff

django/db/models/base.py

@@ -15 +15 @@
-from django.utils.encoding import smart_str, force_unicode, smart_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):
+        """
+        this method will either create an instance (by calling the default implementation)
+        or try to retrieve one from the class-wide cache by infering the pk value from 
+        args and kwargs. If instance caching is enabled for this class, the cache is 
+        populated whenever possible (ie when it is possible to infer the pk value). If 'meta__disable_caching'
+        is set to True in kwargs, then the instance is constructed and we flush 
+        the associated cache entry. 
+        """
+        def new_instance():
+            return super(ModelBase, cls).__call__(*args, **kwargs)
+        
+        cache_this_instance = cls.instance_caching_enabled()
+        # we always pop those settings from kwargs, the instance shouldn't see this
+        if kwargs.pop('meta__disable_caching', False):
+            # user explicitely requested not to use cache, we flush the cache to prevent inconsitencies
+            cls._flush_cached_by_key(cls._get_cache_key(args, kwargs))
+            cache_this_instance = False
+            
+        # simplest case, just create a new instance every time 
+        if not cache_this_instance:
+            return new_instance()
+        
+        instance_key = cls._get_cache_key(args, kwargs)
+        # depending on the arguments, we might not be able to infer the PK, so in that case we create a new instance
+        if instance_key is None:
+            cls._instance_cache_nokey_misses += 1
+            return new_instance()
+
+        cached_instance = cls.get_cached_instance(instance_key)
+        if cached_instance is None:
+            cached_instance = new_instance()
+            cls.cache_instance(cached_instance)
+
+        return cached_instance
+
-class Model(object):
-    __metaclass__ = ModelBase
-
@@ -97 +134 @@
-    def __ne__(self, other):
-        return not self.__eq__(other)
-
+    def _get_cache_key(cls, args, kwargs):
+        """
+        This method is used by the caching subsystem to infer the PK value from the constructor arguments. 
+        It is used to decide if an instance has to be built or is already in the cache. 
+        """
+        result = None
+        pk = cls._meta.pk
+        # get the index of the pk in the class fields. this should be calculated *once*, but isn't atm
+        pk_position = cls._meta.fields.index(pk)
+        if len(args) > pk_position:
+            # if it's in the args, we can get it easily by index
+            result = args[pk_position]
+        elif pk.attname in kwargs:
+            # retrieve the pk value. Note that we use attname instead of name, to handle the case where the pk is a 
+            # a ForeignKey.
+            result = kwargs[pk.attname]
+        elif pk.name != pk.attname and pk.name in kwargs:
+            # ok we couldn't find the value, but maybe it's a FK and we can find the corresponding object instead
+            result = kwargs[pk.name]
+        
+        if result is not None and isinstance(result, Model):
+            # if the pk value happens to be a model instance (which can happen wich a FK), we'd rather use its own pk as the key
+            result = result._get_pk_val()
+        return result
+    _get_cache_key = classmethod(_get_cache_key)
+
+    def get_cached_instance(cls, id):
+        """
+        Method to retrieve a cached instance by pk value. Returns None when not found 
+        (which will always be the case when caching is disabled for this class). Please 
+        note that the lookup will be done even when instance caching is disabled, thus 
+        generating a miss in the stats.
+        """
+        result = cls.__instance_cache__.get(id)
+        if result is None:
+            cls._instance_cache_misses += 1
+        else:
+            cls._instance_cache_hits += 1
+        return result
+    get_cached_instance = classmethod(get_cached_instance)
+
+    def cache_instance(cls, instance):
+        """
+        Method to store an instance in the cache. TODO: add a store counter in the stats 
+        """
+        if cls.instance_caching_enabled() and instance._get_pk_val() is not None:
+            cls.__instance_cache__[instance._get_pk_val()] = instance
+    cache_instance = classmethod(cache_instance)
+
+    def _flush_cached_by_key(cls, key):
+        if cls.__instance_cache__.pop(key, None) is not None:
+            cls._instance_cache_flushes += 1
+    _flush_cached_by_key = classmethod(_flush_cached_by_key)
+        
+    def flush_cached_instance(cls, instance):
+        """
+        Method to flush an instance from the cache. The instance will always be flushed from the cache, 
+        since this is most likely called from delete(), and we want to make sure we don't cache dead objects.
+        We do not test the pk value because delete() does it and it will fail silently anyway. 
+        """
+        if cls.instance_caching_enabled():
+            cls._flush_cached_by_key(instance._get_pk_val())
+    flush_cached_instance = classmethod(flush_cached_instance)
+
+    def instance_caching_enabled(cls):
+        """
+        Accessor for the cache settings.
+        """
+        # cache is off by default! 
+        return getattr(cls, '_meta__instance_caching', False)
+    instance_caching_enabled = classmethod(instance_caching_enabled)
+
+    def set_instance_caching(cls, enable):
+        """
+        Accessor for the cache settings. Note that the cache is flushed and the stats reset when
+        the settings are switched (ie enabling the cache multiple times will not flush). 
+        """
+        current_settings = cls.instance_caching_enabled()
+        cls._meta__instance_caching = enable
+        # completely flush the cache every time the settings are changed 
+        if enable != current_settings:
+            cls.__instance_cache__.clear()
+            cls.instance_caching_stats_reset()
+    set_instance_caching = classmethod(set_instance_caching)
+    
+    def instance_caching_stats_reset(cls):
+        # also used to init the stats in '_prepare()'
+        cls._instance_cache_hits = 0
+        cls._instance_cache_misses = 0
+        cls._instance_cache_nokey_misses = 0
+        cls._instance_cache_flushes = 0
+    instance_caching_stats_reset = classmethod(instance_caching_stats_reset)
+    
+    def instance_caching_stats(cls):
+        return {'enabled': cls.instance_caching_enabled(), 
+                'hits' : cls._instance_cache_hits, 
+                'misses': cls._instance_cache_misses, 
+                'flushes': cls._instance_cache_flushes, 
+                'misses_nokey': cls._instance_cache_nokey_misses, 
+                'cache_size': len(cls.__instance_cache__) }
+    instance_caching_stats = classmethod(instance_caching_stats)
+        
-    def __init__(self, *args, **kwargs):
-        dispatcher.send(signal=signals.pre_init, sender=self.__class__, args=args, kwargs=kwargs)
-
@@ -197 +336 @@
-        if hasattr(cls, 'get_absolute_url'):
-            cls.get_absolute_url = curry(get_absolute_url, opts, cls.get_absolute_url)
-
+        cls.__instance_cache__ = WeakValueDictionary()
+        cls.instance_caching_stats_reset()
+        # enable the cache according to user preferences (off by default)
+        # FIXME better interface for setting this value (meta class attribute ?)
+        cls.set_instance_caching(getattr(cls, 'meta__instance_caching', False))
+
-        dispatcher.send(signal=signals.class_prepared, sender=cls)
-
-    _prepare = classmethod(_prepare)
@@ -261 +406 @@
-                setattr(self, self._meta.pk.attname, connection.ops.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.
+        self.__class__.cache_instance(self)
+
-        # Run any post-save hooks.
-        dispatcher.send(signal=signals.post_save, sender=self.__class__,
-                instance=self, created=(not record_exists))
@@ -321 +469 @@
-        seen_objs = SortedDict()
-        self._collect_sub_objects(seen_objs)
-
+        # remove ourself from the cache
+        self.__class__.flush_cached_instance(self)
-        # Actually delete the objects
-        delete_objects(seen_objs)
-

django/db/models/fields/related.py

@@ -159 +159 @@
-        try:
-            return getattr(instance, cache_name)
-        except AttributeError:
+            related_cls = self.field.rel.to
-            val = getattr(instance, self.field.attname)
-            if val is None:
-                # If NULL is an allowed value, return it.
-                if self.field.null:
-                    return None
-
-
-
-
+
+
+
+
+
-            else:
-
-
+
+
+
+
+
+
+
+
-            setattr(instance, cache_name, rel_obj)
-            return rel_obj
-

django/db/models/query.py

@@ -1134 +1134 @@
-            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 instance in seen_objs.values():
+            cls.flush_cached_instance(instance)
+
-        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):
@@ -1167 +1172 @@
-    for cls in ordered_classes:
-        seen_objs[cls].reverse()
-        pk_list = [pk for pk,instance in seen_objs[cls]]
+        for instance in seen_objs.values():
+            cls.flush_cached_instance(instance)
-        for offset in range(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),

django/core/serializers/xml_serializer.py

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

django/core/serializers/python.py

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

tests/modeltests/select_related/models.py

@@ -75 +75 @@
-        obj.save()
-        parent = obj
-
+def set_instance_caching(settings):
+    for cls in [Domain, Kingdom, Phylum, Klass, Order, Family, Genus]:
+        cls.set_instance_caching(settings)
-__test__ = {'API_TESTS':"""
-
-# Set up.
@@ -147 +150 @@
->>> len(db.connection.queries)
-5
-
+# CACHING TESTS
+>>> Genus.instance_caching_stats()
+{'hits': 0, 'misses_nokey': 0, 'enabled': False, 'flushes': 0, 'misses': 0, 'cache_size': 0}
+
+# ENABLE CACHING ON ALL MODELS IN THE TEST EXCEPT SPECIES
+>>> set_instance_caching(True)
+
+# This should be the same as without caching
+>>> db.reset_queries()
+>>> fly = Species.objects.get(name="melanogaster")
+>>> fly.genus.family.order.klass.phylum.kingdom.domain
+<Domain: Eukaryota>
+>>> len(db.connection.queries)
+8
+
+# This should be the same as without caching
+>>> db.reset_queries()
+>>> person = Species.objects.select_related().get(name="sapiens")
+>>> person.genus.family.order.klass.phylum.kingdom.domain
+<Domain: Eukaryota>
+>>> len(db.connection.queries)
+1
+
+# now let's see how caching helps
+>>> create_tree("Eukaryota Animalia Chordata Mammalia Primates Hominidae Homo sapiens_2")
+>>> create_tree("Eukaryota Animalia Chordata Mammalia Primates Hominidae Homo sapiens_3")
+>>> create_tree("Eukaryota Animalia Chordata Mammalia Primates Hominidae Homo sapiens_4")
+>>> create_tree("Eukaryota Animalia Chordata Mammalia Primates Hominidae Homo sapiens_5")
+>>> create_tree("Eukaryota Animalia Chordata Mammalia Primates Hominidae Homo sapiens_6")
+>>> set_instance_caching(False)
+>>> db.reset_queries()
+>>> world = Species.objects.all()
+>>> geni_of_world = [o.genus for o in world]
+>>> len(db.connection.queries) # 1 for Species and 9 for the Geni
+10
+>>> Genus.instance_caching_stats()
+{'hits': 0, 'misses_nokey': 0, 'enabled': False, 'flushes': 0, 'misses': 0, 'cache_size': 0}
+
+>>> set_instance_caching(True)
+>>> db.reset_queries()
+>>> world = Species.objects.all()
+>>> geni_of_world == [o.genus for o in world]
+True
+>>> len(db.connection.queries) # 1 for Species and 4 for the distinct Geni
+5
+
+# here we get 8 misses because ReverseSingleRelatedObjectDescriptor misses twice when the object isn't in the cache
+>>> Genus.instance_caching_stats() # 4 distinct Geni and 5 rows generating hits. 
+{'hits': 5, 'misses_nokey': 0, 'enabled': True, 'flushes': 0, 'misses': 8, 'cache_size': 4}
+>>> Genus.get_cached_instance(2)
+<Genus: Homo>
+>>> Genus.instance_caching_stats()['hits'] # one more hit !
+6
+>>> Genus.instance_caching_stats_reset()
+>>> Genus.instance_caching_stats()
+{'hits': 0, 'misses_nokey': 0, 'enabled': True, 'flushes': 0, 'misses': 0, 'cache_size': 4}
+
+>>> Genus.instance_caching_stats_reset()
+>>> Genus.get_cached_instance(2)
+<Genus: Homo>
+>>> Genus.instance_caching_stats()
+{'hits': 1, 'misses_nokey': 0, 'enabled': True, 'flushes': 0, 'misses': 0, 'cache_size': 4}
+>>> Genus.objects.get(id=2)
+<Genus: Homo>
+>>> Genus.instance_caching_stats()
+{'hits': 2, 'misses_nokey': 0, 'enabled': True, 'flushes': 0, 'misses': 0, 'cache_size': 4}
+>>> Species.objects.get(id=2).genus
+<Genus: Homo>
+>>> Genus.instance_caching_stats()
+{'hits': 3, 'misses_nokey': 0, 'enabled': True, 'flushes': 0, 'misses': 0, 'cache_size': 4}
+
+>>> set_instance_caching(False)
+>>> set_instance_caching(True)
+>>> Genus.get_cached_instance(2)
+>>> Genus.objects.get(id=2)
+<Genus: Homo>
+>>> Genus.get_cached_instance(2)
+<Genus: Homo>
+>>> Genus.instance_caching_stats_reset()
+>>> db.reset_queries()
+>>> world = Species.objects.all()
+>>> sapiens = world[1]
+>>> len(db.connection.queries) # 1 for Species and the rest is in the cache, whoa
+1
+>>> Genus.instance_caching_stats() # we haven't touched geni yet 
+{'hits': 0, 'misses_nokey': 0, 'enabled': True, 'flushes': 0, 'misses': 0, 'cache_size': 0}
+>>> homo = sapiens.genus
+>>> Genus.instance_caching_stats() # 2 misses from ReverseSingleRelatedObjectDescriptor even if only one object was retrieved
+{'hits': 0, 'misses_nokey': 0, 'enabled': True, 'flushes': 0, 'misses': 2, 'cache_size': 1}
+
+>>> set_instance_caching(False)
+>>> Genus.get_cached_instance(2)
+
+# This one is tricky, we get() the instance so it gets cached, 
+# then test that instanciating with the same PK retrieves the instance
+>>> set_instance_caching(True)
+>>> Genus.instance_caching_enabled()
+True
+>>> Species.instance_caching_enabled()
+False
+>>> first_homo = Genus.objects.get(id=2)
+>>> first_homo
+<Genus: Homo>
+>>> Genus.instance_caching_stats()['cache_size']
+1
+>>> Genus.instance_caching_stats()['hits']
+0
+>>> homo = Genus.get_cached_instance(2)
+>>> homo
+<Genus: Homo>
+>>> Genus.instance_caching_stats()['hits']
+1
+>>> Genus.instance_caching_stats_reset()
+>>> kwargs = {'id': 2}
+>>> Genus._get_cache_key([], kwargs)
+2
+>>> Genus(id = 2)
+<Genus: Homo>
+>>> Genus.instance_caching_stats()['hits']
+1
+>>> Genus.flush_cached_instance(homo)
+>>> Genus.get_cached_instance(2) == None
+True
+>>> Genus.instance_caching_stats()['cache_size']
+0
+>>> Genus.instance_caching_stats_reset()
+>>> first_homo = Genus.objects.get(id=2)
+>>> Genus.instance_caching_stats()['misses']
+1
+
+## each of the initial species has it own genus but the 5 sapiens dupes will hit the cache
+#>>> Genus.instance_caching_stats()
+#{'hits': 5, 'misses_nokey': 0, 'enabled': True, 'flushes': 0, 'misses': 4, 'cache_size': 4}
+#
+#>>> set_instance_caching(False) # Flushes the cache
+#>>> set_instance_caching(True)
+#>>> Genus.instance_caching_stats_reset()
+#>>> temp = [o.genus for o in (list(Species.objects.all()) + list(Species.objects.all()))]
+#>>> Genus.instance_caching_stats()
+#{'hits': 5, 'misses_nokey': 0, 'enabled': True, 'flushes': 0, 'misses': 0, 'cache_size': 4}
+#
+#>>> set_instance_caching(False) # Flushes the cache
+#>>> set_instance_caching(True)
+#>>> db.reset_queries()
+#>>> Genus.instance_caching_stats_reset()
+#>>> world = Species.objects.all().select_related()
+#>>> [o.genus for o in world]
+#[<Genus: Drosophila>, <Genus: Homo>, <Genus: Pisum>, <Genus: Amanita>, <Genus: Homo>, <Genus: Homo>, <Genus: Homo>, <Genus: Homo>, <Genus: Homo>]
+#>>> len(db.connection.queries)
+#1
+#>>> Genus.instance_caching_stats()
+#{'hits': 5, 'misses_nokey': 0, 'enabled': True, 'flushes': 0, 'misses': 4, 'cache_size': 4}
+
-# Reset DEBUG to where we found it.
->>> settings.DEBUG = False
-"""}