• 
      

    Optimize diff processing for Subversion.

    Review Request #12640 — Created Sept. 25, 2022 and submitted

    Information

    RBTools
    release-4.x

    Reviewers

    When generating Subversion diffs, RBTools does a lot of diff processing
    to:

    1. Ensure that added/deleted empty files are present
    2. Ensure renamed files have the right information
    3. Convert relative paths to absolute paths
    4. Filter for any excluded files

    Each step of this would iterate over the previous diff and then generate
    a new one. If the diff had, say, 10,000 lines, we'd parse and then
    re-build those 10,000 lines in almost every one of those stages, before
    finally returning the result. This was slow.

    This change converts each processing stage to a generator, allowing one
    pass through the diff. Each stage will iterate through and yield lines
    for the next stage, the result of which is iteratively joined into a
    final byte string.

    The stages themselves have been optimized a bit. We used to perform
    repeated lookups on SVNClient for the same attributes (compiled
    regexes and constants) on each loop. Those are now pulled out of the
    loops.

    A potential bug was also found and fixed that could have led to an
    infinite loop during diff generation when processing empty files. If a
    very specific (unlikely) set of conditions were met (deleted empty file,
    revision values of 0 for the diff operation, and no Revision found for
    the file in svn info), we'd repeate the loop, but with no advancement.
    That'd cause the same condition to be hit over and over. We now advance
    the iterator.

    Unit tests pass.

    Summary ID
    Optimize diff processing for Subversion.
    When generating Subversion diffs, RBTools does a lot of diff processing to: 1. Ensure that added/deleted empty files are present 2. Ensure renamed files have the right information 3. Convert relative paths to absolute paths 4. Filter for any excluded files Each step of this would iterate over the previous diff and then generate a new one. If the diff had, say, 10,000 lines, we'd parse and then re-build those 10,000 lines in almost every one of those stages, before finally returning the result. This was slow. This change converts each processing stage to a generator, allowing one pass through the diff. Each stage will iterate through and yield lines for the next stage, the result of which is iteratively joined into a final byte string. The stages themselves have been optimized a bit. We used to perform repeated lookups on `SVNClient` for the same attributes (compiled regexes and constants) on each loop. Those are now pulled out of the loops. A potential bug was also found and fixed that could have led to an infinite loop during diff generation when processing empty files. If a very specific (unlikely) set of conditions were met (deleted empty file, revision values of 0 for the diff operation, and no `Revision` found for the file in `svn info`), we'd repeate the loop, but with no advancement. That'd cause the same condition to be hit over and over. We now advance the iterator.
    82872b7df55c81c4524a67a696c3cdd8a620fdd0
    Description From Last Updated

    'typing.Iterable' imported but unused Column: 1 Error code: F401

    reviewbotreviewbot

    'typing.Iterable' imported but unused Column: 1 Error code: F401

    reviewbotreviewbot

    This needs the type.

    daviddavid

    This needs the type.

    daviddavid
    Checks run (1 failed, 1 succeeded)
    flake8 failed.
    JSHint passed.

    flake8

    chipx86
    Review request changed
    Change Summary:

    Fixed an issue that could occur with garbage data in the empty file processor.

    Commits:
    Summary ID
    Optimize diff processing for Subversion.
    When generating Subversion diffs, RBTools does a lot of diff processing to: 1. Ensure that added/deleted empty files are present 2. Ensure renamed files have the right information 3. Convert relative paths to absolute paths 4. Filter for any excluded files Each step of this would iterate over the previous diff and then generate a new one. If the diff had, say, 10,000 lines, we'd parse and then re-build those 10,000 lines in almost every one of those stages, before finally returning the result. This was slow. This change converts each processing stage to a generator, allowing one pass through the diff. Each stage will iterate through and yield lines for the next stage, the result of which is iteratively joined into a final byte string. The stages themselves have been optimized a bit. We used to perform repeated lookups on `SVNClient` for the same attributes (compiled regexes and constants) on each loop. Those are now pulled out of the loops. A potential bug was also found and fixed that could have led to an infinite loop during diff generation when processing empty files. If a very specific (unlikely) set of conditions were met (deleted empty file, revision values of 0 for the diff operation, and no `Revision` found for the file in `svn info`), we'd repeate the loop, but with no advancement. That'd cause the same condition to be hit over and over. We now advance the iterator.
    8a9fe1bc0095c50f45cfd225efd03f30e2982143
    Optimize diff processing for Subversion.
    When generating Subversion diffs, RBTools does a lot of diff processing to: 1. Ensure that added/deleted empty files are present 2. Ensure renamed files have the right information 3. Convert relative paths to absolute paths 4. Filter for any excluded files Each step of this would iterate over the previous diff and then generate a new one. If the diff had, say, 10,000 lines, we'd parse and then re-build those 10,000 lines in almost every one of those stages, before finally returning the result. This was slow. This change converts each processing stage to a generator, allowing one pass through the diff. Each stage will iterate through and yield lines for the next stage, the result of which is iteratively joined into a final byte string. The stages themselves have been optimized a bit. We used to perform repeated lookups on `SVNClient` for the same attributes (compiled regexes and constants) on each loop. Those are now pulled out of the loops. A potential bug was also found and fixed that could have led to an infinite loop during diff generation when processing empty files. If a very specific (unlikely) set of conditions were met (deleted empty file, revision values of 0 for the diff operation, and no `Revision` found for the file in `svn info`), we'd repeate the loop, but with no advancement. That'd cause the same condition to be hit over and over. We now advance the iterator.
    3f8f0c3abce3aa475d4da34feac65f9b427bcb36

    Checks run (1 failed, 1 succeeded)

    flake8 failed.
    JSHint passed.

    flake8

    david
    1. 
        
    2. rbtools/clients/svn.py (Diff revision 2)
       
       
       
      Show all issues

      This needs the type.

    3. rbtools/clients/svn.py (Diff revision 2)
       
       
       
      Show all issues

      This needs the type.

    4. 
        
    chipx86
    maubin
    1. Ship It!
    2. 
        
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-4.x (fbf07f4)