[PATCH 3 of 5 V3] copies: refactor checkcopies() into a top level method

Kevin Bullock kbullock+mercurial at ringworld.org
Thu May 9 16:36:28 CDT 2013


On 9 May 2013, at 3:32 PM, Durham Goode wrote:

> On 5/9/13 12:44 PM, "Kevin Bullock" <kbullock+mercurial at ringworld.org>
> wrote:
> 
>> On 8 May 2013, at 6:24 PM, Durham Goode wrote:
>> 
>>> +def checkcopies(ctx, f, m1, m2, ca, limit, diverge, copy, fullcopy):
>>> +    """
>>> +    check possible copies of f from m1 to m2
>>> +
>>> +    ctx = function accepting (filename, node) that returns a filectx.
>>> +    f = the filename to check
>>> +    m1 = the source manifest
>>> +    m2 = the destination manifest
>>> +    ca = the changectx of the common ancestor
>>> +    limit = the rev number to not search beyond
>>> +    diverge = record all diverges in this dict
>>> +    copy = record all non-divergent copies in this dict
>>> +    fullcopy = record all copies in this dict
>>> +    """
>> 
>> Since the last 3 of these are essentially out params, can we just return
>> them in a tuple?
> 
> The function is called once per file that is different between the m1 and
> m2, which could be quite a lot of calls. If I construct a new
> diverge/copy/fullcopy each time it would be a lot of allocations, and I'd
> have to merge them into the master diverge/copy/fullcopy instances after
> each call.  The diverge one in particular is a pain because it's a
> dictionary with list values, so I couldn't just call dict.update()
> 
> I figured it's better to avoid the overhead in this case.

Hmm, complex state that has to be kept across calls? At the risk of invoking Matt's wrath, that sounds like we might need a new class.

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock



More information about the Mercurial-devel mailing list