[PATCH 7 of 7] revlog: avoid apply delta chain when cache hit in revlog.revision

[Ryan McElroy]

On 3/28/17 8:49 AM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1490683696 25200
> #      Mon Mar 27 23:48:16 2017 -0700
> # Node ID 8c9f728ef3a3fff029d7fe6c875ed783c66dc254
> # Parent  d3d803ed16fe8e9d43f7a4daeca079e4022c297a
> revlog: avoid apply delta chain when cache hit in revlog.revision
>
> Previously, revlog.revision(raw=False) may try to apply the delta chain
> even on cache hit. That could happen if flags are non-empty so all fast
> paths won't be executed. But if cache hit, the raw test is still useful.
> So let's use it.
>
> diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> --- a/mercurial/revlog.py
> +++ b/mercurial/revlog.py
> @@ -1259,19 +1259,20 @@ class revlog(object):
>           cachedrev = None
>           flags = None
> +        text = None
>           if node == nullid:
>               return ""
>           if self._cache:
>               if self._cache[0] == node:
> +                text = self._cache[2]
>                   if raw:
> -                    return self._cache[2]
> +                    return text
>                   if rev is None:
>                       rev = self.rev(node)
>                   flags = self.flags(rev)
>                   if not flags:
> -                    return self._cache[2]
> +                    return text
>               cachedrev = self._cache[1]
>   
>           # look up what we need to read
> -        text = None
>           if rev is None:
>               rev = self.rev(node)
>

This one lgtm

This overall series is a bit hard to review mostly because the code 
you're modifying is pretty dense. The patches themselves made a lot of 
sense once I dove into the code, though.

I have enough review comments that I think it's obvious that I would 
like to see a second version (after you've responded to anything you 
don't think should change and we've had that discussion, if needed). 
Also feel free to "respond" by adding comments in a v2 where I asked 
about things and you think they should stay the same.

Overall, these changes clearly correct some pretty nasty bugs that would 
bite us badly when we start using flag processing. Thanks for diving 
into this tricky code area and fixing them, and adding a good test to 
prevent regressions.

I'll mark these are "changes requested" in patchview in anticipation of 
a V2.