• 
      

    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. Show all issues

      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:
    Completed
    Change Summary:
    Pushed to release-2.5.x (c9c7fc5)