contrib.admin:list_editable - ForeignKey Performance is O(m*n)?

[chadc]

Hi there,

I have been experiencing poor performance with django-admin when
list_editable includes ForeignKey fields. In particular, rendering the
changelist requires O(m*n) database queries where m is the number of
ForeignKey fields included in list_editable and n is the number of
rows in the changelist. I have searched extensively for possible
causes and, after finding nothing and receiving no response on either
django-users ('list_editable duplicate queries') or IRC, I am starting
to think that it is a legitimate bug.

The problem, as I understand it, stems from the fact that the choices
for the ForeignKey widgets are not cached. So, when every ForeignKey
widget on every row is rendered, it queries the database to retrieve
the list of possible values. The result in an unacceptible number of
queries, especially given the default changelist length of n=100.

For my own purposes, I have addressed this issue in three ways:

1. Adding a widget cache to django/forms/widgets.py:~431 -- A patch is
available upon request.

2. Manually rendering the data using a custom display function in the
admin model.

3. Overriding the ForeignKey widget with a custom CachedSelect widget.

The last fix has worked best for me because it does not require
hacking up django, but I think that this issue is worth addressing
properly in Django. So, does this warrant a ticket?

Thanks,

Chad





PS: For the sake of posterity, I have included the code for my
CachedSelect widget. Yes, I am aware that using regular expressions to
parse the key is a nasty hack. If you have any better ideas, please
let me know!

~~~widgets.py~~~

import re

from django.forms.widgets import Select

class CachedSelect(Select):

cache = {}
regex = re.compile('^form-(?P<id>[0-9]+)-(?P<model>.*)$')

def render(self, name, value, attrs=None, choices=()):
# If name does not match form-<num>-<model>, render widget
regularly
match = self.regex.match(name)
if not match:
return super(CachedSelect, self).render(name, value,
attrs, choices)

id = match.group('id')
model = match.group('model')

# Cache the data if necessary (first hit of changelist or not
cached)
if id == '0' or not self.cache.has_key(model):
self.cache[model] = [choice for choice in self.choices]

self.choices = self.cache[model]
return super(CachedSelect, self).render(name, value, attrs,
choices)


~~~admin.py~~~

from widgets import CachedSelect
...

class BlahAdmin(admin.ModelAdmin):
formfield_overrides = {models.ForeignKey:{'widget':
CachedSelect()}}
...

[Jeremy Dunck]

On Wed, Jun 30, 2010 at 2:04 AM, chadc <chad...@hotmail.com> wrote:
...

> 3. Overriding the ForeignKey widget with a custom CachedSelect widget.
>

...

>
> PS: For the sake of posterity, I have included the code for my
> CachedSelect widget. Yes, I am aware that using regular expressions to
> parse the key is a nasty hack. If you have any better ideas, please
> let me know!

For clarity of what's going on here, would you mind posting an example model?

[chadc]

Oh, yah, sure. Sorry.

class Host(models.Model):
name = models.CharField(max_length=128, unique=True)

class Account(models.Model):
host = models.ForeignKey(Host, related_name="accounts")
name = models.CharField(max_length=128)

class AccountAdmin(admin.ModelAdmin):
list_display = ('name', 'host')
list_editable = ('host',)
list_per_page = 25
admin.site.register(Account, AccountAdmin)

Rendering the FK widgets here requires n*m=25*1=25 database queries:

Form: SELECT "hosts_host"."id", "hosts_host"."name" FROM "hosts_host"
ORDER BY "hosts_host"."name" ASC
Total time: 330 ms
Numer of queries: 25

Twenty five queries is, of course, not a big deal. However, the
default changelist is n=100, and with m=4 this becomes a real problem.

If you comment out the list_editable line or use the fix below
( formfield_overrides = {models.ForeignKey:{'widget':
CachedSelect()}} ), the number of queries returns to O(1).

The jist of what I am wondering is if the changelist is going to allow
editable ForeignKeys, should it be caching the choices rather than
hitting the database every time?



On Jun 29, 7:14 pm, Jeremy Dunck <jdu...@gmail.com> wrote:

[Jeremy Dunck]

On Thu, Jul 1, 2010 at 12:40 AM, chadc <chad...@hotmail.com> wrote:
...

> The jist of what I am wondering is if the changelist is going to allow
> editable ForeignKeys, should it be caching the choices rather than
> hitting the database every time?

Yep, your models and explanation are clear. I'm not sure that
CachedSelect is a good solution; it seems like there's a general
problem with repeated queries within ModelFormSet, not just admin--
it's just cropping up here. I'll try to look at it by tomorrow.

File a ticket and let me know the number?

[chadc]

Alright. I filed it as #13871 under contrib.admin because I do not
understand the larger issue, but please update it as you see fit.

Thanks, Jeremy.

Ticket Link: http://code.djangoproject.com/ticket/13871