Patchwork [9,of,9,V2] revlog: add a fast path for revision(raw=False)

login
register
mail settings
Submitter Jun Wu
Date March 31, 2017, 4:45 a.m.
Message ID <70c6b2eecf4d580d3840.1490935518@x1c>
Download mbox | patch
Permalink /patch/19861/
State Accepted
Headers show

Comments

Jun Wu - March 31, 2017, 4:45 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1490934075 25200
#      Thu Mar 30 21:21:15 2017 -0700
# Node ID 70c6b2eecf4d580d38404f157ef99da237593a68
# Parent  3a4dd24ccf078c47722582f00872a88d33042ac3
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 70c6b2eecf4d
revlog: add a fast path for revision(raw=False)

If cache hit and flags are empty, no flag processor runs and "text" equals
to "rawtext". So we check flags, and return rawtext.

This resolves performance issue introduced by a previous patch.
Ryan McElroy - March 31, 2017, 10:08 a.m.
On 3/31/17 5:45 AM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1490934075 25200
> #      Thu Mar 30 21:21:15 2017 -0700
> # Node ID 70c6b2eecf4d580d38404f157ef99da237593a68
> # Parent  3a4dd24ccf078c47722582f00872a88d33042ac3
> revlog: add a fast path for revision(raw=False)

Wow! This series is enormously more clear than it's precursor. The way 
you refactored the test is superb and clearly shows the progression of 
the fixes. This is fantastic!

I have reviewed the patches and they all look good to me. These are 
important fixes and they are now clearly presented.

*slowclap*

(Marking as pre-reviewed in patchwork)

>
> If cache hit and flags are empty, no flag processor runs and "text" equals
> to "rawtext". So we check flags, and return rawtext.
>
> This resolves performance issue introduced by a previous patch.
>
> diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> --- a/mercurial/revlog.py
> +++ b/mercurial/revlog.py
> @@ -1275,4 +1275,11 @@ class revlog(object):
>                   if raw:
>                       return self._cache[2]
> +                # duplicated, but good for perf
> +                if rev is None:
> +                    rev = self.rev(node)
> +                # no extra flags set, no flag processor runs, text = rawtext
> +                if self.flags(rev) == REVIDX_DEFAULT_FLAGS:
> +                    return self._cache[2]
> +
>               cachedrev = self._cache[1]
>   
>
Yuya Nishihara - April 2, 2017, 8:40 a.m.
On Fri, 31 Mar 2017 11:08:56 +0100, Ryan McElroy wrote:
> On 3/31/17 5:45 AM, Jun Wu wrote:
> > # HG changeset patch
> > # User Jun Wu <quark@fb.com>
> > # Date 1490934075 25200
> > #      Thu Mar 30 21:21:15 2017 -0700
> > # Node ID 70c6b2eecf4d580d38404f157ef99da237593a68
> > # Parent  3a4dd24ccf078c47722582f00872a88d33042ac3
> > revlog: add a fast path for revision(raw=False)
> 
> Wow! This series is enormously more clear than it's precursor. The way 
> you refactored the test is superb and clearly shows the progression of 
> the fixes. This is fantastic!

Yeah, queued with delight, thanks.

> > --- a/mercurial/revlog.py
> > +++ b/mercurial/revlog.py
> > @@ -1275,4 +1275,11 @@ class revlog(object):
> >                   if raw:
> >                       return self._cache[2]
> > +                # duplicated, but good for perf
> > +                if rev is None:
> > +                    rev = self.rev(node)
> > +                # no extra flags set, no flag processor runs, text = rawtext
> > +                if self.flags(rev) == REVIDX_DEFAULT_FLAGS:
> > +                    return self._cache[2]

Maybe flags can be cached to avoid extra node->rev lookup.

Patch

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -1275,4 +1275,11 @@  class revlog(object):
                 if raw:
                     return self._cache[2]
+                # duplicated, but good for perf
+                if rev is None:
+                    rev = self.rev(node)
+                # no extra flags set, no flag processor runs, text = rawtext
+                if self.flags(rev) == REVIDX_DEFAULT_FLAGS:
+                    return self._cache[2]
+
             cachedrev = self._cache[1]