Code

Opened 7 years ago

Closed 6 years ago

#5570 closed (fixed)

Improve performance of generic relations

Reported by: msaelices 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 msaelices 7 years ago.
Patch that improves generic relations performance

Download all attachments as: .zip

Change History (11)

Changed 7 years ago by msaelices

Patch that improves generic relations performance

comment:1 Changed 7 years ago by msaelices

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:2 Changed 7 years ago by mir

  • Has patch unset
  • Resolution set to invalid
  • Status changed from assigned to closed

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 7 years ago by msaelices

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 7 years ago by mir

  • Resolution invalid deleted
  • Status changed from closed to reopened

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

Sorry.

comment:5 Changed 7 years ago by anonymous

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

comment:6 follow-up: Changed 6 years ago by Simon G <dev@…>

  • Has patch set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

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 6 years ago by msaelices

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 6 years ago by jacob

  • Needs documentation set
  • Needs tests set
  • Owner changed from msaelices to jacob
  • Status changed from reopened to new

comment:9 Changed 6 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 6 years ago by jacob

  • Resolution set to fixed
  • Status changed from new to closed

(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].

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


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

 
Note: See TracTickets for help on using tickets.