Repository: iterative/dvc

Issue:
Pull with masked targets is slow
`dvc pull` with specifying targets by shell-expanded glob mask (e.g. `dvc pull */*.dvc`) works significantly slower then global `dvc pull`. This is especially hurtful when pulling large number of small files. At the same time, passing glob mask to dvc via `dvc pull '*/*.dvc'` does not seem to be supported.

Should this be optimized?

As mentioned in chat, workaround is `dvc pull -R dir`.

DVC version: 0.50.1
Platform: Ubuntu 18.04, installation via pip

Discussion:

Patch:
diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py
index 7b83789d2..ed78a1875 100644
--- a/dvc/command/data_sync.py
+++ b/dvc/command/data_sync.py
@@ -11,32 +11,17 @@ logger = logging.getLogger(__name__)
 
 
 class CmdDataBase(CmdBase):
-    UP_TO_DATE_MSG = "Everything is up to date."
-
-    def do_run(self, target):
-        pass
-
-    def run(self):
-        if not self.args.targets:
-            return self.do_run()
-
-        ret = 0
-        for target in self.args.targets:
-            if self.do_run(target):
-                ret = 1
-        return ret
-
     @classmethod
     def check_up_to_date(cls, processed_files_count):
         if processed_files_count == 0:
-            logger.info(cls.UP_TO_DATE_MSG)
+            logger.info("Everything is up to date.")
 
 
 class CmdDataPull(CmdDataBase):
