[PATCH 6 of 8] merge: use separate lists for each action type
Mads Kiilerich
mads at kiilerich.com
Thu May 8 08:20:22 CDT 2014
Heh - this is the patch I have been referring to as the 8th patch ;-)
On 05/07/2014 11:30 PM, Pierre-Yves David wrote:
> On 05/01/2014 04:42 PM, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski at unity3d.com>
>> # Date 1393550758 -3600
>> # Fri Feb 28 02:25:58 2014 +0100
>> # Branch stable
>> # Node ID 68cb67727ca4447bad6349b6f7d75d799d35f6ed
>> # Parent 9d786e6be4319bf31467eb715fb432c68ccd4507
>> merge: use separate lists for each action type
>>
>> This replaces the grand unified action list that had multiple action
>> types as
>> tuples in one big list. That list was iterated multiple times just to
>> find
>> actions of a specific type. This data model also made some code more
>> convoluted than necessary.
>>
>> Instead we now store actions as a tuple of lists. Using multiple
>> lists gives a
>> bit of cut'n'pasted code but also enables other optimizations.
>
> I like this change very much.
>
>> This patch uses 'if True:' to preserve indentations and help
>> reviewing. It also
>> limits the number of conflicts with other pending patches. It can
>> trivially be
>> cleaned up later.
>>
>> The changes to test output are due to changes in the ordering of
>> debug output.
>> That is mainly because we now do the debug logging for files when we
>> actually
>> process them. Files are also processed in a slightly different but still
>> correct order. It is now primarily ordered by action type,
>> secondarily by
>> filename.
>
> I though this reordering already happened in patch 2 of this series?
Correct. The patch description should have been updated after I split
the patch up in the preceding minor parts. This patch do not have any
test changes.
>
>> diff --git a/mercurial/merge.py b/mercurial/merge.py
>> --- a/mercurial/merge.py
>> +++ b/mercurial/merge.py
>> @@ -310,63 +310,44 @@ def _forgetremoved(wctx, mctx, branchmer
>> as removed.
>> """
>>
>> - actions = []
>> - state = branchmerge and 'r' or 'f'
>> + ractions = []
>> + factions = xactions = []
>> + if branchmerge:
>> + xactions = ractions
>> for f in wctx.deleted():
>> if f not in mctx:
>> - actions.append((f, state, None, "forget deleted"))
>> + xactions.append((f, None, "forget deleted"))
>>
>> if not branchmerge:
>> for f in wctx.removed():
>> if f not in mctx:
>> - actions.append((f, "f", None, "forget removed"))
>> + factions.append((f, None, "forget removed"))
>>
>> - return actions
>> + return ractions, factions
>>
>> def _checkcollision(repo, wmf, actions):
>> # build provisional merged manifest up
>> pmmf = set(wmf)
>>
>> - def addop(f, args):
>> - pmmf.add(f)
>> - def removeop(f, args):
>> + # k, dr, e and rd are nop
>> + for m in 'a', 'f', 'g', 'cd', 'dc':
>> + for f, args, msg in actions[m]:
>> + pmmf.add(f)
>> + for f, args, msg in actions['r']:
>> pmmf.discard(f)
>> - def nop(f, args):
>> - pass
>> -
>> - def renamemoveop(f, args):
>> + for f, args, msg in actions['dm']:
>> f2, flags = args
>> pmmf.discard(f2)
>> pmmf.add(f)
>> - def renamegetop(f, args):
>> + for f, args, msg in actions['dg']:
>> f2, flags = args
>> pmmf.add(f)
>> - def mergeop(f, args):
>> + for f, args, msg in actions['m']:
>> f1, f2, fa, move, anc = args
>> if move:
>> pmmf.discard(f1)
>> pmmf.add(f)
>>
>> - opmap = {
>> - "a": addop,
>> - "dm": renamemoveop,
>> - "dg": renamegetop,
>> - "dr": nop,
>> - "e": nop,
>> - "k": nop,
>> - "f": addop, # untracked file should be kept in working
>> directory
>> - "g": addop,
>> - "m": mergeop,
>> - "r": removeop,
>> - "rd": nop,
>> - "cd": addop,
>> - "dc": addop,
>> - }
>> - for f, m, args, msg in actions:
>> - op = opmap.get(m)
>> - assert op, m
>> - op(f, args)
>> -
>> # check case-folding collision in provisional merged manifest
>> foldmap = {}
>> for f in sorted(pmmf):
>> @@ -386,7 +367,9 @@ def manifestmerge(repo, wctx, p2, pa, br
>> acceptremote = accept the incoming changes without prompting
>> """
>>
>> - actions, copy, movewithdir = [], {}, {}
>> + actions = dict((m, []) for m in ['a', 'f', 'g', 'cd', 'dc', 'r',
>> + 'dm', 'dg', 'm', 'dr', 'e',
>> 'rd', 'k'])
>
> This line is twice the standard size, you can probably switch to a
> version for loop.
I don't understand that comment. Please clarify.
>
>> @@ -803,126 +775,120 @@ def calculateupdates(repo, wctx, mctx, a
>> actions = manifestmerge(repo, wctx, mctx, ancestor,
>> branchmerge, force,
>> partial, acceptremote,
>> followcopies)
>> - for a in sorted(actions, key=lambda a: (a[1], a)):
>> - f, m, args, msg = a
>> - repo.ui.debug(' %s: %s -> %s\n' % (f, msg, m))
>> - if f in fbids:
>> - fbids[f].append(a)
>> - else:
>> - fbids[f] = [a]
>> + for m, l in sorted(actions.items()):
>> + for a in l:
>> + f, args, msg = a
>> + repo.ui.debug(' %s: %s -> %s\n' % (f, msg, m))
>> + if f in fbids:
>> + d = fbids[f]
>> + if m in d:
>> + d[m].append(a)
>> + else:
>> + d[m] = [a]
>> + else:
>> + fbids[f] = {m: [a]}
>>
>> # Pick the best bid for each file
>> repo.ui.note(_('\nauction for merging merge bids\n'))
>> - actions = []
>> + actions = dict((m, []) for m in actions.keys())
>> for f, bidsl in sorted(fbids.items()):
>> # Consensus?
>> - a0 = bidsl[0]
>> - if util.all(a == a0 for a in bidsl[1:]): # len(bidsl) is
>> > 1
>> - repo.ui.note(" %s: consensus for %s\n" % (f, a0[1]))
>> - actions.append(a0)
>> - continue
>> - # Group bids by kind of action
>> - bids = {}
>> - for a in bidsl:
>> - m = a[1]
>> - if m in bids:
>> - bids[m].append(a)
>> - else:
>> - bids[m] = [a]
>> + if len(bidsl) == 1: # one kind of method
>> + m, l = bidsl.items()[0]
>> + if util.all(a == l[0] for a in l[1:]): # len(bidsl)
>> is > 1
>> + repo.ui.note(" %s: consensus for %s\n" % (f, m))
>> + actions[m].append(l[0])
>> + continue
>
> I do not understand the "# one kind of method"
That is if there is bids for one kind of method, for instance if all
bids are for merge.
/Mads
More information about the Mercurial-devel
mailing list