Make the diff viewer load diffs progressively

Review Request #721 — Created Jan. 28, 2009 and submitted

Information

Review Board SVN (deprecated)
trunk
692

Reviewers

One of the slowest parts of Review Board is the diff viewer. It takes a while to download every file, patch them, perform the diff operation, and render the result. This means that for large diffs especially, users have to wait a while until they can see the diff. This is worsened due to the fact that the entire diff is processed even if it's split across pages, which really slows things down and makes it nearly impossible to review really large diffs.

This change makes the initial diff viewer page load immediately, with the actual diff content loading progressively after the page has loaded. The diff viewer page itself just loads the diff metadata from the database and passes it to the template, which sets up fake diff tables with spinners and kicks off an async load chain for the diffs.

This means a few things:

 * We only ever load the files we explicitly request, when we request them.
 * Users can start reviewing the change a lot faster, since the first file should load quickly and render, and by the time they finish reviewing that, the other files will probably have loaded.
 * In theory, a diff can span 100 pages without really slowing things down, solving one of the big problems we've had for a while now.

Next steps after this would be to improve browser-side caching of these diff fragments, which I'll handle in an upcoming change.
Tested several diffs of various sizes, as well as interdiffs. Navigation worked, reviewing worked, and comments loaded correctly. I couldn't find any regressions.
david
  1. 
      
  2. Can you add an error handler here? I'd like to see something graceful happen if a patch doesn't apply cleanly.
  3. 
      
chipx86
Review request changed

Change Summary:

Added diff fragment errors to the diff viewer and improved the ones for the review request page. These include the error info and a traceback (collapsed by default, but can be expanded by clicking "Details").

This also fixes navigation when diff fragments failed to load, and fixed a double-load of diffviewer.js.

Diff:

Revision 2 (+344 -167)

Show changes

david
  1. Looks awesome.
  2. 
      
Loading...