-    def do_run(self, target=None):
+    def run(self):
         try:
             processed_files_count = self.repo.pull(
-                target=target,
+                targets=self.args.targets,
                 jobs=self.args.jobs,
                 remote=self.args.remote,
                 show_checksums=self.args.show_checksums,
@@ -54,10 +39,10 @@ class CmdDataPull(CmdDataBase):
 
 
 class CmdDataPush(CmdDataBase):
-    def do_run(self, target=None):
+    def run(self):
         try:
             processed_files_count = self.repo.push(
-                target=target,
+                targets=self.args.targets,
                 jobs=self.args.jobs,
                 remote=self.args.remote,
                 show_checksums=self.args.show_checksums,
@@ -74,10 +59,10 @@ class CmdDataPush(CmdDataBase):
 
 
 class CmdDataFetch(CmdDataBase):
-    def do_run(self, target=None):
+    def run(self):
         try:
             processed_files_count = self.repo.fetch(
-                target=target,
+                targets=self.args.targets,
                 jobs=self.args.jobs,
                 remote=self.args.remote,
                 show_checksums=self.args.show_checksums,
diff --git a/dvc/command/status.py b/dvc/command/status.py
index 1f8caea80..61195b2fc 100644
--- a/dvc/command/status.py
+++ b/dvc/command/status.py
@@ -40,11 +40,11 @@ class CmdDataStatus(CmdDataBase):
                 logger.info("{}{}:".format(ind, key))
                 self._show(value, indent + 1)
 
-    def do_run(self, target=None):
+    def run(self):
         indent = 1 if self.args.cloud else 0
         try:
             st = self.repo.status(
-                target=target,
+                targets=self.args.targets,
                 jobs=self.args.jobs,
                 cloud=self.args.cloud,
                 show_checksums=self.args.show_checksums,
diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py
index 50f8097f4..2efdaa4da 100644
--- a/dvc/repo/__init__.py
+++ b/dvc/repo/__init__.py
@@ -180,7 +180,7 @@ class Repo(object):
 
     def used_cache(
         self,
-        target=None,
+        targets=None,
         all_branches=False,
         active=True,
         with_deps=False,
@@ -215,18 +215,22 @@ class Repo(object):
         for branch in self.brancher(
             all_branches=all_branches, all_tags=all_tags
         ):
-            if target:
-                if recursive and os.path.isdir(target):
-                    stages = self.stages(target)
-                else:
-                    stages = self.collect(target, with_deps=with_deps)
+            if targets:
+                stages = []
+                for target in targets:
+                    if recursive and os.path.isdir(target):
+                        stages.extend(self.stages(target))
+                    else:
+                        stages.extend(
+                            self.collect(target, with_deps=with_deps)
+                        )
             elif active:
                 stages = self.active_stages()
             else:
                 stages = self.stages()
 
             for stage in stages:
-                if active and not target and stage.locked:
+                if active and not targets and stage.locked:
                     logger.warning(
                         "DVC-file '{path}' is locked. Its dependencies are"
                         " not going to be pushed/pulled/fetched.".format(
diff --git a/dvc/repo/fetch.py b/dvc/repo/fetch.py
index 8d7fb0d96..3132e30d3 100644
--- a/dvc/repo/fetch.py
+++ b/dvc/repo/fetch.py
@@ -3,7 +3,7 @@ from __future__ import unicode_literals
 
 def fetch(
     self,
-    target=None,
+    targets=None,
     jobs=None,
     remote=None,
     all_branches=False,
@@ -14,7 +14,7 @@ def fetch(
 ):
     with self.state:
         used = self.used_cache(
-            target,
+            targets,
             all_branches=all_branches,
             all_tags=all_tags,
             with_deps=with_deps,
diff --git a/dvc/repo/gc.py b/dvc/repo/gc.py
index e61a4fc96..89b16e35c 100644
--- a/dvc/repo/gc.py
+++ b/dvc/repo/gc.py
@@ -35,7 +35,7 @@ def _load_all_used_cache(
     for repo in repos:
         with repo.state:
             repo_clist = repo.used_cache(
-                target=None,
+                targets=None,
                 all_branches=all_branches,
                 active=False,
                 with_deps=with_deps,
diff --git a/dvc/repo/pull.py b/dvc/repo/pull.py
index 380bc86a3..096ccedfb 100644
--- a/dvc/repo/pull.py
+++ b/dvc/repo/pull.py
@@ -3,7 +3,7 @@ from __future__ import unicode_literals
 
 def pull(
     self,
-    target=None,
+    targets=None,
     jobs=None,
     remote=None,
     all_branches=False,
@@ -14,7 +14,7 @@ def pull(
     recursive=False,
 ):
     processed_files_count = self.fetch(
-        target,
+        targets,
         jobs,
         remote=remote,
         all_branches=all_branches,
@@ -23,7 +23,11 @@ def pull(
         with_deps=with_deps,
         recursive=recursive,
     )
-    self.checkout(
-        target=target, with_deps=with_deps, force=force, recursive=recursive
-    )
+    for target in targets or [None]:
+        self.checkout(
+            target=target,
+            with_deps=with_deps,
+            force=force,
+            recursive=recursive,
+        )
     return processed_files_count
diff --git a/dvc/repo/push.py b/dvc/repo/push.py
index 8787a07c9..8110ac633 100644
--- a/dvc/repo/push.py
+++ b/dvc/repo/push.py
@@ -3,7 +3,7 @@ from __future__ import unicode_literals
 
 def push(
     self,
-    target=None,
+    targets=None,
     jobs=None,
     remote=None,
     all_branches=False,
@@ -14,7 +14,7 @@ def push(
 ):
     with self.state:
         used = self.used_cache(
-            target,
+            targets,
             all_branches=all_branches,
             all_tags=all_tags,
             with_deps=with_deps,
diff --git a/dvc/repo/status.py b/dvc/repo/status.py
index 7b880e9e6..16204146c 100644
--- a/dvc/repo/status.py
+++ b/dvc/repo/status.py
@@ -1,15 +1,19 @@
 from __future__ import unicode_literals
 
 import logging
+from funcy.py3 import cat
 
 
 logger = logging.getLogger(__name__)
 
 
-def _local_status(self, target=None, with_deps=False):
+def _local_status(self, targets=None, with_deps=False):
     status = {}
 
-    stages = self.collect(target, with_deps=with_deps)
+    if targets:
+        stages = cat(self.collect(t, with_deps=with_deps) for t in targets)
+    else:
+        stages = self.collect(None, with_deps=with_deps)
 
     for stage in stages:
         if stage.locked:
@@ -27,7 +31,7 @@ def _local_status(self, target=None, with_deps=False):
 
 def _cloud_status(
     self,
-    target=None,
+    targets=None,
     jobs=None,
     remote=None,
     show_checksums=False,
@@ -64,7 +68,7 @@ def _cloud_status(
     import dvc.remote.base as cloud
 
     used = self.used_cache(
-        target,
+        targets,
         all_branches=all_branches,
         all_tags=all_tags,
         with_deps=with_deps,
@@ -92,7 +96,7 @@ def _cloud_status(
 
 def status(
     self,
-    target=None,
+    targets=None,
     jobs=None,
     cloud=False,
     remote=None,
@@ -105,7 +109,7 @@ def status(
         if cloud or remote:
             return _cloud_status(
                 self,
-                target,
+                targets,
                 jobs,
                 remote=remote,
                 show_checksums=show_checksums,
@@ -113,4 +117,4 @@ def status(
                 with_deps=with_deps,
                 all_tags=all_tags,
             )
-        return _local_status(self, target, with_deps=with_deps)
+        return _local_status(self, targets, with_deps=with_deps)


Test patch:
diff --git a/tests/func/test_commit.py b/tests/func/test_commit.py
index 4aa7f0ba7..2ec74a185 100644
--- a/tests/func/test_commit.py
+++ b/tests/func/test_commit.py
@@ -37,7 +37,7 @@ def test_commit_force(dvc_repo, repo_dir):
 
     dvc_repo.commit(stage.path, force=True)
 
-    assert dvc_repo.status(stage.path) == {}
+    assert dvc_repo.status([stage.path]) == {}
 
 
 def test_commit_with_deps(dvc_repo, repo_dir):
diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py
index a827e6441..f1a006ede 100644
--- a/tests/func/test_repro.py
+++ b/tests/func/test_repro.py
@@ -898,13 +898,13 @@ class TestReproExternalBase(TestDvc):
 
         self.assertTrue(os.path.exists("import"))
         self.assertTrue(filecmp.cmp("import", self.FOO, shallow=False))
-        self.assertEqual(self.dvc.status(import_stage.path), {})
+        self.assertEqual(self.dvc.status([import_stage.path]), {})
         self.check_already_cached(import_stage)
 
         import_remote_stage = self.dvc.imp_url(
             out_foo_path, out_foo_path + "_imported"
         )
-        self.assertEqual(self.dvc.status(import_remote_stage.path), {})
+        self.assertEqual(self.dvc.status([import_remote_stage.path]), {})
 
         cmd_stage = self.dvc.run(
             outs=[out_bar_path],
@@ -912,7 +912,7 @@ class TestReproExternalBase(TestDvc):
             cmd=self.cmd(foo_path, bar_path),
         )
 
-        self.assertEqual(self.dvc.status(cmd_stage.path), {})
+        self.assertEqual(self.dvc.status([cmd_stage.path]), {})
         self.assertEqual(self.dvc.status(), {})
         self.check_already_cached(cmd_stage)
 
@@ -923,24 +923,24 @@ class TestReproExternalBase(TestDvc):
         self.dvc.update(import_stage.path)
         self.assertTrue(os.path.exists("import"))
         self.assertTrue(filecmp.cmp("import", self.BAR, shallow=False))
-        self.assertEqual(self.dvc.status(import_stage.path), {})
+        self.assertEqual(self.dvc.status([import_stage.path]), {})
 
         self.dvc.update(import_remote_stage.path)
-        self.assertEqual(self.dvc.status(import_remote_stage.path), {})
+        self.assertEqual(self.dvc.status([import_remote_stage.path]), {})
 
         stages = self.dvc.reproduce(cmd_stage.path)
         self.assertEqual(len(stages), 1)
-        self.assertEqual(self.dvc.status(cmd_stage.path), {})
+        self.assertEqual(self.dvc.status([cmd_stage.path]), {})
 
         self.assertEqual(self.dvc.status(), {})
         self.dvc.gc()
         self.assertEqual(self.dvc.status(), {})
 
         self.dvc.remove(cmd_stage.path, outs_only=True)
-        self.assertNotEqual(self.dvc.status(cmd_stage.path), {})
+        self.assertNotEqual(self.dvc.status([cmd_stage.path]), {})
 
         self.dvc.checkout(cmd_stage.path, force=True)
-        self.assertEqual(self.dvc.status(cmd_stage.path), {})
+        self.assertEqual(self.dvc.status([cmd_stage.path]), {})
 
 
 @pytest.mark.skipif(os.name == "nt", reason="temporarily disabled on windows")
@@ -1207,7 +1207,7 @@ class TestReproExternalHTTP(TestReproExternalBase):
             shutil.move(self.local_cache, cache_id)
             self.assertFalse(os.path.exists(self.local_cache))
 
-            self.dvc.pull(import_stage.path, remote="mycache")
+            self.dvc.pull([import_stage.path], remote="mycache")
 
             self.assertTrue(os.path.exists(import_output))
 
diff --git a/tests/func/test_update.py b/tests/func/test_update.py
index 7345627aa..c20eb4374 100644
--- a/tests/func/test_update.py
+++ b/tests/func/test_update.py
@@ -34,9 +34,9 @@ def test_update_import(dvc_repo, erepo):
 
     os.chdir(saved_dir)
 
-    assert dvc_repo.status(stage.path) == {}
+    assert dvc_repo.status([stage.path]) == {}
     dvc_repo.update(stage.path)
-    assert dvc_repo.status(stage.path) == {}
+    assert dvc_repo.status([stage.path]) == {}
 
     assert os.path.exists(dst)
     assert os.path.isfile(dst)
@@ -60,9 +60,9 @@ def test_update_import_url(repo_dir, dvc_repo):
     os.unlink(src)
     shutil.copyfile(repo_dir.BAR, src)
 
-    assert dvc_repo.status(stage.path) == {}
+    assert dvc_repo.status([stage.path]) == {}
     dvc_repo.update(stage.path)
-    assert dvc_repo.status(stage.path) == {}
+    assert dvc_repo.status([stage.path]) == {}
 
     assert os.path.exists(dst)
     assert os.path.isfile(dst)