Restructure DiffFragmentView to improve caching and extensibility.
Review Request #7169 — Created April 5, 2015 and submitted — Latest diff uploaded
DiffFragmentView
and the main subclass,ReviewsDiffFragmentView
,
performed a number of expensive operations prior to the point where we
could generate an ETag. When we did reach that point, we generated an
ETag based off the cache key, which needed a lot of information that was
otherwise available in the URL. On top of all this, we stored a lot of
state (such asFileDiff
s) on the instance, making it hard to extend this
class with a version that could generate fragments for multiple
FileDiffs.Now, a lot more of the work happens in the base
DiffFragmentView
. The
responsibilities of the different functions are a bit more clear (for
instance,create_renderer
is no longer trying to queryDiffSet
/FileDiff
instances).ReviewsDiffFragmentView
has a lot less it must override, and
what it does need to override is a lot more controlled.The end result is a more organized view that can perform very fast ETag
checks with very fast responses, bringing the total request/response
time for cached pages into the single-digit millisecond range (at least
locally -- probably double digits otherwise).Some future work will build upon the reorganization to let us stream
diff fragments in a single HTTP request, avoiding a lot of connection
and database query overhead.
Tested with a bunch of diffs.
Unit tests pass.
I checked that 304s were very quickly returned with minimum code execution
if the diff was browsed before.