Drastically reduce the number of SQL queries made by the ratings API (bug 1135979) #2937
Conversation
| # FIXME: This is only right when ?app= filtering is applied, if no | ||
| # filtering or if ?user= filtering is done, it's completely wrong. | ||
| @property | ||
| def count(self): |
There was a problem hiding this comment.
count is used 4 times in django's paginator code. (The default paginator uses _count to calculate the results once). This means that at least 4 times, we loaded the whole object list, and 4 times, we loaded the related addon instance. Because the our models have transformers loading extra stuff, that's roughly 25 extra queries per review in the list.
diox
force-pushed
the
ratings-api-perf
branch
from
4489dff
to
9666b55
Compare
| # `addon` property on the instance with it, saving some costly queries. | ||
| app = getattr(self, 'app', None) | ||
| if app is not None: | ||
| obj.addon = app |
There was a problem hiding this comment.
Saves us ~ 16 queries per review.
|
We're saving a lot of queries there, this should improve performance quite a bit. We still need to check indexes, but that can be done separately. I still need to test whether cache-machine will like the |
diox
force-pushed
the
ratings-api-perf
branch
2 times, most recently
from
0ea91be
to
27db069
Compare
diox
force-pushed
the
ratings-api-perf
branch
from
27db069
to
dbfa1d6
Compare
| self.app.update_version() | ||
|
|
||
| reset_queries() | ||
| with self.assertNumQueries(5): |
There was a problem hiding this comment.
When I started, this test did more than a hundred queries.
|
I read it over a few times. r+ |
|
Checked with cache-machine enabled and it seems to be doing the right thing. |
Drastically reduce the number of SQL queries made by the ratings API (bug 1135979)
https://bugzilla.mozilla.org/show_bug.cgi?id=1135979