Hi,

I think there's a potential to make this opt-in, and improve the out-of-box 
experience.

Summarising the discussion, it seems that the rough consensus is that if we 
were building the ORM from scratch, then this would be entirely sensible 
behaviour (with necessary per-QS ways to disable) - it would remove a 
common performance problem (N+1 queries), would improve areas where adding 
prefetch_related to queries is awkward, and in rare cases where it 
decreased performance there would be documented ways to fix.

So the main disagreement is about how to get there from here, and there's 
concern about three types of users:

1 - Existing, non-expert users whose sites work now (because otherwise they 
would have already fixed the problem) but who have lurking N+1 issues which 
will break them later
2 - Existing, non-expert users whose sites work now, but would have 
performance issues if this was enabled by an upgrade
3 - Existing, expert users who have already found and fixed the issues and 
who could therefore get no benefit but might suffer a performance 
degradation.

I'll assert that the size of these populations above is listed in roughly 
size order, with #1 being the biggest. This is a hunch based on most sites 
not having huge tables where N+1 becomes a problem - at least not until 
they've been running for a few years and accumulated lots of data...

There is another population that hasn't been considered - users starting 
django projects (ie people running django-admin startproject). Over time, 
this is by far the largest population. 

So would a sensible approach be:

- Feature is globally opt-in 
- startproject opts in for new projects
- Release notes mention the new flag loudly, and encourage people to try 
switching it on
- We add the debug tracing to help people find places where this setting 
would help - and encourage them to enable it globally before trying 
individual queryset.prefetch_related

Then over time, all new projects will have the new behaviour. Old projects 
will gradually upgrade - everyone in category 1 will hit the "make it work" 
switch the first time they see the warning / see a problem. Experts can 
choose how they migrate - as Adam points out, even experts can miss things.

Finally after a suitable warning period, this can become an opt-out feature 
and we arrive in the sunny world of an ORM that works better for all users.

Cheers,

Malcolm

On Wednesday, 16 August 2017 21:17:49 UTC+1, Aymeric Augustin wrote:
>
> On 15 Aug 2017, at 11:44, Gordon Wrigley <gordon....@gmail.com 
> <javascript:>> wrote:
>
> I'd like to discuss automatic prefetching in querysets. Specifically 
> automatically doing prefetch_related where needed without the user having 
> to request it.
>
>
>
> Hello,
>
> I'm rather sympathetic to this proposal. Figuring out N + 1 problems in 
> the admin or elsewhere gets old.
>
>
> In addition to everything that was said already, I'd like to point out 
> that Django already has a very similar "magic auto prefetching" behavior in 
> some cases :-)
>
> I'm referring to the admin which calls select_related() on non-nullable 
> foreign keys in the changelist view. The "non-nullable" condition makes 
> that behavior hard to predict — I'd go as far as to call it non 
> deterministic. For details, see slide 54 of 
> https://myks.org/data/20161103-Django_Under_the_Hood-Debugging_Performance.pdf
>  and 
> the audio commentary at https://youtu.be/5fheDDj3oHY?t=2024.
>
>
> The feature proposed here is most useful if it's opt-out because it 
> targets people who aren't aware that the problem even exists — at best they 
> notice that Django is slow and that reminds them vaguely of a rant that 
> explains why ORMs are the worst thing since object oriented programming.
>
> It should kick in only when no select_related or prefetch_related is in 
> effect, to avoid interfering with pre-existing optimizations. It's still 
> easy to construct an example where it would degrade performance but I don't 
> think such situations will be common in practice. Still, there should be a 
> per-queryset opt-out for these cases.
>
> We may want to introduce it with a deprecation path, that is, make it 
> opt-in at first and log a deprecation warning where the behavior would 
> kick-in, so developers who want to disable it can add the per-queryset 
> opt-out.
>
>
> At this point, my main concerns are:
>
> 1) The difficulty of identifying where the queryset originates, given that 
> querysets are lazy. Passing objects around is common; sometimes it can be 
> hard to figure out where an object comes from. It isn't visible in the 
> stack trace. In my opinion this is the strongest argument against the 
> feature.
>
> 2) The lack of this feature for reverse one-to-one relations; it's only 
> implemented for foreign keys. It's hard to tell them apart in Python code. 
> The subtle differences, like return None vs. raise ObjectDoesNotExist when 
> there's no related object, degrade the developer experience.
>
> 3) The strong opinions expressed against the feature. I'm not sure that 
> consensus is within reach. If we can't agree that this is an adequate 
> amount of magic, we're likely to stick with the status quo. I'd rather not 
> have this question decided by a vote of the technical board.
>
>
> In the grand scheme of things, going from "prefetching a related instance 
> for an object" to "prefetching related instances for all objects in the 
> queryset" isn't that much of a stretch... But I admit it's rather scary to 
> make this change for all existing Django projects!
>
>
> Best regards,
>
> -- 
> Aymeric.
>
>
>
>
>

-- 
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 
https://groups.google.com/d/msgid/django-developers/7e2115fe-dd61-4a64-8600-f3fe62fb687c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
    • R... Alexander Hill
      • ... Josh Smeaton
        • ... Adam Johnson
          • ... 'Tom Evans' via Django developers (Contributions to Django itself)
            • ... Shai Berger
              • ... Josh Smeaton
              • ... Gordon Wrigley
        • ... Anssi Kääriäinen
  • Re: Au... Aymeric Augustin
    • R... Gordon Wrigley
    • R... Malcolm Box
      • ... Andrew Godwin
        • ... Collin Anderson
    • R... Aymeric Augustin
      • ... Anssi Kääriäinen
        • ... Shai Berger
          • ... Collin Anderson
          • ... Luke Plant
            • ... Tobias McNulty
              • ... Shai Berger
        • ... Todor Velichkov

Reply via email to