Code

Opened 2 years ago

Closed 2 years ago

#17535 closed Cleanup/optimization (fixed)

list_detail call to len(queryset) should be queryset.exists()

Reported by: hedleyroos@… Owned by: aaugustin
Component: Generic views Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Refer to https://code.djangoproject.com/browser/django/trunk/django/views/generic/list_detail.py#L97. The line if not allow_empty and len(queryset) == 0 executes slowly for large querysets. We are already dealing with a real Queryset and not just any iterable (there is an earlier call queryset._clone) so we can use the exists method. I propose if not allow_empty and not queryset.exists().

Attachments (0)

Change History (3)

comment:1 Changed 2 years ago by claudep

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

The code you mention is in deprecated code. Moreover, this queryset evaluation is done in the non paginated section of the context build. The query, even if large, would be evaluated anyway later in the code. So this part seems a 'won't fix' for me.

However, the issue is similar in the new class-based view code (https://code.djangoproject.com/browser/django/trunk/django/views/generic/list.py#L116). And in this case, the len(queryset)==0 is done before the pagination take place. So this might be an issue.

If you take the case of a costly query with not so many results, your proposal would call the query twice (once for exists and once for the real query), hence resulting in poorer performance when the query is not empty. Your solution might be better for a quick query that returns many results. The real solution would be probably to follow the same logic than the old function-based view and do this test later in the process (get_context_data).

comment:2 Changed 2 years ago by aaugustin

  • Owner changed from nobody to aaugustin

#18087 was extremely similar, let's apply the same technique here.

comment:3 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

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

In [009e237cf0c25ce13e73d58feb54ef4f86e47e4e]:

Fixed #17535 -- Optimized list generic views.

When allow_empty is False, prevented the view from loading
the entire queryset in memory when pagination is enabled.

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.