Hi François, 

Thank you for a good write-up of the issue. 

Having looked at this from each direction I can't see a way of squaring all 
the desires here.

I see the possibility of an inconsistency between the __len__() and 
__iter__() calls. This would certainly be nice to avoid but I'm not sure it 
is the overriding concern here. 

I can easily see the memory issue being more significant than you're giving 
it credit for. 

The original issue (#23623) cited a ≈25% reduction in peak memory usage 
moving to the iterator. Choices are used in validation and not just 
template rendering. If not reset, the queryset is retained for the lifetime 
of the form instance, not just the ModelChoiceIterator iteration. With a 
large formset, with forms using ModelChoiceField on foreign keys with many 
thousands of records, the retained queryset would likely add up quickly. 

As such, I don't think we can just switch the default implementation. (I 
think we'd quickly see a lot of complaints.)

I **do see** the appeal of re-using the queryset for a whole lot of cases. 
Many times I would trade a bit more memory for the lifetime of a form 
instance for one less query. (Although, just personally, I'm not sure how 
often `if field.choices` actually comes up, for me.)

So, conclusions (with _I think we should…_ etc elided):

* Close your PR as is. https://github.com/django/django/pull/9867 — We 
can't just change the behaviour. 
* Look at Tim's PoC PR. https://github.com/django/django/pull/8649 — Can we 
implement __len__() to not _fetch_all()? Can we implement the custom 
__bool__ to avoid `count()`? Can we do that without an extra DB query? 
* If we **can't** avoid the extra query then we have to decide whether it's 
an acceptable price? 
    * wontfix: If it's not, the existing behaviour is _OK_: it's not 
incorrect for __len__() to _fetch_all(), and your previous fix makes sure 
we don't waste that effort if it has been done. 
    * Fix: I'd guess the extra query probably is OK, especially with an 
optimised __bool__ just checking existence. It's the price of consistency 
* Can we then document, and maybe provide, an alternate ModelChoiceIterator 
that just re-uses the queryset? 
    * Advantages of this are consistency, fewer DB hits etc. 
    * Cost is the higher memory use.
That's my take on the issue thus far. 

Kind Regards,


You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
For more options, visit https://groups.google.com/d/optout.

Reply via email to