[PATCH 2 of 5] findrenames: Optimise "addremove -s100" by matching files by their SHA1 hashes

[Benoit Boissinot]

On Sun, Mar 21, 2010 at 9:21 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
> David Greenaway wrote, On 03/21/2010 08:33 AM:
>>

Thanks for the additional review Mads.

>> # HG changeset patch
>> # User David Greenaway<hg-dev at davidgreenaway.com>
>> # Date 1269151891 -39600
>> # Node ID 153c602f7b20c95484033d2b91d05171f62f91b2
>> # Parent  8bf2c8f1b35705f4f5b615aaf954a3cb034c5dc0
>> findrenames: Optimise "addremove -s100" by matching files by their SHA1
>> hashes.
>>
>
>> +    addedfiles = [x for x in addedfiles if x.size()>  0]
>> +    removedfiles = [x for x in removedfiles if x.size()>  0]
>>
>
> Why not do this while building the "sets" in the first place?
>
>> +    # Find exact matches.
>> +    handledfiles = set()
>> +    for (a, b) in _findexactmatches(repo, addedfiles, removedfiles):
>> +        handledfiles.add(b)
>> +        yield (a, b, 1.0)
>>
>
> Shouldn't "a" be "removed" from removedfiles as well?
>
> Perhaps it would be simpler to let addedfiles and removedfiles be sets from
> the beginning and just remove handled files here. (The sets could perhaps be
> converted to (non-mutated) lists in the call to _findexactmatches.
>

Indeed, at least the correct code doesn't yield sorted elements, so it
should be fine.