Hacker News Reader

Comment by Izkata | Hacker News Reader

> (usually people writing dumb code without realizing it's expensive)

Some years ago, one morning I gave a co-worker a recommendation on how to improve a loop that was unnecessarily hitting database through the Django ORM. He committed the fix that afternoon. Barely an hour later I accidentally reintroduced the exact same slowdown in the exact same loop when adding a different piece of data to it.

Soooo yeah, ORMs can be so simplistic it's too easy to do by accident even if you know exactly what's going on under the hood.

View on HN

That's a nice benefit of using async ORMs (not yet available in django), the db calls are explicit!

You can also commit to always use .values() in django. That makes it error out when you access non-prefetched entities

also, values is awesome for returning your models as dicts when that's all you need most of the time, avoiding all the overhead of objects

Something about select_related? Please do share.

My guess is accessing a related field within a loop causing a database request per iteration, e.g.

``` [book.author.name for book in Book.objects.all()] ```

That’d be my guess: prefetch_related is great but you need to guard it with something like an assertNumQueries test to avoid accidental regressions.

Funnily enough, I recently optimized some code along these lines.

The way I sped it up was to call `.values()` on the query, which serializes the data into a dict and prevented me from accidentally making subsueqent calls.

PS: Indent by 4 spaces for code formatting.

s/ident/indent/

Thanks :).

Maybe I spent too much time with Django already, but if I see anyone doing anything but Book.objects.values_list('author__name', flat=True) for this type of expression, I would mark it as a 3x WTF? in the code review.

I’d also allow prefetch_related if you’re using more than the most trivial data - no point in duplicating logic you have in your models if you have a method which generates something like a name, URL, etc. based on multiple fields.

As written, it's obvious you should be doing something else like `values()` or `values_list()`. You're much more likely to fall victim to this anti-pattern if it's done within a standard for-loop that has a bunch of other stuff going on. I just wrote it as a list comprehension to avoid having to muck about with formatting on my phone.

It wasn't anything that fancy, it's just the solution was something you usually try not to do so it just wasn't coming to mind for him. The data being looped over came from solr, and some of the fields were primary keys used in lookup tables in the database, for getting translated text. Instead of doing the lookup inline, load the entire table into a python dict before the loop (<100 rows for each of these tables) and do the lookup from the dict in the loop.

And like I said above, usually you don't just select out the entire contents of a table and handle it in the application, so I reflexively did the wrong thing as well, because of how easy it was to do with Django's ORM.

Reply on news.ycombinator.com