• 
      

    Greatly improve performance of the diff parser.

    Review Request #8880 — Created April 6, 2017 and submitted — Latest diff uploaded

    Information

    Review Board
    release-2.5.x
    d8c19be...

    Reviewers

    The diff parser had some major performance problems that didn't really
    affect standard diffs, but were absolutely noticeable for multi-megabyte
    diffs.

    The primary performance problems had to do with the building of the diff
    data strings. We were concatenating strings, which is not fast or
    memory-efficient. To solve this, a breaking change to the diff parser
    API had to be made (which is unlikely to affect too many people out
    there). We no longer work with the data string, but instead call methods
    to prepend or append data to the diff's ParsedDiffFile object (renamed
    from File, while we're breaking stuff anyway -- further changes will be
    coming in for parsing/attribute/doc improvements). These methods in turn
    work on a cStringIO, which is a much faster way of performing the string
    building.

    Along with this, there were many places where we did conditional lookups
    or range checks to determine whether to process one or more lines of
    content. These checks were preventing us from unintentionally indexing
    past the end of the diff. Since in each case it's more likely than not
    that the current line will be somewhere before the end of the diff, the
    parser now attempts the index unconditionally, and handles alternative
    logic if hitting an IndexError.

    Prior to these speedups, a 13MB diff took so long to parse that I gave
    up trying to time it. After, it took a few seconds. This should
    certainly help lighten the load on busy Review Board servers, and may
    even make general uploading of diffs faster in many real-world cases.

    All unit tests passed.

    Uploaded a very large diff. Saw that it parsed correctly in seconds.