[PATCH 2 of 5] adjustlinkrev: use C implementation to test ancestor if possible

Jun Wu quark at fb.com
Tue Nov 1 05:33:10 EDT 2016


Not related to this series directly but I want to remove "_ancestrycontext".

"_ancestrycontext" seems to be redundant because we have "_descendantrev".
And the C implementation is fast enough to just use "_descendantrev". The
most important thing seems to be getting a good enough "_descendantrev",
like 2896f53509a7. Otherwise the slow changelog.d walk will be terrible.

I did some archeology, looked at issue 4514, and it seems if I just remove
"_ancestrycontext", the reproduce command that Mathias De Maré mentioned 
"hg diff -r 0:de519517f597 -g" is still fast. But I'm not sure if that
command is enough or not to test the issue.

Another related issue is 4537, where I have some questions to the fix
9372180b8c9b:

  https://www.mercurial-scm.org/repo/hg/rev/9372180b8c9b#l1.36
  + ctx._ancestrycontext = ac

    It seems "_ancestrycontext" is a fctx-only thing, setting it on ctx
    object looks unnecessary.

  https://www.mercurial-scm.org/repo/hg/rev/9372180b8c9b#l1.44
  + fctx._ancestrycontext = ac
  + fctx._descendantrev = rev

    _ancestrycontext and _descendantrev conflicts. We only use one of them.
    So it seems _ancestrycontext can be dropped here, because we have set
    _descendantrev.

I don't fully understand what copies.py does and how to test it. However, if
I understand correctly, we can drop "_ancestrycontext" anyway.

Any suggestion on how to test it programmingly is welcome. Should I just
build a repo full of renames and run log -f on it?

Excerpts from Jun Wu's message of 2016-11-01 08:51:03 +0000:
> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1477989396 0
> #      Tue Nov 01 08:36:36 2016 +0000
> # Node ID 93e073edc7f673d76d1113f5830ec46210707c25
> # Parent  ac063914b3a3c01f1d7ed253c73113903fccb7a9
> # Available At https://bitbucket.org/quark-zju/hg-draft 
> #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 93e073edc7f6
> adjustlinkrev: use C implementation to test ancestor if possible
> 
> The C ancestors implementation is about 10-100x faster than the Python
> lazyancestors implementation (and it has room to improve: 1. we only need
> isancestor, not common ancestor; 2. we can shrink revs[p:q] to a single
> ranged revision if all(revs[i].parentrevs() == [i-1] for i in range(p,q)),
> the state of known ranged revisions can be stored in the C index object).
> 
> In the real world, the following command:
> 
>   HGRCPATH=/dev/null hg log -f mercurial/c*.py -T '{rev}\n' > /dev/null
> 
> Takes 2.3 to 2.5 seconds user-space CPU time before the patch,
>   and 1.7 to 1.9 after.
> 
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -838,9 +838,14 @@ class basefilectx(object):
>          else:
>              revs = [srcrev]
> +        # check if this linkrev is an ancestor of srcrev
>          if memberanc is None:
> -            memberanc = iteranc = cl.ancestors(revs, lkr,
> -                                               inclusive=inclusive)
> -        # check if this linkrev is an ancestor of srcrev
> -        if lkr not in memberanc:
> +            # cl.isancestor is backed by C code, faster than cl.ancestors
> +            if any(cl.isancestor(lkr, r) for r in revs):
> +                return lkr
> +        else:
> +            if lkr in memberanc:
> +                return lkr
> +        # fallback to walk through the changelog
> +        if True:
>              if iteranc is None:
>                  iteranc = cl.ancestors(revs, lkr, inclusive=inclusive)


More information about the Mercurial-devel mailing list