This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3809

synthrepo: use progress helper
ClosedPublic

Authored by martinvonz on Jun 18 2018, 8:45 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG6540333acb95: synthrepo: use progress helper
Summary

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

martinvonz created this revision.Jun 18 2018, 8:45 PM
pulkit accepted this revision.Jun 19 2018, 7:10 AM
pulkit added a subscriber: pulkit.
pulkit added inline comments.
contrib/synthrepo.py
178

progress.increment()?

Closed by commit rHG6540333acb95: synthrepo: use progress helper (authored by martinvonz). · Explain WhyJun 19 2018, 7:18 AM
This revision was automatically updated to reflect the committed changes.
martinvonz added inline comments.Jun 19 2018, 12:14 PM
contrib/synthrepo.py
178

That would mean that it would now start at 1 instead of 0. I made that change in one or two other patches and then I felt unsure if that's what we want, so I left the current behavior in this and later patches.

I meant to have a discussion about which behavior we want. For example, when the progress bar says 1/3, do you interpret that as processing the first item or that 1 item is already complete?

A related issue is that it seems like either 0% or 100% wouldn't happen (for more than a split second, regardless of how slow each item is). Let's say we have 3 items. We can then do either of these:

progress.update(0)
for i [1,2,3]:
  progress.update(i)
  process(i)
progress.complete()
progress.update(0)
for i [1,2,3]:
  process(i)
  progress.update(i)
progress.complete()

As you can see, either the 0/3 or the 3/3 step will never really be displayed.

pulkit added inline comments.Jul 1 2018, 5:24 PM
contrib/synthrepo.py
178

(sorry I missed this. I have my phabricator emails turned off)

I meant to have a discussion about which behavior we want. For example, when the progress
bar says 1/3, do you interpret that as processing the first item or that 1 item is already
complete?

