Optimize most pages, but especially the diff viewer and diff fragments.
Review Request #3219 — Created July 15, 2012 and submitted — Latest diff uploaded
Optimize most pages, but especially the diff viewer and diff fragments. This is another round of optimizations to reduce query counts on pages. In general, most pages will now have at least 1 less query. Some will have 2 or 3 fewer. The default session backend caused one query per view that always looked up the session in the database. This was one of the first queries executed. However, Django has another backend, cached_db, that will first attempt to look up the data in memcached, and if it doesn't find it, it will fall back to the database and then populate that data in memcached. It's much faster, and saves one query per page. The @valid_prefs_required decorator looked up the user's profile using get_or_create, meaning the profile never got cached for the user. This resulted in an additional profile query later on in most cases. We now use get_profile() optimistically, and fall back on logic to create it. This affects many pages on the site. The My Account page got similar optimizations to reduce query counts, as did get_enable_highlighting (used in diff generation). The big changes though are to the diff viewer/fragment code. In view_diff_fragment, we had one query for the DiffSet, another for the interdiff's DiffSet (if specified), another for the FileDiff, and another resulting one later for the FileDiff's DiffSet (since it wasn't cached). And that was just for the core diffviewer's view_diff_fragment. The wrapper in reviews added another one for the DiffSet and one for the interdiff's DiffSet. This was cleaned up to only have two queries total. One to fetch both DiffSets (or just one if not doing an interdiff) at once, and one to fetch the FileDiff. We grab the correct objects out of the query and we assign the diffset to the FileDiff so we don't look it up again later. view_diff_fragment now also accepts IDs instead of objects, so the reviews wrapper doesn't need to do a fetch and pass it off. This is a savings of 3-5 queries per fragment. The other savings is in _query_for_diff, which would call review_request.diffset_history and get_draft(user). The former was unnecessary, since all we needed was the ID, so we save a query by referencing the ID and not grabbing the object itself. The get_draft(user) was a bit redundant. Many of the call sites already had a draft. Now it no longer makes its own call, and instead requires that a draft be provided. All told, we save quite a few queries now, and every page (and API call) has at least 1 fewer query (assuming the session is in the cache). Some highlights: The diff viewer (minus subsequent fragments) went from 20 to 16 queries. Individual diff fragments went from 14 to 8. The My Account page went from 10 to 7.
Tested each of the pages pretty thoroughly. Diff viewer, individual fragments, interdiffs, My Account, dashboard, Submitters, Groups, APIs. I didn't see any regressions. Unit tests all passed.