Code

Opened 4 years ago

Last modified 3 years ago

#13871 new New feature

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

Reported by: chadc Owned by: nobody
Component: contrib.admin Version: 1.2
Severity: Normal Keywords: list_editable, admin, ForeignKey, admin efficiency
Cc: chadcogar@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Overview

Including ForeignKey fields in list_editable results in m*n database queries where m is the number of ForeignKey fields in list_editable and n is the number of rows in the changelist.

Description

I have been experiencing poor performance with the admin changelist 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. The problem, as I understand it, stems from the fact that the choices for the ForeignKey widgets are not cached. So, when each ForeignKey widget is rendered, it queries the database to retrieve the list of possible values.

My solution to this problem has been to override the Select widget with a CachedSelect widget in the admin model. However, as Jeremy Dunck noted in django-developers (link below), this may stem from a more general problem with ModelFormSet. I have ticketed this under contrib.admin for now, but I hope that Jeremy will update it as any larger issues become clear.

Example

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 ForeignKey widgets in this example requires n*m=25*1=25 database queries:

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

Related Links

django-developers: http://groups.google.ca/group/django-developers/browse_thread/thread/76066baed6ba70dc

django-users: http://groups.google.ca/group/django-users/browse_thread/thread/7b63fd40c891ec19

Attachments (0)

Change History (7)

comment:1 Changed 4 years ago by patrys

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Keep in mind the formset is using forms that can apply additional filtering to the data sets thus making each choice set unique. If you want to introduce some sort of caching, you pretty much have to cache by serialized query params. Also you need to make sure the cache is thread-safe and does not outlive the formset (your CachedSelect does not seem to ever invalidate the cache).

comment:2 Changed 4 years ago by chadc

Hmm. I had just intended to use CachedSelect as a temporary measure -- for my own purposes -- until the actual problem is addressed. I included it for the sake of anyone else who might be interested in a temporary fix. As I mentioned above, Jeremy seemed to indicate that there is some larger issue here of which I am not fully aware. I hope that someone with more knowledge of the problem might be willing to take a look.

In any case:

  1. How could you implement additional filtering? I simply use formfield_overrides = {models.ForeignKey:{'widget':CachedSelect()}} on any AdminForm with an editable ForeignKey. I have no idea how any additional filtering could be implemented, so please let me know. Also, would it use the same naming convention?
  1. The cache is invalidated when it hits name == 'form-0-<model_name>'.

comment:3 Changed 3 years ago by joni

  • Triage Stage changed from Unreviewed to Accepted

I can confirm this is actually happening.

Regarding the additional filtering, maybe you need to filter the hosts list by some property of each Account.

comment:4 Changed 3 years ago by graham_king

  • Severity set to Normal
  • Type set to New feature

comment:5 follow-up: Changed 3 years ago by uriel.bertoche

  • Easy pickings unset

It also happens when you try to add a object that has a foreign key with too many possible values.
Like, if I have a table users, with 700 entries, and a models that has users as a foreign key, when I try adding an entry for this models, it will make +700 sql queries to fill the select.

Is there any fix for this yet?

comment:6 in reply to: ↑ 5 Changed 3 years ago by jdunck

Replying to uriel.bertoche:

It also happens when you try to add a object that has a foreign key with too many possible values.
Like, if I have a table users, with 700 entries, and a models that has users as a foreign key, when I try adding an entry for this models, it will make +700 sql queries to fill the select.

Is there any fix for this yet?

That is fairly unrelated -- use raw_id_fields. This issue is about list_editable. The problem you're describing is on a different admin screen.

comment:7 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.