-
-
Tiny optimization, for both functions: def func(self): review_request = self.review_request for x in self.xs.all(): x._review_request = review_request I also tried using a map for these functions, something like: map(lambda x: setattr(x, '_review_request', review_request), all) but after some quick tests, the for loop still performs better. -
The placement of "is_reply.boolean = True" seems weird or out of place. Also, shouldn't it be: is_reply = self.is_reply() or something of the sort?
-
-
Greatly optimize the review request and diff viewer pages.
Review Request #3160 — Created June 25, 2012 and submitted
The review request page and diff viewer pages have been producing quite a large number of queries, and this count goes up as the review/comment counts go up. This change precomputes a lot of data for the review request and diff viewer pages in order to reduce those queries. Some things that were previously computed as individual queries per-review are now filtered and categorized out of single queries up-front. Some models, like BaseComment and Review, now have accessors that wrap access to related objects. By default, these are equivalent to accessing the object directly, but can be short-circuited by setting special variables on the model, which the review request and diff viewer pages do. There were also many places in templates where we only needed the ID of a related object, but we'd still access the entire object just to get it. Now we use the IDs directly. With this change, a small review request has gone from 60 queries (total render time of 506ms) to 24 (180ms). A large one went from 396 queries (3.1s) to 24 (348ms) The diff viewer on the large change went from 270 queries (2.8s) to 24 (330ms).
Tested with several review requests with lots of moving parts, and even compared HTML output. See the timing above for examples. We'll need to test this on reviews.reviewboard.org a fair bit before we release, just to make sure there isn't any fallout.
| Description | From | Last Updated |
|---|---|---|
|
Tiny optimization, for both functions: def func(self): review_request = self.review_request for x in self.xs.all(): x._review_request = review_request I also tried … |
ME medanat | |
|
The placement of "is_reply.boolean = True" seems weird or out of place. Also, shouldn't it be: is_reply = self.is_reply() or … |
ME medanat | |
|
Why are you asserting False here? |
ME medanat | |
|
This doesn't seem right. |
 david
|
|
|
Alphabetical order, unless these imports follow a different order. |
ME medanat |
ME
- Change Summary:
-
* Removed a debugging assert. * Reordered imports. * Cache a review_request lookup before a loop, in order to avoid going through the ForeignKey's cache.
- Diff:
-
Revision 2 (+451 -128)