[PATCH] hgweb: build the "entries" list directly in filelog command

[Martin von Zweigbergk]

On Sat, Jan 14, 2017, 23:52 Pierre-Yves David <
pierre-yves.david at ens-lyon.org> wrote:

>
>
> On 01/14/2017 06:14 AM, Gregory Szorc wrote:
> > On Fri, Jan 13, 2017 at 8:08 AM, Pierre-Yves David
> > <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>>
> > wrote:
> >
> >
> >
> >     On 01/13/2017 04:31 PM, Denis Laxalde wrote:
> >
> >         # HG changeset patch
> >         # User Denis Laxalde <denis.laxalde at logilab.fr
> >         <mailto:denis.laxalde at logilab.fr>>
> >         # Date 1484299345 -3600
> >         #      Fri Jan 13 10:22:25 2017 +0100
> >         # Node ID ad41cc2eb799156b0de2dd71adeed88ec42a98d9
> >         # Parent  e882c7bb5a0ba2589a44108c9a87b300a13e08df
> >         hgweb: build the "entries" list directly in filelog command
> >
> >         There's no apparent reason to have this "entries" generator
> >         function that
> >         builds a list and then yields its elements in reverse order and
> >         which is only
> >         called to build the "entries" list. So just build the list
> >         directly, in
> >         reverse order.
> >
> >
> >     revlog are usually read from 0 to max for performance reason (data
> >     in N+1 might rely on data on N and last access might be cached).
> >     This might explain why the list was build in one direction and then
> >     reversed.
> >     Can you double check if such mechanism is in play in this case?
> >
> >
> > Your observation about desiring to iterate revlogs from 0 to max is spot
> > on and this patch would normally be wrong. However, in this code `start
> >> end` (as can be seen in the context just above the first changed
> > line), so we're already iterating in the suboptimal direction. This
> > patch actually fixes that by throwing in a reversed() of a descending
> > fctx.filelog().revs()!
>

These are the lines from the context:

count = fctx.filerev() + 1
start = max(0, fctx.filerev() - revcount + 1) # first rev on this page
end = min(count, start + revcount) # last rev on this page

Let's say count=100, revcount=10. Then start=90 and end=100. Since you two
(Greg and Pierre-Yves) agree that start > end, I suspect I'm just missing
something, but I couldn't figure out what.

(Btw, "start = max(0, count - revcount)" would be slightly simpler.)


>
> > (This confused me on first read too.)
>
> oh, right. Thanks for pointing that out.
>
> I've pushed that patch Thanks!
>
> Cheers,
>
> --
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170115/adf7adfa/attachment.html>