Opened 10 years ago
Closed 9 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 , 10 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 by , 9 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)))
I'm not sure about this either, but if we have a tested patch we can evaluate it.