Repository: iterative/dvc

Issue:
Computing file/dir hashes multiple times (when reportedly only done once reported)
## Bug Report

Performed a dvc init --subdir in an example/DL folder (part of a larger project)
set to use common shared cache
Did not define remote (yet)

created examples/DL/dataset1 and examples/DL/dataset2 sub directories
cd examples/DL/dataset1
ran 2 dvc imports (images.dvc and labels.dvc from dataset1.git) - remote A

cd examples/DL/dataset2
ran 2 dvc imports (images.dvc and labels.dvc from dataset2.git) - remote B

The files were already in cache... During each import/received message indication "Computing file/dir hashes (only done once)".  In total this took ~40 minutes (though all files were already in cache).

cd examples/DL
dvc pull  (expecting it to say all files up to date), as this is a new dvc repo - not tracking anything other than the dvc imports.... But instead I get:
"Computing file/dir hashes (only done once)"- and waiting another 40 min.

dataset1.git and dataset2.git where unchanged between import and pull attempt

repeating dvc pull - same behavior (Computing hashes)

It appears if you issue the dvc pull command in a project that has a dvc import(s) - it ends up  recomputing the hashes every time? This doesn't seem right - is there a way to prevent?

dvc update - works as I would expect...  But this almost implies that you can't easily have a dvc project that tracks outputs (e.g. created model) - when you have dvc imports?  This doesn't sound right either. 

Verified that the dvc pull behavior is as expected in the dataset1 repo checkout

Also - I had set dvc config --global core.checksum_jobs 32
Top reveals a single core being used ~100% - but this machine is virtually idle (24 cores total).  I had assumed that option would perform the hash computation in parallel (there are certainly plenty of CPU and IO resources available) - but the option didn't seem to have much effect / if any.  
htop only revealed 3 threads/processes:

### Please provide information about your setup

**Output of `dvc version`:**

```console
DVC version: 1.6.1 (conda)
---------------------------------
Platform: Python 3.8.5 on Linux-4.15.0-64-generic-x86_64-with-glibc2.10
Supports: azure, gdrive, gs, hdfs, http, https, s3, ssh, oss
Cache types: hardlink, symlink
Repo: dvc (subdir), git
```

**Additional Information (if any):**

If applicable, please also provide a `--verbose` output of the command, eg: `dvc add --verbose`.


Discussion:

Patch:
diff --git a/dvc/cache/local.py b/dvc/cache/local.py
index 66cf4a660..d0a9ff894 100644
--- a/dvc/cache/local.py
+++ b/dvc/cache/local.py
@@ -437,7 +437,7 @@ class LocalCache(CloudCache):
 
     @index_locked
     def pull(self, named_cache, remote, jobs=None, show_checksums=False):