I think both :(. In network related bars where number of kbs or mbs are involved I think, 1/3 means 1 is already done. Sometimes in other processing, I think okay this one is processing.

However, I think if have verbs before the progress bars saying processing, sending, recieving, in those cases processing 1/3 will mean processing the 1st item.

I still don't feel strong either way but I will like to have the same meaning throughout core.

martinvonz added inline comments.Jul 2 2018, 1:52 AM
contrib/synthrepo.py
178

Another point is that when we pass an item argument (often a filename), then it's helpful to show it before starting processing of the filename, so one can tell which one is slow. Also, if we updated the progress after processing, the last item would only be printed for a spit second (no matter how long time it took to process it). For those reasons, I now lean towards updating progress before processing. For consistency, I think it makes more sense to update progress before processing also when there's no item argument. I agree about doing it after processing when it's about processed bytes, though.

Revision Contents
Changeset List

PathPackages
Mcontrib/synthrepo.py (18 lines)

Diff 9225

contrib/synthrepo.py

dirsadded = zerodict() dirsadded = zerodict()
tzoffset = zerodict() tzoffset = zerodict()
# If a mercurial repo is available, also model the commit history. # If a mercurial repo is available, also model the commit history.
if repo: if repo:
revs = scmutil.revrange(repo, revs) revs = scmutil.revrange(repo, revs)
revs.sort() revs.sort()
progress = ui.progress progress = ui.makeprogress(_('analyzing'), unit=_('changesets'),
_analyzing = _('analyzing') total=len(revs))
_changesets = _('changesets')
_total = len(revs)
for i, rev in enumerate(revs): for i, rev in enumerate(revs):
progress(_analyzing, i, unit=_changesets, total=_total) progress.update(i)
pulkitUnsubmitted
Not Done

progress.increment()?

pulkit: progress.increment()?
martinvonzAuthorUnsubmitted
Not Done

That would mean that it would now start at 1 instead of 0. I made that change in one or two other patches and then I felt unsure if that's what we want, so I left the current behavior in this and later patches.

I meant to have a discussion about which behavior we want. For example, when the progress bar says 1/3, do you interpret that as processing the first item or that 1 item is already complete?

A related issue is that it seems like either 0% or 100% wouldn't happen (for more than a split second, regardless of how slow each item is). Let's say we have 3 items. We can then do either of these:

progress.update(0)
for i [1,2,3]:
  progress.update(i)
  process(i)
progress.complete()
progress.update(0)
for i [1,2,3]:
  process(i)
  progress.update(i)
progress.complete()

As you can see, either the 0/3 or the 3/3 step will never really be displayed.

martinvonz: That would mean that it would now start at 1 instead of 0. I made that change in one or two…
pulkitUnsubmitted
Not Done

(sorry I missed this. I have my phabricator emails turned off)

I meant to have a discussion about which behavior we want. For example, when the progress
bar says 1/3, do you interpret that as processing the first item or that 1 item is already
complete?

I think both :(. In network related bars where number of kbs or mbs are involved I think, 1/3 means 1 is already done. Sometimes in other processing, I think okay this one is processing.

However, I think if have verbs before the progress bars saying processing, sending, recieving, in those cases processing 1/3 will mean processing the 1st item.

I still don't feel strong either way but I will like to have the same meaning throughout core.

pulkit: (sorry I missed this. I have my phabricator emails turned off) > I meant to have a discussion…
martinvonzAuthorUnsubmitted
Not Done

Another point is that when we pass an item argument (often a filename), then it's helpful to show it before starting processing of the filename, so one can tell which one is slow. Also, if we updated the progress after processing, the last item would only be printed for a spit second (no matter how long time it took to process it). For those reasons, I now lean towards updating progress before processing. For consistency, I think it makes more sense to update progress before processing also when there's no item argument. I agree about doing it after processing when it's about processed bytes, though.

martinvonz: Another point is that when we pass an `item` argument (often a filename), then it's helpful to…
ctx = repo[rev] ctx = repo[rev]
pl = ctx.parents() pl = ctx.parents()
pctx = pl[0] pctx = pl[0]
prev = pctx.rev() prev = pctx.rev()
children[prev] += 1 children[prev] += 1
p1distance[rev - prev] += 1 p1distance[rev - prev] += 1
parents[len(pl)] += 1 parents[len(pl)] += 1
tzoffset[ctx.date()[1]] += 1 tzoffset[ctx.date()[1]] += 1
l.append(w) l.append(w)
return ' '.join(l) return ' '.join(l)
wlock = repo.wlock() wlock = repo.wlock()
lock = repo.lock() lock = repo.lock()
nevertouch = {'.hgsub', '.hgignore', '.hgtags'} nevertouch = {'.hgsub', '.hgignore', '.hgtags'}
progress = ui.progress
_synthesizing = _('synthesizing') _synthesizing = _('synthesizing')
_files = _('initial files') _files = _('initial files')
_changesets = _('changesets') _changesets = _('changesets')
# Synthesize a single initial revision adding files to the repo according # Synthesize a single initial revision adding files to the repo according
# to the modeled directory structure. # to the modeled directory structure.
initcount = int(opts['initfiles']) initcount = int(opts['initfiles'])
if initcount and initdirs: if initcount and initdirs:
pctx = repo[None].parents()[0] pctx = repo[None].parents()[0]
dirs = set(pctx.dirs()) dirs = set(pctx.dirs())
files = {} files = {}
def validpath(path): def validpath(path):
# Don't pick filenames which are already directory names. # Don't pick filenames which are already directory names.
if path in dirs: if path in dirs:
return False return False
# Don't pick directories which were used as file names. # Don't pick directories which were used as file names.
while path: while path:
if path in files: if path in files:
return False return False
path = os.path.dirname(path) path = os.path.dirname(path)
return True return True
progress = ui.makeprogress(_synthesizing, unit=_files, total=initcount)
for i in xrange(0, initcount): for i in xrange(0, initcount):
ui.progress(_synthesizing, i, unit=_files, total=initcount) progress.update(i)
path = pickpath() path = pickpath()
while not validpath(path): while not validpath(path):
path = pickpath() path = pickpath()
data = '%s contents\n' % path data = '%s contents\n' % path
files[path] = data files[path] = data
dir = os.path.dirname(path) dir = os.path.dirname(path)
while dir and dir not in dirs: while dir and dir not in dirs:
dirs.add(dir) dirs.add(dir)
dir = os.path.dirname(dir) dir = os.path.dirname(dir)
def filectxfn(repo, memctx, path): def filectxfn(repo, memctx, path):
return context.memfilectx(repo, memctx, path, files[path]) return context.memfilectx(repo, memctx, path, files[path])
ui.progress(_synthesizing, None) progress.complete()
message = 'synthesized wide repo with %d files' % (len(files),) message = 'synthesized wide repo with %d files' % (len(files),)
mc = context.memctx(repo, [pctx.node(), nullid], message, mc = context.memctx(repo, [pctx.node(), nullid], message,
files, filectxfn, ui.username(), files, filectxfn, ui.username(),
'%d %d' % dateutil.makedate()) '%d %d' % dateutil.makedate())
initnode = mc.commit() initnode = mc.commit()
if ui.debugflag: if ui.debugflag:
hexfn = hex hexfn = hex
else: else:
hexfn = short hexfn = short
ui.status(_('added commit %s with %d files\n') ui.status(_('added commit %s with %d files\n')
% (hexfn(initnode), len(files))) % (hexfn(initnode), len(files)))
# Synthesize incremental revisions to the repository, adding repo depth. # Synthesize incremental revisions to the repository, adding repo depth.
count = int(opts['count']) count = int(opts['count'])
heads = set(map(repo.changelog.rev, repo.heads())) heads = set(map(repo.changelog.rev, repo.heads()))
progress = ui.makeprogress(_synthesizing, unit=_changesets, total=count)
for i in xrange(count): for i in xrange(count):
progress(_synthesizing, i, unit=_changesets, total=count) progress.update(i)
node = repo.changelog.node node = repo.changelog.node
revs = len(repo) revs = len(repo)
def pickhead(heads, distance): def pickhead(heads, distance):
if heads: if heads:
lheads = sorted(heads) lheads = sorted(heads)
rev = revs - min(pick(distance), revs) rev = revs - min(pick(distance), revs)