[PATCH 2 of 2 stable v2] copies: drop _tracefile limit when finding copy sources in actx manifest

[Pierre-Yves David]

On 01/01/2015 01:31 PM, Mads Kiilerich wrote:
> On 12/30/2014 11:32 PM, Pierre-Yves David wrote:
>>
>>
>> On 12/29/2014 02:26 PM, Matt Mackall wrote:
>>> On Thu, 2014-12-11 at 21:27 +0100, Mads Kiilerich wrote:
>>>> # HG changeset patch
>>>> # User Mads Kiilerich <madski at unity3d.com>
>>>> # Date 1418329283 -3600
>>>> #      Thu Dec 11 21:21:23 2014 +0100
>>>> # Branch stable
>>>> # Node ID 0fcb69be20206cfe1582244afd93185f2c056e91
>>>> # Parent  837f7c70551db57aef3a80dac4b50129bb647529
>>>> copies: drop _tracefile limit when finding copy sources in actx
>>>> manifest
>>>
>>> Pierre-Yves tells me he's hot on the trail for a fix for this (but is
>>> also sick so hasn't finished his patches). Going to drop this for now.
>>
>> So looking closer to the test case in the first patch of this series,
>> I discovered that the merge itself was not the source of the issue.
>> And that good old linkrev-shadowing bug was making the rename
>> detection fails.
>
> Correct. The patch with the test do not try to make a (possibly wrong or
> at least debatable) analysis of the problem. It just adds a test case
> for something that we don't have any other test coverage for. The commit
> message do give a hint that linkrev should be blamed for problem.
>
>> Re-using some of the idea Matt brought up in december, I poke at
>> solving this kind of linkrev-induced misbehavior. I think I come with
>> a performance acceptable solution that have been patchbombed on the list.
>
> Yes, fixing linkrev would also make the test case work. A linkrev fix is
> however not suitable for stable. I think the fix I propose here is
> suitable for stable, even if it will be replaced by another fix later.

There is not stable release scheduled until the next feature release. 
And this feature release contains the linkrev fixes.

> Long term, with linkrev fixed, I still think we want another kind of
> limit than the one I propose to remove here. Using a revision number for
> pruning will in some situations be worthless, depending on which
> topological sorting the repo has. If we want the pruning, we can do
> better. (I'm still not entirely convinced that the pruning is worth it,
> considered the complexity and cost of it and how often it will make any
> significant difference.)
>
> Starting a slightly different place: Looking at copies.py from the point
> of view of how "common ancestor heads" are used and considering how it
> will work if we have multiple such ancestors, I think it would be better
> if we kept the "path" to the ancestor instead of repeatedly iterating
> the DAG while searching for the same ancestor.
>
> Similarly, if we want pruning in _tracefile, we should keep the
> "::fctx-::actx" set and make sure we don't iterate the filelog DAG
> outside that set. That could give a pruning that was independent of the
> topological sorting and it would often be more efficient than just using
> a revision number.
>
> So: Also long term, with linkrev fixed, I don't think we want the old
> _tracefile limit and this change is still relevant.


I think you are right when saying the limit could be improved. We can 
probably save some ancestors computation there.

I disagree about dropping the limit. negative result for rename would 
full tree traversals and the resulting complexity would be much worth 
than the current one. But I may get something wrong here.

-- 
Pierre-Yves David