Deduplicate links on checkout

[pared]

In #2016
We touched the subject of deduplicating links when re-adding dependencies.

Currently, checkout, due to this check we will not recreate link, even if particular path_info link type is different than cache.type one. checkout should re-checkout with proper link type and protection level.

@pared , is this related to this comment? #2358 (review)

[pared]

@MrOutis Yes

[dmpetrov]

@pared could you please clarify - is this a bug or we just talking about different link-types (which might lead to some bugs)?

[efiop]

Need to not forget to check performance after this is implemented.

[pared]

@dmpetrov
This is the same situation that we have been working on #2016. When doing checkout, we only check whether file checksum did not change.

What we should do is to resotore file "state".
What I mean by that could be explained with example:

• we have repo with hardlink cache type
• we unprotect some file, its copy is created
• upon checkout this file, we should "restore" this file as hardlink, so that it reflects current repository configuration

I guess you could say it is a bug.

[efiop]

Keeping open until benchmark results are confirmed.

[pared]

Ok, so here are results of my performance test:

The test environment:

• add dir with 100K files
• run status to trigger unpacked dir creation

Test scenarios:

1. just checkout data
2. remove one file and then checkout
3. (hardlink cache) unprotect one file and then checkout

Naming:

• old - previous implementation that was using remote/base.changed inside remote/base.checkout to check whether checked-out target has changed
• new - not checking with changed, relies on _checkout_file down the line to verify if something changed

Results:
Scenario 1:

• old: 4 s
• new: 40 s

Why so? :
No metadata changed in data dir (mtime, size, checksum), so self.changed just checked mtime and size and skipped iterating over all files and checking one by one.
New implementation was slower, because each file has been checked.

old: 1 new: 0

Scenario 2:

• old: 58 s
• new: 42 s

Why so?:
One file has been removed so each "metadata" has changed. Old implementation had to calculate modified dir md5 to verify that something has actually changed, and then had to _checkout_file each file, one by one.
New checkout went straight to the _checkout_file, so saved some time on dir md5 calculation.

old: 1 new: 1

Scenario 3:

• old: 20 s
• new: 41 s

Why so?
mtime and size has changed, so old implementation recalculated md5, it did not change, skip _checkout_file
New, as always went straight to _checkout_file so it took the same amount of time as previously

I would call draw here, because old implementation was faster, but it did not deduplicate unprotected file, which it should.

My thoughts: I think removing self.changed was not the optimal move. So is leaving it. What we should do is to rely on mtime and size check, if it did not change, skip one by one _checkout_file. If mtime and size changed, proceed to one by one _checkout_file.
Did I miss something? @efiop?