Reviews Of Large Changesets Are Slow And Unwieldy

Issue #523 resolved
William Roush created an issue

Doing a pull request for Redmine from default to 2.0.3 (so we can pretend "default" is "develop" and "2.0.3" is "default" for a roll out).

Marcin mentioned limiting the amount of changesets to 200 on #rhodecode earlier, I'm looking at 550 diffs @ 10 seconds.

My findings:

{{{ INFO [rhodecode.controllers.pullrequests] statuses: 1.10977005959 INFO [rhodecode.controllers.pullrequests] run differ: 2.42482995987 INFO [rhodecode.controllers.pullrequests] diff processor: 2.65571188927 INFO [rhodecode.controllers.pullrequests] prepare: 5.45235109329 INFO [rhodecode.controllers.pullrequests] files count: 554 INFO [rhodecode.controllers.pullrequests] load time: 9.63583803177 }}}

Diff processor is run on every page load, other then exporting the finished diff and storing it with the push request, is there a better way you can think of doing this?

Code is small, I'd be tempted to store it (pretty sure that is what other review systems do too).

It's taking about 4 seconds to generate the massive amount of HTML that is needed to display this change set... to help prevent this:

  • Load individual files via AJAX while on the review screen.
  • Paginate the review.

Typically when looking at a pull-request, I would figure it's more useful to see people have posted reviews, be able to dig into those individual reviews and reply, or navigate to a seperate page for reviewing.

A VERY rough idea would be something like this:

  • Changesets show a general overview summary unless clicked to view more.
  • Reviews loaded on click. ** When viewing a review, I'm only interested in what has been commented on.
  • Code can be viewed by clicking "Open a review". ** Keep code to a minimum per page, maybe 10-20 files.
  • General comment section at the bottom. ** I'm bad at mocks, pretend it's there, this took a bit of :effort:.

Ideas? Comments? Suggestions?

Comments (5)

  1. William Roush reporter

    Code is small, I'd be tempted to store it (pretty sure that is what other review systems do too).

    Ignore this, diffs can be calculated at runtime with pagination with absolutely no problems.

  2. Marcin Kuzminski repo owner

    Thanks for this, I like the ideas, to simplify the view, I'll review all this and comment soon.

  3. Marcin Kuzminski repo owner

    The new diff engine should be much faster in the cases, also it will cut-off really huge diffs. Can you try out latest beta and share the results ?

  4. Log in to comment