[PATCH 1 of 2] log: slowpath: do not read the full changelog

[Mads Kiilerich]

  Nicolas Dumazet wrote, On 07/04/2010 04:20 PM:
> # HG changeset patch
> # User Nicolas Dumazet<nicdumz.commits at gmail.com>
> # Date 1278234450 -32400
> # Branch stable
> # Node ID c6b0a3edb48a91859df9949ebeb11802becfe9f8
> # Parent  239f3210c970615dc1d5f861a92b61b4662a71a5
> log: slowpath: do not read the full changelog
>
> When in the slowpath, we are examining _all_ changesets in revs.
> We need to do this backwards, to optimize changelog reads.
>
> Increasing windows were used to read changelog backwards in a windowed manner,
> reading the changelog forward inside each window.

I'm confused. (Not only by the "slowpath" terminology...) The patch 
seems fine to me, but the description seems to contradict both the 
understanding I had and what the patch does.

AFAIK reads has to be done _forward_ (from lover revision numbers to 
bigger) to optimize changelog reads.

(The "fastpath" must traverse the graph backwards in order to find the 
chain of parents and track renames. It uses increasing windows to go 
backwards while taking forward steps (moonwalking?) and thus get fast 
reads without too much waste. We are however not discussing fastpath 
here...)

I don't see why we in "slowpath" should read revs backwards or use 
windows, so the patch is fine when it now always moves forward - and for 
the specified revs.

> But since no revision range
> was specified, it was equivalent to reading the full changelog, even if a
> single revision was passed on the commandline.
>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -1103,15 +1103,11 @@
>                                  'filenames'))
>
>           # The slow path checks files modified in every changeset.
> -        def changerevgen():
> -            for i, window in increasing_windows(len(repo) - 1, nullrev):
> -                for j in xrange(i - window, i + 1):
> -                    yield change(j)
> -
> -        for ctx in changerevgen():
> +        for i in sorted(revs):
> +            ctx = change(i)
>               matches = filter(match, ctx.files())
> +            fncache[ctx.rev()] = matches

I assume you could use i instead of ctx.rev() ?

>               if matches:
> -                fncache[ctx.rev()] = matches
>                   wanted.add(ctx.rev())
>
>       class followfilter(object):

/Mads