Opened 9 years ago

Closed 9 years ago

#5570 closed (fixed)

Improve performance of generic relations

Reported by: Manuel Saelices Owned by: Jacob
Component: Database layer (models, ORM) Version: master
Severity: Keywords: generic relations GenericForeignKey
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: UI/UX:

Description

Every access of an object relates to another object with generic relations send two SQL statements. I think it should be cached more efficiently.

Look at this example model:

class Note(models.Model):
    content_type = models.ForeignKey(ContentType)
    object_id = models.IntegerField()
    related_object = generic.GenericForeignKey('content_type', 'object_id')

class Project(models.Model):
    name = models.CharField(maxlength=50)

Imagine there are two notes asociated both to one project object (not necessary the same object).

Now look at this code:

>>> from django.db import connection
>>> [ n.related_object for n in Note.objects.all() ]
[<Project: Equipo Yaco Community>, <Project: Equipo Yaco Community>]

>>> connection.queries
[{'sql': u'SELECT "myapp_note"."id","myapp_note"."content_type_id","myapp_note"."object_id" FROM "projects_note"',
  'time': '0.003'},
 {'sql': u'SELECT "django_content_type"."id",... FROM "django_content_type" WHERE ("django_content_type"."id" = 22)',
  'time': '0.002'},
 {'sql': u'SELECT "django_content_type"."id",... FROM "django_content_type" WHERE ("django_content_type"."id" = 22)',
  'time': '0.002'},
 {'sql': u'SELECT "myapp_project"."id","myapp_project"."name" FROM "myapp_project" WHERE ("myapp_project"."id" = 1)',

As you can see, there are two identical SQL sentences (number 2 and 3). But with the patch uploaded, these two sentences reduces in one.

This can be a good improvement for apps that uses intensively generic relations. In the best of cases, can reduce 50% of SQL sentences in a loop of adquiring related objects.

Attachments (1)

generic_relations_r6403.diff (5.0 KB) - added by Manuel Saelices 9 years ago.
Patch that improves generic relations performance

Download all attachments as: .zip

Change History (11)

Changed 9 years ago by Manuel Saelices

Patch that improves generic relations performance

comment:1 Changed 9 years ago by Manuel Saelices

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Status: newassigned

comment:2 Changed 9 years ago by Michael Radziej

Has patch: unset
Resolution: invalid
Status: assignedclosed

This is not a bug, it's normal behaviour. If you have multiple notes refering to the same content type, you get multiple queries. If you want it changed, you need to use caching in your application. This is the same for generic foreign keys or normal foreign keys.

You can't really simply add a cache for this without proper cache invalidation etc.

If you disagree, please discuss this at the developer mailing list.

comment:3 Changed 9 years ago by Manuel Saelices

I know thats is the normal behaviour. But for example on [source:django/contrib/contenttypes/models.py django/contrib/contenttypes/models.py] does same type of caching. And there are a lot of caching in other files. Remember, I don't use django cache framework. I use a simple dictionary for caching. It's not the same.

comment:4 Changed 9 years ago by Michael Radziej

Resolution: invalid
Status: closedreopened

I retract ... and go to bed ;-)

Sorry.

comment:5 Changed 9 years ago by anonymous

i also noticed that there are some indentical queries in the same page if generic relations was used.

comment:6 Changed 9 years ago by Simon G <dev@…>

Has patch: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

anonymous: are you saying that there are other instances of these duplicate queries?

msaelices: well spotted, thank you, but why are you adding/deleting blank lines in the patch?

comment:7 in reply to:  6 Changed 9 years ago by Manuel Saelices

Replying to Simon G <dev@simon.net.nz>:

anonymous: are you saying that there are other instances of these duplicate queries?

msaelices: well spotted, thank you, but why are you adding/deleting blank lines in the patch?

I only remove ugly white spaces... sorry, I hate put spaces for nothing.

One question. Why patch needs improvement? I think is good.

comment:8 Changed 9 years ago by Jacob

Needs documentation: set
Needs tests: set
Owner: changed from Manuel Saelices to Jacob
Status: reopenednew

comment:9 Changed 9 years ago by Jacob

(In [7216]) Beefed up caching of ContentType lookups by adding ContentType.objects.get_for_id(). This shares the same cache as get_for_model(), and thus things that do lots of ctype lookups by ID will benefit. Refs #5570.

comment:10 Changed 9 years ago by Jacob

Resolution: fixed
Status: newclosed

(In [7228]) Fixed #5570: generic foreign keys no longer do multiple lookups on the content type. This uses the new ctype caching bit added in [7216].

Note: See TracTickets for help on using tickets.
Back to Top