search

dominno / django-moderation

django-moderation is reusable application for Django framework, that allows to moderate any model objects.
BSD 3-Clause "New" or "Revised" License
269 stars 91 forks source link

Huge performance hit on large queries #70

Closed mgerring closed 11 years ago

mgerring commented 11 years ago

Without django_moderation:

146.66 ms (6 queries)

With django_moderation:

453.57 ms (865 queries)

What was one query became hundreds, mainly because of statements like this:

SELECT "moderation_moderatedobject"."id", "moderation_moderatedobject"."content_type_id", "moderation_moderatedobject"."object_pk", "moderation_moderatedobject"."date_created", "moderation_moderatedobject"."moderation_state", "moderation_moderatedobject"."moderation_status", "moderation_moderatedobject"."moderated_by_id", "moderation_moderatedobject"."moderation_date", "moderation_moderatedobject"."moderation_reason", "moderation_moderatedobject"."changed_object", "moderation_moderatedobject"."changed_by_id" FROM "moderation_moderatedobject" WHERE ("moderation_moderatedobject"."object_pk" = 597 AND "moderation_moderatedobject"."content_type_id" = 7 )

This is completely insane. Am I doing something wrong here, or is this expected behavior?

treyhunner commented 11 years ago

Knowing the query (or at least the type of query) would help others resolve the issue you're experiencing.

mgerring commented 11 years ago

MyModel.objects.all()

Reading through the docs it looks like there's one extra lookup per item in the queryset. There must be a better way to do this. What about fetching the status of all the items in one query and matching them in Python insteadof trying to do this at the DB level?

treyhunner commented 11 years ago

That does seem inefficient. It does sounds like this needs to be rewritten somehow.

Maybe this could be optimized to use a select_related() somehow? Your suggest of using an aggregate on multi-object queries could work also.

aldarund commented 11 years ago

When i`m trying to get only 1 instance with objects.get(pk=1) it will still query ALL existing records in my table one by one. Thats unusable =(

ebrelsford commented 11 years ago

Does anyone have a fork that tries to fix this?

ebrelsford commented 11 years ago

Took a stab at it using a subquery in 56cf4cd9. filter_moderated_objects was a bit convoluted, and calculating changes in models on the fly seems unadvisable.

I'm testing it out--so far it reduces the number of queries to 1, which is a bit faster. Thoughts?

dominno commented 11 years ago

You need to use the visibility_column on moderator class for better performance.