This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3119

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

Authored by indygreg on Apr 5 2018, 12:39 AM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG7b7ca9ba2de5: commands: don't violate storage abstractions in `manifest --all`
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.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Apr 5 2018, 12:39 AM
durin42 accepted this revision.Apr 6 2018, 8:49 PM
This revision is now accepted and ready to land.Apr 6 2018, 8:49 PM
Closed by commit rHG7b7ca9ba2de5: commands: don't violate storage abstractions in `manifest --all` (authored by indygreg). · Explain WhyApr 6 2018, 9:22 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/commands.py (18 lines)
Mtests/test-convert-git.t (2 lines)
Mtests/test-manifest.t (6 lines)

Diff 7844

mercurial/commands.py

""" """
opts = pycompat.byteskwargs(opts) opts = pycompat.byteskwargs(opts)
fm = ui.formatter('manifest', opts) fm = ui.formatter('manifest', opts)
if opts.get('all'): if opts.get('all'):
if rev or node: if rev or node:
raise error.Abort(_("can't specify a revision with --all")) raise error.Abort(_("can't specify a revision with --all"))
res = [] res = set()
# TODO this is a massive layering violation. It assumes the repo is for rev in repo:
# backed by revlogs with a well-defined naming scheme. ctx = repo[rev]
prefix = "data/" res |= set(ctx.files())
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])
ui.pager('manifest') ui.pager('manifest')
for f in res: for f in sorted(res):
fm.startitem() fm.startitem()
fm.write("path", '%s\n', f) fm.write("path", '%s\n', f)
fm.end() fm.end()
return return
if rev and node: if rev and node:
raise error.Abort(_("please specify just one revision")) raise error.Abort(_("please specify just one revision"))

tests/test-convert-git.t

remove .gitmodules and submodule git-repo5 remove .gitmodules and submodule git-repo5
$ hg -R git-repo6-hg tip -T "{file_dels}\n" $ hg -R git-repo6-hg tip -T "{file_dels}\n"
.hgsub .hgsubstate .hgsub .hgsubstate
skip submodules in the conversion skip submodules in the conversion
$ hg convert -q git-repo6 no-submodules --config convert.git.skipsubmodules=True $ hg convert -q git-repo6 no-submodules --config convert.git.skipsubmodules=True
$ hg -R no-submodules manifest --all $ hg -R no-submodules manifest --all
.gitmodules-renamed (no-reposimplestore !) .gitmodules-renamed
convert using a different remote prefix convert using a different remote prefix
$ git init git-repo7 $ git init git-repo7
Initialized empty Git repository in $TESTTMP/git-repo7/.git/ Initialized empty Git repository in $TESTTMP/git-repo7/.git/
$ cd git-repo7 $ cd git-repo7
TODO: it'd be nice to use (?) lines instead of grep -v to handle the TODO: it'd be nice to use (?) lines instead of grep -v to handle the
git output variance, but that doesn't currently work in the middle of git output variance, but that doesn't currently work in the middle of
a block, so do this for now. a block, so do this for now.

tests/test-manifest.t

l l
$ hg manifest tip $ hg manifest tip
a a
b/a b/a
l l
$ hg manifest --all $ hg manifest --all
a (no-reposimplestore !) a
b/a (no-reposimplestore !) b/a
l (no-reposimplestore !) l
The next two calls are expected to abort: The next two calls are expected to abort:
$ hg manifest -r 2 $ hg manifest -r 2
abort: unknown revision '2'! abort: unknown revision '2'!
[255] [255]
$ hg manifest -r tip tip $ hg manifest -r tip tip
abort: please specify just one revision abort: please specify just one revision
[255] [255]