This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8882

mergeresult: yield from getactions() instead of buidling a list and returning
ClosedPublic

Authored by pulkit on Aug 5 2020, 4:53 AM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHGcdc50e1929b0: mergeresult: yield from getactions() instead of buidling a list and returning
Summary

Only 7 out of 29 callers change the underlying dict while iterating. So it's
better to yield and wrap the 7 callers with list().

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

pulkit created this revision.Aug 5 2020, 4:53 AM
pulkit updated this revision to Diff 22303.Aug 6 2020, 7:14 AM
indygreg accepted this revision.Aug 8 2020, 12:25 PM
indygreg added a subscriber: indygreg.

Yeah, I suppose mutation during iteration will make a lot of noise. So as long as we have test coverage of this code, we should be fine.

This revision is now accepted and ready to land.Aug 8 2020, 12:25 PM

Revision Contents
Changeset List

PathPackages
Mmercurial/merge.py (40 lines)

Diff 22340

mercurial/merge.py

fileconflicts.add(f) fileconflicts.add(f)
allconflicts = fileconflicts | pathconflicts allconflicts = fileconflicts | pathconflicts
ignoredconflicts = {c for c in allconflicts if repo.dirstate._ignore(c)} ignoredconflicts = {c for c in allconflicts if repo.dirstate._ignore(c)}
unknownconflicts = allconflicts - ignoredconflicts unknownconflicts = allconflicts - ignoredconflicts
collectconflicts(ignoredconflicts, ignoredconfig) collectconflicts(ignoredconflicts, ignoredconfig)
collectconflicts(unknownconflicts, unknownconfig) collectconflicts(unknownconflicts, unknownconfig)
else: else:
for f, args, msg in mresult.getactions( for f, args, msg in list(
[mergestatemod.ACTION_CREATED_MERGE] mresult.getactions([mergestatemod.ACTION_CREATED_MERGE])
): ):
fl2, anc = args fl2, anc = args
different = _checkunknownfile(repo, wctx, mctx, f) different = _checkunknownfile(repo, wctx, mctx, f)
if repo.dirstate._ignore(f): if repo.dirstate._ignore(f):
config = ignoredconfig config = ignoredconfig
else: else:
config = unknownconfig config = unknownconfig
) )
for f in sorted(warnconflicts): for f in sorted(warnconflicts):
if repo.wvfs.isfileorlink(f): if repo.wvfs.isfileorlink(f):
repo.ui.warn(_(b"%s: replacing untracked file\n") % f) repo.ui.warn(_(b"%s: replacing untracked file\n") % f)
else: else:
repo.ui.warn(_(b"%s: replacing untracked files in directory\n") % f) repo.ui.warn(_(b"%s: replacing untracked files in directory\n") % f)
for f, args, msg in mresult.getactions([mergestatemod.ACTION_CREATED]): for f, args, msg in list(
mresult.getactions([mergestatemod.ACTION_CREATED])
):
backup = ( backup = (
f in fileconflicts f in fileconflicts
or f in pathconflicts or f in pathconflicts
or any(p in pathconflicts for p in pathutil.finddirs(f)) or any(p in pathconflicts for p in pathutil.finddirs(f))
) )
(flags,) = args (flags,) = args
mresult.addfile(f, mergestatemod.ACTION_GET, (flags, backup), msg) mresult.addfile(f, mergestatemod.ACTION_GET, (flags, backup), msg)
del self._actionmapping[action][filename] del self._actionmapping[action][filename]
def getactions(self, actions, sort=False): def getactions(self, actions, sort=False):
""" get list of files which are marked with these actions """ get list of files which are marked with these actions
if sort is true, files for each action is sorted and then added if sort is true, files for each action is sorted and then added
Returns a list of tuple of form (filename, data, message) Returns a list of tuple of form (filename, data, message)
""" """
res = []
for a in actions: for a in actions:
if sort: if sort:
for f in sorted(self._actionmapping[a]): for f in sorted(self._actionmapping[a]):
args, msg = self._actionmapping[a][f] args, msg = self._actionmapping[a][f]
res.append((f, args, msg)) yield f, args, msg
else: else:
for f, (args, msg) in pycompat.iteritems( for f, (args, msg) in pycompat.iteritems(
self._actionmapping[a] self._actionmapping[a]
): ):
res.append((f, args, msg)) yield f, args, msg
return res
def len(self, actions=None): def len(self, actions=None):
""" returns number of files which needs actions """ returns number of files which needs actions
if actions is passed, total of number of files in that action if actions is passed, total of number of files in that action
only is returned """ only is returned """
if actions is None: if actions is None:
return mresult return mresult
def _resolvetrivial(repo, wctx, mctx, ancestor, mresult): def _resolvetrivial(repo, wctx, mctx, ancestor, mresult):
"""Resolves false conflicts where the nodeid changed but the content """Resolves false conflicts where the nodeid changed but the content
remained the same.""" remained the same."""
# We force a copy of actions.items() because we're going to mutate # We force a copy of actions.items() because we're going to mutate
# actions as we resolve trivial conflicts. # actions as we resolve trivial conflicts.
for f, args, msg in mresult.getactions( for f, args, msg in list(
[mergestatemod.ACTION_CHANGED_DELETED] mresult.getactions([mergestatemod.ACTION_CHANGED_DELETED])
): ):
if f in ancestor and not wctx[f].cmp(ancestor[f]): if f in ancestor and not wctx[f].cmp(ancestor[f]):
# local did change but ended up with same content # local did change but ended up with same content
mresult.addfile( mresult.addfile(
f, mergestatemod.ACTION_REMOVE, None, b'prompt same' f, mergestatemod.ACTION_REMOVE, None, b'prompt same'
) )
for f, args, msg in list( for f, args, msg in list(
# the other side of filenode was choosen while merging, store this in # the other side of filenode was choosen while merging, store this in
# mergestate so that it can be reused on commit # mergestate so that it can be reused on commit
if op == b'other': if op == b'other':
ms.addmergedother(f) ms.addmergedother(f)
moves = [] moves = []
# 'cd' and 'dc' actions are treated like other merge conflicts # 'cd' and 'dc' actions are treated like other merge conflicts
mergeactions = mresult.getactions( mergeactions = list(
mresult.getactions(
[ [
mergestatemod.ACTION_CHANGED_DELETED, mergestatemod.ACTION_CHANGED_DELETED,
mergestatemod.ACTION_DELETED_CHANGED, mergestatemod.ACTION_DELETED_CHANGED,
mergestatemod.ACTION_MERGE, mergestatemod.ACTION_MERGE,
], ],
sort=True, sort=True,
) )
)
for f, args, msg in mergeactions: for f, args, msg in mergeactions:
f1, f2, fa, move, anc = args f1, f2, fa, move, anc = args
if f == b'.hgsubstate': # merged internally if f == b'.hgsubstate': # merged internally
continue continue
if f1 is None: if f1 is None:
fcl = filemerge.absentfilectx(wctx, fa) fcl = filemerge.absentfilectx(wctx, fa)
else: else:
repo.ui.debug(b" preserving %s for resolve of %s\n" % (f1, f)) repo.ui.debug(b" preserving %s for resolve of %s\n" % (f1, f))
cost = 0 if wctx.isinmemory() else 0.001 cost = 0 if wctx.isinmemory() else 0.001
# remove in parallel (must come before resolving path conflicts and getting) # remove in parallel (must come before resolving path conflicts and getting)
prog = worker.worker( prog = worker.worker(
repo.ui, repo.ui,
cost, cost,
batchremove, batchremove,
(repo, wctx), (repo, wctx),
mresult.getactions([mergestatemod.ACTION_REMOVE], sort=True), list(mresult.getactions([mergestatemod.ACTION_REMOVE], sort=True)),
) )
for i, item in prog: for i, item in prog:
progress.increment(step=i, item=item) progress.increment(step=i, item=item)
removed = mresult.len((mergestatemod.ACTION_REMOVE,)) removed = mresult.len((mergestatemod.ACTION_REMOVE,))
# resolve path conflicts (must come before getting) # resolve path conflicts (must come before getting)
for f, args, msg in mresult.getactions( for f, args, msg in mresult.getactions(
[mergestatemod.ACTION_PATH_CONFLICT_RESOLVE], sort=True [mergestatemod.ACTION_PATH_CONFLICT_RESOLVE], sort=True
threadsafe = repo.ui.configbool( threadsafe = repo.ui.configbool(
b'experimental', b'worker.wdir-get-thread-safe' b'experimental', b'worker.wdir-get-thread-safe'
) )
prog = worker.worker( prog = worker.worker(
repo.ui, repo.ui,
cost, cost,
batchget, batchget,
(repo, mctx, wctx, wantfiledata), (repo, mctx, wctx, wantfiledata),
mresult.getactions([mergestatemod.ACTION_GET], sort=True), list(mresult.getactions([mergestatemod.ACTION_GET], sort=True)),
threadsafe=threadsafe, threadsafe=threadsafe,
hasretval=True, hasretval=True,
) )
getfiledata = {} getfiledata = {}
for final, res in prog: for final, res in prog:
if final: if final:
getfiledata = res getfiledata = res
else: else:
# that when the action for a file is changed it is automatically # that when the action for a file is changed it is automatically
# reflected in the other action lists. But that involves a more # reflected in the other action lists. But that involves a more
# complex data structure, so this will do for now. # complex data structure, so this will do for now.
# #
# We don't need to do the same operation for 'dc' and 'cd' because # We don't need to do the same operation for 'dc' and 'cd' because
# those lists aren't consulted again. # those lists aren't consulted again.
mfiles.difference_update(a[0] for a in acts) mfiles.difference_update(a[0] for a in acts)
for a in mresult.getactions([mergestatemod.ACTION_MERGE]): for a in list(mresult.getactions((mergestatemod.ACTION_MERGE,))):
if a[0] not in mfiles: if a[0] not in mfiles:
mresult.removefile(a[0]) mresult.removefile(a[0])
progress.complete() progress.complete()
assert len(getfiledata) == ( assert len(getfiledata) == (
mresult.len((mergestatemod.ACTION_GET,)) if wantfiledata else 0 mresult.len((mergestatemod.ACTION_GET,)) if wantfiledata else 0
) )
return updateresult(updated, merged, removed, unresolved), getfiledata return updateresult(updated, merged, removed, unresolved), getfiledata