-        return self._process(
+        ret = self._process(
             named_cache,
             remote,
             jobs=jobs,
@@ -445,6 +445,18 @@ class LocalCache(CloudCache):
             download=True,
         )
 
+        if not remote.tree.verify:
+            for checksum in named_cache.scheme_keys("local"):
+                cache_file = self.tree.hash_to_path_info(checksum)
+                if self.tree.exists(cache_file):
+                    # We can safely save here, as existing corrupted files will
+                    # be removed upon status, while files corrupted during
+                    # download will not be moved from tmp_file
+                    # (see `BaseTree.download()`)
+                    self.tree.state.save(cache_file, checksum)
+
+        return ret
+
     @staticmethod
     def _log_missing_caches(hash_info_dict):
         missing_caches = [
diff --git a/dvc/command/plots.py b/dvc/command/plots.py
index 8aaf07d8f..2fa779669 100644
--- a/dvc/command/plots.py
+++ b/dvc/command/plots.py
@@ -86,7 +86,12 @@ class CmdPlotsShow(CmdPlots):
 
 class CmdPlotsDiff(CmdPlots):
     def _func(self, *args, **kwargs):
-        return self.repo.plots.diff(*args, revs=self.args.revisions, **kwargs)
+        return self.repo.plots.diff(
+            *args,
+            revs=self.args.revisions,
+            experiment=self.args.experiment,
+            **kwargs,
+        )
 
 
 class CmdPlotsModify(CmdPlots):
@@ -151,6 +156,13 @@ def add_parser(subparsers, parent_parser):
         help="Plots file to visualize. Shows all plots by default.",
         metavar="<path>",
     ).complete = completion.FILE
+    plots_diff_parser.add_argument(
+        "-e",
+        "--experiment",
+        action="store_true",
+        default=False,
+        help=argparse.SUPPRESS,
+    )
     plots_diff_parser.add_argument(
         "revisions", nargs="*", default=None, help="Git commits to plot from",
     )
diff --git a/dvc/data_cloud.py b/dvc/data_cloud.py
index be132654d..eb9189e21 100644
--- a/dvc/data_cloud.py
+++ b/dvc/data_cloud.py
@@ -81,25 +81,10 @@ class DataCloud:
         """
         remote = self.get_remote(remote, "pull")
 
-        downloaded_items_num = self.repo.cache.local.pull(
+        return self.repo.cache.local.pull(
             cache, jobs=jobs, remote=remote, show_checksums=show_checksums
         )
 
-        if not remote.tree.verify:
-            self._save_pulled_checksums(cache)
-
-        return downloaded_items_num
-
-    def _save_pulled_checksums(self, cache):
-        for checksum in cache.scheme_keys("local"):
-            cache_file = self.repo.cache.local.tree.hash_to_path_info(checksum)
-            if self.repo.cache.local.tree.exists(cache_file):
-                # We can safely save here, as existing corrupted files will
-                # be removed upon status, while files corrupted during
-                # download will not be moved from tmp_file
-                # (see `RemoteBASE.download()`)
-                self.repo.state.save(cache_file, checksum)
-
     def status(
         self,
         cache,
diff --git a/dvc/repo/experiments/__init__.py b/dvc/repo/experiments/__init__.py
index 1d65a743a..ca5663f37 100644
--- a/dvc/repo/experiments/__init__.py
+++ b/dvc/repo/experiments/__init__.py
@@ -76,6 +76,9 @@ class Experiments:
     STASH_EXPERIMENT_RE = re.compile(
         r"(?:On \(.*\): )dvc-exp-(?P<baseline_rev>[0-9a-f]+)$"
     )
+    BRANCH_RE = re.compile(
+        r"^(?P<baseline_rev>[a-f0-9]{7})-(?P<exp_sha>[a-f0-9]+)$"
+    )
 
     def __init__(self, repo):
         from dvc.lock import make_lock
@@ -575,6 +578,26 @@ class Experiments:
         git_repo.reset("HEAD")
         git_repo.stash("drop", "stash@{0}")
 
+    @scm_locked
+    def get_baseline(self, rev):
+        """Return the baseline rev for an experiment rev."""
+        from git.exc import GitCommandError
+
+        rev = self.scm.resolve_rev(rev)
+        try:
+            name = self.scm.repo.git.name_rev(rev, name_only=True)
+        except GitCommandError:
+            return None
+        if not name:
+            return None
+        if name in ("undefined", "stash"):
+            _, baseline = self.stash_revs.get(rev, (None, None))
+            return baseline
+        m = self.BRANCH_RE.match(name)
+        if m:
+            return self.scm.resolve_rev(m.group("baseline_rev"))
+        return None
+
     def checkout(self, *args, **kwargs):
         from dvc.repo.experiments.checkout import checkout
 
diff --git a/dvc/repo/experiments/show.py b/dvc/repo/experiments/show.py
index 36f82d227..55594d951 100644
--- a/dvc/repo/experiments/show.py
+++ b/dvc/repo/experiments/show.py
@@ -1,5 +1,4 @@
 import logging
-import re
 from collections import OrderedDict, defaultdict
 from datetime import datetime
 
@@ -10,9 +9,6 @@ from dvc.repo.params.show import _collect_configs, _read_params
 logger = logging.getLogger(__name__)
 
 
-EXP_RE = re.compile(r"(?P<rev_sha>[a-f0-9]{7})-(?P<exp_sha>[a-f0-9]+)")
-
-
 def _collect_experiment(repo, branch, stash=False):
     res = defaultdict(dict)
     for rev in repo.brancher(revs=[branch]):
@@ -60,9 +56,9 @@ def show(
 
     # collect reproduced experiments
     for exp_branch in repo.experiments.scm.list_branches():
-        m = re.match(EXP_RE, exp_branch)
+        m = repo.experiments.BRANCH_RE.match(exp_branch)
         if m:
-            rev = repo.scm.resolve_rev(m.group("rev_sha"))
+            rev = repo.scm.resolve_rev(m.group("baseline_rev"))
             if rev in revs:
                 exp_rev = repo.experiments.scm.resolve_rev(exp_branch)
                 with repo.experiments.chdir():
diff --git a/dvc/repo/plots/__init__.py b/dvc/repo/plots/__init__.py
index a3222b395..3fac7fb1d 100644
--- a/dvc/repo/plots/__init__.py
+++ b/dvc/repo/plots/__init__.py
@@ -75,7 +75,7 @@ class Plots:
             for datafile, desc in plots.items()
         }
 
-    def show(self, targets=None, revs=None, props=None):
+    def show(self, targets=None, revs=None, props=None, templates=None):
         from .data import NoMetricInHistoryError
 
         data = self.collect(targets, revs)
@@ -90,7 +90,9 @@ class Plots:
         if not data:
             raise NoPlotsError()
 
-        return self.render(data, revs, props, self.repo.plot_templates)
+        if templates is None:
+            templates = self.repo.plot_templates
+        return self.render(data, revs, props, templates)
 
     def diff(self, *args, **kwargs):
         from .diff import diff
diff --git a/dvc/repo/plots/diff.py b/dvc/repo/plots/diff.py
index 4c1fa0ae5..a5daf535f 100644
--- a/dvc/repo/plots/diff.py
+++ b/dvc/repo/plots/diff.py
@@ -1,13 +1,23 @@
-def _revisions(revs, is_dirty):
+def _revisions(repo, revs, experiment):
     revisions = revs or []
+    if experiment and len(revisions) == 1:
+        baseline = repo.experiments.get_baseline(revisions[0])
+        if baseline:
+            revisions.append(baseline[:7])
     if len(revisions) <= 1:
-        if len(revisions) == 0 and is_dirty:
+        if len(revisions) == 0 and repo.scm.is_dirty():
             revisions.append("HEAD")
         revisions.append("workspace")
     return revisions
 
 
-def diff(repo, *args, revs=None, **kwargs):
-    return repo.plots.show(
-        *args, revs=_revisions(revs, repo.scm.is_dirty()), **kwargs
+def diff(repo, *args, revs=None, experiment=False, **kwargs):
+    if experiment:
+        # use experiments repo brancher with templates from the main repo
+        kwargs["templates"] = repo.plot_templates
+        plots_repo = repo.experiments.exp_dvc
+    else:
+        plots_repo = repo
+    return plots_repo.plots.show(
+        *args, revs=_revisions(repo, revs, experiment), **kwargs
     )
diff --git a/dvc/tree/dvc.py b/dvc/tree/dvc.py
index 1ffc931a5..db917d76d 100644
--- a/dvc/tree/dvc.py
+++ b/dvc/tree/dvc.py
@@ -24,6 +24,9 @@ class DvcTree(BaseTree):  # pylint:disable=abstract-method
     as a fallback.
     """
 
+    scheme = "local"
+    PARAM_CHECKSUM = "md5"
+
     def __init__(self, repo, fetch=False, stream=False):
         super().__init__(repo, {"url": repo.root_dir})
         self.fetch = fetch
@@ -138,21 +141,26 @@ class DvcTree(BaseTree):  # pylint:disable=abstract-method
         except OutputNotFoundError:
             return False
 
-    def _add_dir(self, top, trie, out, download_callback=None, **kwargs):
-        if not self.fetch and not self.stream:
-            return
-
+    def _fetch_dir(
+        self, out, filter_info=None, download_callback=None, **kwargs
+    ):
         # pull dir cache if needed
-        dir_cache = out.get_dir_cache(**kwargs)
+        out.get_dir_cache(**kwargs)
 
         # pull dir contents if needed
-        if self.fetch and out.changed_cache(filter_info=top):
-            used_cache = out.get_used_cache(filter_info=top)
+        if self.fetch and out.changed_cache(filter_info=filter_info):
+            used_cache = out.get_used_cache(filter_info=filter_info)
             downloaded = self.repo.cloud.pull(used_cache, **kwargs)
             if download_callback:
                 download_callback(downloaded)
 
-        for entry in dir_cache:
+    def _add_dir(self, top, trie, out, **kwargs):
+        if not self.fetch and not self.stream:
+            return
+
+        self._fetch_dir(out, filter_info=top, **kwargs)
+
+        for entry in out.dir_cache:
             entry_relpath = entry[out.tree.PARAM_RELPATH]
             if os.name == "nt":
                 entry_relpath = entry_relpath.replace("/", os.sep)
@@ -212,6 +220,12 @@ class DvcTree(BaseTree):  # pylint:disable=abstract-method
 
         yield from self._walk(root, trie, topdown=topdown, **kwargs)
 
+    def walk_files(self, path_info, **kwargs):
+        for root, _, files in self.walk(path_info):
+            for fname in files:
+                # NOTE: os.path.join is ~5.5 times slower
+                yield PathInfo(f"{root}{os.sep}{fname}")
+
     def isdvc(self, path, recursive=False, strict=True):
         try:
             meta = self.metadata(path)
@@ -224,6 +238,19 @@ class DvcTree(BaseTree):  # pylint:disable=abstract-method
     def isexec(self, path):  # pylint: disable=unused-argument
         return False
 
+    def get_dir_hash(self, path_info, **kwargs):
+        try:
+            outs = self._find_outs(path_info, strict=True)
+            if len(outs) == 1 and outs[0].is_dir_checksum:
+                out = outs[0]
+                # other code expects us to fetch the dir at this point
+                self._fetch_dir(out, **kwargs)
+                return out.tree.PARAM_CHECKSUM, out.checksum
+        except OutputNotFoundError:
+            pass
+
+        return super().get_dir_hash(path_info, **kwargs)
+
     def get_file_hash(self, path_info):
         outs = self._find_outs(path_info, strict=False)
         if len(outs) != 1:
diff --git a/dvc/tree/repo.py b/dvc/tree/repo.py
index d5c8db357..0d92e7170 100644
--- a/dvc/tree/repo.py
+++ b/dvc/tree/repo.py
@@ -306,6 +306,17 @@ class RepoTree(BaseTree):  # pylint:disable=abstract-method
             for fname in files:
                 yield PathInfo(root) / fname
 
+    def get_dir_hash(self, path_info, **kwargs):
+        if not self.exists(path_info):
+            raise FileNotFoundError
+        _, dvc_tree = self._get_tree_pair(path_info)
+        if dvc_tree and dvc_tree.exists(path_info):
+            try:
+                return dvc_tree.get_dir_hash(path_info, **kwargs)
+            except OutputNotFoundError:
+                pass
+        return super().get_dir_hash(path_info, **kwargs)
+
     def get_file_hash(self, path_info):
         """Return file checksum for specified path.
 


Test patch:
diff --git a/tests/func/experiments/test_experiments.py b/tests/func/experiments/test_experiments.py
index 52330df0a..7ebf71a3f 100644
--- a/tests/func/experiments/test_experiments.py
+++ b/tests/func/experiments/test_experiments.py
@@ -76,3 +76,27 @@ def test_checkout(tmp_dir, scm, dvc):
     dvc.experiments.checkout(exp_b)
     assert (tmp_dir / "params.yaml").read_text().strip() == "foo: 3"
     assert (tmp_dir / "metrics.yaml").read_text().strip() == "foo: 3"
+
+
+def test_get_baseline(tmp_dir, scm, dvc):
+    tmp_dir.gen("copy.py", COPY_SCRIPT)
+    tmp_dir.gen("params.yaml", "foo: 1")
+    stage = dvc.run(
+        cmd="python copy.py params.yaml metrics.yaml",
+        metrics_no_cache=["metrics.yaml"],
+        params=["foo"],
+        name="copy-file",
+    )
+    scm.add(["dvc.yaml", "dvc.lock", "copy.py", "params.yaml", "metrics.yaml"])
+    scm.commit("init")
+    expected = scm.get_rev()
+    assert dvc.experiments.get_baseline(expected) is None
+
+    dvc.reproduce(stage.addressing, experiment=True, params=["foo=2"])
+    rev = dvc.experiments.scm.get_rev()
+    assert dvc.experiments.get_baseline(rev) == expected
+
+    dvc.reproduce(
+        stage.addressing, experiment=True, params=["foo=3"], queue=True
+    )
+    assert dvc.experiments.get_baseline("stash@{0}") == expected
diff --git a/tests/unit/command/test_plots.py b/tests/unit/command/test_plots.py
index 1cc7543ee..219b9d543 100644
--- a/tests/unit/command/test_plots.py
+++ b/tests/unit/command/test_plots.py
@@ -2,7 +2,7 @@ from dvc.cli import parse_args
 from dvc.command.plots import CmdPlotsDiff, CmdPlotsShow
 
 
-def test_metrics_diff(dvc, mocker):
+def test_plots_diff(dvc, mocker):
     cli_args = parse_args(
         [
             "plots",
@@ -24,6 +24,7 @@ def test_metrics_diff(dvc, mocker):
             "x_title",
             "--y-label",
             "y_title",
+            "--experiment",
             "HEAD",
             "tag1",
             "tag2",
@@ -50,10 +51,11 @@ def test_metrics_diff(dvc, mocker):
             "x_label": "x_title",
             "y_label": "y_title",
         },
+        experiment=True,
     )
 
 
-def test_metrics_show(dvc, mocker):
+def test_plots_show(dvc, mocker):
     cli_args = parse_args(
         [
             "plots",
diff --git a/tests/unit/repo/plots/test_diff.py b/tests/unit/repo/plots/test_diff.py
index f3525384d..9a0e80404 100644
--- a/tests/unit/repo/plots/test_diff.py
+++ b/tests/unit/repo/plots/test_diff.py
@@ -13,4 +13,27 @@ from dvc.repo.plots.diff import _revisions
     ],
 )
 def test_revisions(mocker, arg_revisions, is_dirty, expected_revisions):
-    assert _revisions(arg_revisions, is_dirty) == expected_revisions
+    mock_scm = mocker.Mock()
+    mock_scm.configure_mock(**{"is_dirty.return_value": is_dirty})
+    mock_repo = mocker.Mock(scm=mock_scm)
+    assert _revisions(mock_repo, arg_revisions, False) == expected_revisions
+
+
+@pytest.mark.parametrize(
+    "arg_revisions,baseline,expected_revisions",
+    [
+        (["v1"], "v0", ["v1", "v0"]),
+        (["v1"], None, ["v1", "workspace"]),
+        (["v1", "v2"], "v0", ["v1", "v2"]),
+        (["v1", "v2"], None, ["v1", "v2"]),
+    ],
+)
+def test_revisions_experiment(
+    mocker, arg_revisions, baseline, expected_revisions
+):
+    mock_scm = mocker.Mock()
+    mock_scm.configure_mock(**{"is_dirty.return_value": False})
+    mock_experiments = mocker.Mock()
+    mock_experiments.configure_mock(**{"get_baseline.return_value": baseline})
+    mock_repo = mocker.Mock(scm=mock_scm, experiments=mock_experiments)
+    assert _revisions(mock_repo, arg_revisions, True) == expected_revisions
diff --git a/tests/unit/tree/test_dvc.py b/tests/unit/tree/test_dvc.py
index 4c3a428d1..6d32d96c7 100644
--- a/tests/unit/tree/test_dvc.py
+++ b/tests/unit/tree/test_dvc.py
@@ -201,3 +201,41 @@ def test_isdvc(tmp_dir, dvc):
     tree = DvcTree(dvc)
     assert tree.isdvc("foo")
     assert not tree.isdvc("bar")
+
+
+def test_get_hash_file(tmp_dir, dvc):
+    tmp_dir.dvc_gen({"foo": "foo"})
+    tree = DvcTree(dvc)
+    assert tree.get_hash(PathInfo(tmp_dir) / "foo") == (
+        "md5",
+        "acbd18db4cc2f85cedef654fccc4a4d8",
+    )
+
+
+def test_get_hash_dir(tmp_dir, dvc, mocker):
+    tmp_dir.dvc_gen(
+        {"dir": {"foo": "foo", "bar": "bar", "subdir": {"data": "data"}}}
+    )
+    tree = DvcTree(dvc)
+    get_file_hash_spy = mocker.spy(tree, "get_file_hash")
+    assert tree.get_hash(PathInfo(tmp_dir) / "dir") == (
+        "md5",
+        "8761c4e9acad696bee718615e23e22db.dir",
+    )
+    assert not get_file_hash_spy.called
+
+
+def test_get_hash_granular(tmp_dir, dvc):
+    tmp_dir.dvc_gen(
+        {"dir": {"foo": "foo", "bar": "bar", "subdir": {"data": "data"}}}
+    )
+    tree = DvcTree(dvc, fetch=True)
+    subdir = PathInfo(tmp_dir) / "dir" / "subdir"
+    assert tree.get_hash(subdir) == (
+        "md5",
+        "af314506f1622d107e0ed3f14ec1a3b5.dir",
+    )
+    assert tree.get_hash(subdir / "data") == (
+        "md5",
+        "8d777f385d3dfec8815d20f7496026dc",
+    )
diff --git a/tests/unit/tree/test_repo.py b/tests/unit/tree/test_repo.py
index 23d683944..8d1a348aa 100644
--- a/tests/unit/tree/test_repo.py
+++ b/tests/unit/tree/test_repo.py
@@ -357,3 +357,47 @@ def test_repo_tree_no_subrepos(tmp_dir, dvc, scm):
 
     assert tree.exists(tmp_dir / "dir" / "repo.txt") is True
     assert tree.exists(tmp_dir / "repo" / "ipsum") is False
+
+
+def test_get_hash_cached_file(tmp_dir, dvc, mocker):
+    tmp_dir.dvc_gen({"foo": "foo"})
+    tree = RepoTree(dvc)
+    dvc_tree_spy = mocker.spy(tree._dvctrees[dvc.root_dir], "get_file_hash")
+    assert tree.get_hash(PathInfo(tmp_dir) / "foo") == (
+        "md5",
+        "acbd18db4cc2f85cedef654fccc4a4d8",
+    )
+    assert dvc_tree_spy.called
+
+
+def test_get_hash_cached_dir(tmp_dir, dvc, mocker):
+    tmp_dir.dvc_gen(
+        {"dir": {"foo": "foo", "bar": "bar", "subdir": {"data": "data"}}}
+    )
+    tree = RepoTree(dvc)
+    get_file_hash_spy = mocker.spy(tree, "get_file_hash")
+    dvc_tree_spy = mocker.spy(tree._dvctrees[dvc.root_dir], "get_dir_hash")
+    assert tree.get_hash(PathInfo(tmp_dir) / "dir") == (
+        "md5",
+        "8761c4e9acad696bee718615e23e22db.dir",
+    )
+    assert not get_file_hash_spy.called
+    assert dvc_tree_spy.called
+
+
+def test_get_hash_cached_granular(tmp_dir, dvc, mocker):
+    tmp_dir.dvc_gen(
+        {"dir": {"foo": "foo", "bar": "bar", "subdir": {"data": "data"}}}
+    )
+    tree = RepoTree(dvc, fetch=True)
+    dvc_tree_spy = mocker.spy(tree._dvctrees[dvc.root_dir], "get_file_hash")
+    subdir = PathInfo(tmp_dir) / "dir" / "subdir"
+    assert tree.get_hash(subdir) == (
+        "md5",
+        "af314506f1622d107e0ed3f14ec1a3b5.dir",
+    )
+    assert tree.get_hash(subdir / "data") == (
+        "md5",
+        "8d777f385d3dfec8815d20f7496026dc",
+    )
+    assert dvc_tree_spy.called