Patchwork D3119: commands: don't violate storage abstractions in `manifest --all`

login
register
mail settings
Submitter phabricator
Date April 5, 2018, 4:39 a.m.
Message ID <differential-rev-PHID-DREV-3sxvrf4tjedl7tzudh54-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/30345/
State Superseded
Headers show

Comments

phabricator - April 5, 2018, 4:39 a.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Previously, we asked the store to emit its data files. For modern
  repos, this would use fncache to resolve the set of files then would
  stat() each file. For my copy of the mozilla-unified repository, this
  took 3.3-10s depending on the state of my filesystem cache to render
  449,790 items.
  
  The previous behavior was a massive layering violation because it
  assumed tracked files would have specific filenames in specific
  directories. Alternate storage backends would violate this assumption.
  
  The new behavior scans the changelog entries for the set of files
  changed by each commit. It aggregates them into a set and then
  sorts and prints the result. This reliably takes ~16.3s on my
  machine. ~80% of the time is spent in zlib decompression.
  
  The performance regression is unfortunate. If we want to claw it
  back, we can create a proper storage API to query for the set of
  tracked files. I'm not opposed to doing that. But I'm in no hurry
  because I suspect ~0 people care about the performance of
  `hg manifest --all`.
  
  .. perf::
  
    `hg manifest --all` is likely slower due to changing its
    implementation to respect storage interface boundaries. If you
    are impacted by this regression in a meaningful way, please make
    noise on the development mailing list and it can be dealt with.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3119

AFFECTED FILES
  mercurial/commands.py
  tests/test-convert-git.t
  tests/test-manifest.t

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/tests/test-manifest.t b/tests/test-manifest.t
--- a/tests/test-manifest.t
+++ b/tests/test-manifest.t
@@ -68,9 +68,9 @@ 
   l
 
   $ hg manifest --all
-  a (no-reposimplestore !)
-  b/a (no-reposimplestore !)
-  l (no-reposimplestore !)
+  a
+  b/a
+  l
 
 The next two calls are expected to abort:
 
diff --git a/tests/test-convert-git.t b/tests/test-convert-git.t
--- a/tests/test-convert-git.t
+++ b/tests/test-convert-git.t
@@ -881,7 +881,7 @@ 
 
   $ hg convert -q git-repo6 no-submodules --config convert.git.skipsubmodules=True
   $ hg -R no-submodules manifest --all
-  .gitmodules-renamed (no-reposimplestore !)
+  .gitmodules-renamed
 
 convert using a different remote prefix
   $ git init git-repo7
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -3491,19 +3491,13 @@ 
         if rev or node:
             raise error.Abort(_("can't specify a revision with --all"))
 
-        res = []
-        # TODO this is a massive layering violation. It assumes the repo is
-        # backed by revlogs with a well-defined naming scheme.
-        prefix = "data/"
-        suffix = ".i"
-        plen = len(prefix)
-        slen = len(suffix)
-        with repo.lock():
-            for fn, b, size in repo.store.datafiles():
-                if size != 0 and fn[-slen:] == suffix and fn[:plen] == prefix:
-                    res.append(fn[plen:-slen])
+        res = set()
+        for rev in repo:
+            ctx = repo[rev]
+            res |= set(ctx.files())
+
         ui.pager('manifest')
-        for f in res:
+        for f in sorted(res):
             fm.startitem()
             fm.write("path", '%s\n', f)
         fm.end()