[PATCH 2 of 5] adjustlinkrev: use C implementation to test ancestor if possible
Jun Wu
quark at fb.com
Tue Nov 1 09:33:10 UTC 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