lionleaf / dwitter

Social network for short js demos
https://www.dwitter.net
Apache License 2.0
764 stars 69 forks source link

Remove extra num_comments annotation #477

Closed stianjensen closed 4 years ago

stianjensen commented 4 years ago

We already prefetch all the comments, since all comments are always rendered (even when a 'load more' button is shown). Therefore, we don't need to also annotate the num_comments on the queryset. I am unsure how much impact this annotation has on performance in production, since I probably don't have a similar enough setup when running locally, but I've confirmed that this refactor at least does not add any SQL queries (the count is 5 with and without this change - 7 when logged in).

According to statistics in New Relic, almost all the time in the HotDweetFeed (the frontpage) is spent executing 3 SQL queries touching the dwitter_comment table. That would be: 1) a COUNT query to check how many objects there are in total (for pagination) - this is done automatically by django - maybe we can optimize this away 2) The main select getting the list of dweets as well as annotations 3) a prefetch to actually load all related comments

With the free version of New Relic, I can't dig deeper into each of the variants or what part of the queries are slow. But I know that all annotations added to the queryset for use as part of (2) also get executed in (1). And this num_comments annotation at least seems safe to start by removing to see what impact it has before investigating which other things may be optimized.

When opening your Pull Request, we encourage you to do the following (you can add an X to check each task):