| 1 | | ''Part of DjangoSpecifications'' |
| 2 | | |
| 3 | | |
| 4 | | = Singleton Instances = |
| 5 | | |
| 6 | | This page describes the issues and proposals pertaining to the fact that django's ORM will create as many instances of the same DB object as there are requests. |
| 7 | | |
| 8 | | In short, '''the Identity Map pattern is missing from Django''', see http://martinfowler.com/eaaCatalog/identityMap.html . |
| 9 | | |
| 10 | | == Issues == |
| 11 | | |
| 12 | | === Inconsistency === |
| 13 | | #5514 describes what I think is the main issue with the current behavior of the ORM. Having multiple instances can result in silent data losses. Even worse the actual result would depend on whether one used {{{select_related}}} or not: |
| 14 | | {{{ |
| 15 | | #!python |
| 16 | | class Operand(models.Model): |
| 17 | | value = models.IntField() |
| 18 | | |
| 19 | | class Operation(models.Model): |
| 20 | | arg = models.ForeignKey(Operand) |
| 21 | | |
| 22 | | for operation in Operation.objects.all(): |
| 23 | | operation.arg.value += 1 |
| 24 | | operation.arg.save() |
| 25 | | |
| 26 | | # so far so good, but I want to make it tighter ! Let's use select_related to save a DB query per iteration |
| 27 | | for operation in Operation.objects.all().select_related(): |
| 28 | | operation.arg.value += 1 |
| 29 | | operation.arg.save() |
| 30 | | }}} |
| 31 | | Here we have a problem if two operations point to the same operand. The first version works well but the second preloads all operands from the DB and the same DB operand will result in multiple operand instances in memory. So we're basically modifying and saving the original operand every time instead of cumulating the changes. Here's a band-aid for your foot. |
| 32 | | |
| 33 | | Models with self FKs will exhibit similar issues: |
| 34 | | {{{ |
| 35 | | #!python |
| 36 | | class Directory(models.Model): |
| 37 | | name = models.CharField() |
| 38 | | parent = models.ForeignKey('self') |
| 39 | | |
| 40 | | for dir in Directory.objects.all(): |
| 41 | | if condition(dir): |
| 42 | | dir.modify() |
| 43 | | dir.parent.modify() |
| 44 | | dir.parent.save() |
| 45 | | }}} |
| 46 | | Now let's see, what if condition returns True for a directory and its parent ? If the parent comes first in the main query, it will be modified, saved, reloaded when its child comes up later. If the child comes up first the parent is loaded, modified, saved, and then later on the original value from the outer query will be modified and saved, thus erasing the first change. |
| 47 | | |
| 48 | | The lack of an identity map is the root cause of #8159 - foreign key error when deleting yourself. When an object is deleted it's id is set to None. If you delete yourself, your user is in memory twice. Once as the one being deleted, which gets its id correctly None-ed, and once as request.user, which doesn't. |
| 49 | | |
| 50 | | === Performance === |
| 51 | | The original goal of #17 was reducing memory use and query count by reusing the same instance instead of having a new one. Let's see a typical example of that: |
| 52 | | {{{ |
| 53 | | #!python |
| 54 | | class ArticleType(models.Model): |
| 55 | | name = models.CharField(maxlength=200) |
| 56 | | categories = models.CharField(maxlength=200) |
| 57 | | |
| 58 | | class Article(models.Model): |
| 59 | | title = models.CharField(maxlength=200) |
| 60 | | type_of_article = models.ForeignKey(ArticleType) |
| 61 | | |
| 62 | | def __unicode__(self): |
| 63 | | return u"%s (%s)" % (self.title, self.type_of_article.name) |
| 64 | | |
| 65 | | |
| 66 | | Context({'articles': Article.objects.filter(type=1)}) |
| 67 | | |
| 68 | | {% for article in articles %} |
| 69 | | {{ article }} |
| 70 | | {% endfor %} |
| 71 | | }}} |
| 72 | | |
| 73 | | In the above example, {{{type_of_article}}} should only be queried for once, because it's already in memory. This is a '''very''' common use case for many developers. |
| 74 | | |
| 75 | | If you have a great number of articles and a smaller number of articletypes the performance/memory hit is staggering because: |
| 76 | | * you generate a DB query per article to get the type |
| 77 | | * you instantiate the type once per article instead of once per type |
| 78 | | * you have as many articletype instances in memory as there are articles |
| 79 | | |
| 80 | | == Proposals == |
| 81 | | === Overview === |
| 82 | | The basic idea is to simply reuse existing instances. This works by keeping track of instantiated objects on a per model basis. Whenever we try to instantiate a model, we check if there is already an instance in memory and reuse it if so. This would solve both issues mentioned above. |
| 83 | | |
| 84 | | Please note that the proposal is absolutely NOT a caching system: whenever an instance goes out of scope it is discarded and will have to be reloaded from the DB next time. |
| 85 | | |
| 86 | | This feature would be turned off by default, and only operate for Models which have this feature explicitly enabled. Good candidates for this feature are Models like the articletype above, or models with self references. |
| 87 | | |
| 88 | | === Threads === |
| 89 | | No sharing would occur between threads, as is the case currently. Instances will be unique within each thread. |
| 90 | | |
| 91 | | === API === |
| 92 | | The public API for this feature will be very simple. It will be a single optional parameter in the {{{Meta}}} class: |
| 93 | | |
| 94 | | {{{ |
| 95 | | #!python |
| 96 | | class ArticleType(models.Model): |
| 97 | | name = models.CharField(maxlength=200) |
| 98 | | categories = models.CharField(maxlength=200) |
| 99 | | |
| 100 | | class Meta: |
| 101 | | singleton_instances = True |
| 102 | | }}} |
| 103 | | The default value for the parameter is False, which means that the feature has to be explicitly enabled. |
| 104 | | |
| 105 | | == Implementation == |
| 106 | | #17 currently has a working patch with the following: |
| 107 | | * Per-model {{{WeakValueDictionary}}} with {{{dict[pk_val] = instance}}}. |
| 108 | | * {{{__call__}}} classmethod which either gets a instance from the dict or return a newly created one. |
| 109 | | * a simple interface for enabling or disabling the feature for a given Model (default is disabled). |
| 110 | | * a slightly modified related manager which will first try to get the instance from the Model dict instead of blindly querying the DB. |
| 111 | | * slightly modified serializers to force the instantiation when reading from an outside source. |
| 112 | | * a slightly modified query to force flushing instances when they are delete'd from the DB. |
| 113 | | * tests! |
| 114 | | |
| 115 | | but some things are missing: |
| 116 | | * In order to properly handle shared instances, partial models, or modified models (such as those with extra select clauses) need to be wrapped in a class which will handle those modifications. |
| 117 | | * there is no doc as to how to enable/disable this feature and what to expect from it. |
| 118 | | * the API for enabling/disabling the feature is very crude and consists only of two class methods. It should at the very least be possible to set the value as a member of the {{{Meta}}} class. |
| 119 | | * the Model dict should be set threadlocally. |
| 120 | | * it should be possible to force instantiation, because the serializers want that. |
| 121 | | * For the sake of completeness, it should be possible to do {{{Model.objects.get_from_db(id=something)}}} to bypass the singleton. |
| 122 | | * Document (and maybe fix) the way this would interact with {{{extra()}}} and {{{values}}}. |
| 123 | | |
| 124 | | == Discussions == |
| 125 | | === Internal API === |
| 126 | | ''(David)'' The current patch design overrides the {{{__call__}}} method for the model base which means it automatically returns singleton instances when available. This is convenient because XXX (David or Brian: feel free to fill this out). |
| 127 | | |
| 128 | | ''(Philippe)'' Another approach would be to make this explicit and have a simple API along the lines of: |
| 129 | | {{{ |
| 130 | | #!python |
| 131 | | class ModelBase: |
| 132 | | def get_singleton(pk, kwargs = None): |
| 133 | | """ returns the current singleton for the instance corresponding to pk. If there isn't one in memory and kwargs is specified, it will be built from kwargs """ |
| 134 | | |
| 135 | | }}} |
| 136 | | That means writing {{{Model(kwargs)}}} would return a freshly instantiated object (as is the case now) and bypass the singleton instance. Since Django's ORM would use the API internally, doing {{{Model.objects.get}}} would (by default, overridable in the manager) return the singleton. The default related manager would also have a new {{{get_from_db}}} method to force retrieving and instantiating from the DB only. |
| 137 | | |
| 138 | | === Default behavior === |
| 139 | | Should this feature be enabled by default ? |
| 140 | | * ''(Philippe)'' It should be because it really makes the ORM behave more correctly but for the sake of backward compatibility let's fist commit off by default. We can change the default parameter value later, and in the meantime honor a global setting to turn it on for all models. |
| 141 | | |
| 142 | | * ''(David)'' This would always be enabled. You cannot turn it off. If stuff is broken by having this on then it should be broken. This can cause problems with serializers, and things that modify querysets and shouldn't (e.g. {{{.extra}}}). That is the only reason it should be turned off. If those problems are addressed there is no reason to use #17 and to have it enabled 100% of the time. |
| 143 | | |
| 144 | | === extra, values === |
| 145 | | |
| 146 | | A simple workaround for querysets issues, and this would branch into being able to store {{{.values()}}} as partials as well (this code is untested, but should give a general idea): |
| 147 | | |
| 148 | | {{{ |
| 149 | | #!python |
| 150 | | class SphinxObjectInstance(object): |
| 151 | | """ |
| 152 | | Acts exactly like a normal instance of an object except that |
| 153 | | it will handle any special sphinx attributes in a __sphinx class. |
| 154 | | """ |
| 155 | | _reserved_attributes = ('_sphinx', '__instance') |
| 156 | | |
| 157 | | def __init__(self, instance, attributes={}): |
| 158 | | object.__setattr__(self, '__instance', instance) |
| 159 | | object.__setattr__(self, '_sphinx', SphinxAttributes(attributes)) |
| 160 | | |
| 161 | | def __setattr__(self, name, value): |
| 162 | | if name in _reserved_attributes: |
| 163 | | return object.__setattr__(self, name, value) |
| 164 | | return setattr(self.__instance, name, value) |
| 165 | | |
| 166 | | def __getattr__(self, name, value=None): |
| 167 | | if name in _reserved_attributes: |
| 168 | | return object.__getattr__(self, name, value) |
| 169 | | return getattr(self.__instance, name, value) |
| 170 | | }}} |
| 171 | | |
| 172 | | The proposal would also change how extra is used, it should probably be {{{instance._extra}}} or something similar. |
| 173 | | |
| 174 | | (The code above is taken from an unpublished update to the django-sphinx package.) |
| | 1 | This page and several others were created by a wiki user who was not and is not affiliated with the Django project. Previous contents of this and other similar pages are not and should not be confused with [http://docs.djangoproject.com/ Django's own documentation], which remains the sole source of official documentation for the Django project. |