Improve performance of the diff complexity icon rendering.

Review Request #8887 — Created April 6, 2017 and submitted

Information

Review Board
release-2.5.x
961bc47...

Reviewers

The diff complexity icon had some performance issues that manifested
when working with larger files. When rendering the icon, we get the line
counts for each type of diff chunk (inserts, deletes, replaces, equals)
and pass them to flot. It turns out that the higher the numbers you
provide, the more flot has to do to render the graph, despite the graph
being a small basic pie chart.

To address the performance issue, we now normalize the values ourselves,
scaling down the totals to cap to 365 and ensuring all the segments are
of the correct percentage of that total.

We also have a performance improvement in the code that calculates the
colors. It was building a DOM element and attaching it to the document,
getting the styles from that, and then removing it. That could impact
rendering time of the page, and if the browser was otherwise very busy
(placing a very large diff on the page, for instance), color calculation
could take too long. We now make sure this is hidden before attaching to
the document, which avoids the rendering penalties.

This can make a significant impact on the rendering performance of the
diff viewer.

Tested with very large diffs and measured the performance. There was a
significant drop in rendering time.

Compared the rendered icons between the old and new code for about 15
diffs of various sizes. The renders were identical.

Description From Last Updated

365 -> 360 :)

brenniebrennie
brennie
  1. 
      
  2. 365 -> 360 :)

  3. 
      
gmyers
  1. 
      
  2. As far as I can tell, innerRadius is always in the range [0.0, 0.5], so I'm not sure I see the need for the min().

    1. I thought I had hit an issue with the value, but you're right. It should be within that range.

  3. 
      
chipx86
gmyers
  1. Ship It!
  2. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.5.x (c9c7fc5)
Loading...