Opened 8 years ago

Closed 8 years ago

#26166 closed Cleanup/optimization (fixed)

Accessing a widget via an index on a BoundField results in unnecessary iteration

Reported by: David Seddon Owned by: nobody
Component: Forms Version: 1.9
Severity: Normal Keywords: forms, boundfield
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If you access a widget via its index on a BoundField, the BoundField will iterate over its entire set of widgets. This can cause large unnecessary overheads if you are accessing all the widgets in this way on, say, a ModelChoiceField with a large queryset, as subwidgets for the entire queryset will be generated for each widget.

This is related to this ticket: https://code.djangoproject.com/ticket/22745, and the fix here: https://github.com/django/django/commit/5e06fa1469180909c51c07151692412269e51ea3

The specific example where this caused problems for me was in a custom filter that helped pass the associated instance along with its associated checkbox widget in the template. (http://stackoverflow.com/a/27545910/1005499)

Example:

instance_widgets = []
index = 0
for instance in bound_field.field.queryset.all():
     # Accessing the widget here will generate subwidgets for items in the queryset
     widget = copy(bound_field[index])
     instance_widgets.append((instance, widget))
     index += 1

If we store the instantiated subwidgets on the BoundField instance then we reduce this overhead:

# django/forms/boundfield.py

...
class BoundField(object):
...
    def __getitem__(self, idx):
        if not isinstance(idx, six.integer_types):
            raise TypeError
        # Prevent unnecessary reevaluation when accessing BoundField's attrs
        # from templates.
        if not hasattr(self, '_stored_subwidgets'):
            self._stored_subwidgets = list(self.__iter__())
        return self._stored_subwidgets[idx]

I'm not sure if this is necessarily the correct thing to do, but I thought it worth reporting. An alternative would be just to give a warning if widgets are being accessed in this way.

Change History (4)

comment:1 by Tim Graham, 8 years ago

Triage Stage: UnreviewedAccepted

I'm not sure about this either, but if we have a tested patch we can evaluate it.

comment:2 by Baptiste Mispelon, 8 years ago

It might be worth it to effectively cache BoundField.__iter__() but in your case, can't you use zip as a workaround:

for widget, instance in zip(bound_field, bound_field.field.queryset.all()):
    instance_widgets.append((instance, copy(widget)))

comment:3 by Tim Graham, 8 years ago

Does the patch for #27002 solve this?

comment:4 by Tim Graham, 8 years ago

Resolution: fixed
Status: newclosed

Please reopen if not.

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