optimize dir checkout

[efiop]

#2444 (comment)

[Suor]

If we include mode and link type into dir checksum then we will be able to still use dir.changed to shortcut everything. The question is how much slower that will dir checksum make.

Side consideration. We should really add some performance metrics, this becomes less and less manageable to track.

[efiop]

@Suor if we would include that, then things like dvc status will start reporting those files as changed, which is not really desirable, since for status/repro we don't really care if the file is of correct type. We only care about it on re-adding and checkout, really. But we could definitely look in that direction to optimize it along the way.

Side consideration. We should really add some performance metrics, this becomes less and less manageable to track.

Agreed, we've been talking about setting that up on, for example , dvc-test repository, but just didn't have the time to deal with it yet, unfortunately 🙁

[Suor]

I think bench dir here would be easier to maintain then some separate repo.

[efiop]

For the record, discussed dir vs dvc-test approaches and didn't reach the consensus. To me, it makes sense to have those in dvc-test where they will actually be run regularly and their development won't create noise on the main core repo.

[efiop]

Need to do the research, figure out what is slowing us down and what are the options for us.

[pared]

Seems that this issue might evolve into few smaller ones.
In my opinion, we could start by trying to shortcut the directory check described in original comment For checking out unchanged directory.

In #2488 we dropped shallow dir checksum check, which caused us to mark dirs that might have changed access level or link type as unchanged.

I think that we could implement kind of shortcut for current dir checkout, at least in case of "hardlink" and "symlink".
In those cases, we could simply store directory mtime and size in state db, retrieve it and compare it with existing upon checkout. In case of access change (dvc unprotect data_dir/file) link type of file will change, so will mtime and size.

Regretfully this case is not as easy for "copy" and "reflink", since unprotecting will result only in access rights change which is not reflected in get_mtime_and_size.

In the latter cases, it does not seem to me that we would be able to check whether any file did have its file access level changed without checking each one of them.

I also did performance tests for the case described in the aforementioned comment, respectively for each link type. Interestingly, adding 100K files, remove 1 and checkout for symlink cache type seems to take twice as much time as hardlink or copy, need to investigate why.

[pared]

To keep history: we (with @efiop) discussed removing write permissions from directory upon dvc add dir in case of protected repo. This idea seems ok, for checks whether directory was modified, though with the current unprotect implementation it has a big drawback that cannot be ignored:

• if someone would like to add some files to her/his dataset, one would need to unprotect the whole dir, resulting in copying all files inside
  For now, this idea is dropped.

[pared]

One thing that could be optimized is reintroducing self.changed check that was removed here
But to make sure we also check link type changes and write permision changes, we should also gather mode for all files in dir and verify it is consistent with self.protected. Other thing to check would be that mtime did not changed for dir (if checksum and permission did not changed, changed mtime indicates that we are using *link cache type and that we did unprotect one or more files).

Reintroducing this check in new form would make sure that dir checkout for unchanged dir is faster.

[pared]

Another one (thanks to @efiop) : introduce {X}PoolExecutor to remote.base._checkout_dir.