Patchwork [3,of,6] changegroup: remove one special case from lookupmflinknode

login
register
mail settings
Submitter Augie Fackler
Date Dec. 4, 2015, 7:20 p.m.
Message ID <1477b679417ada21523e.1449256838@arthedain.pit.corp.google.com>
Download mbox | patch
Permalink /patch/11812/
State Superseded
Headers show

Comments

Augie Fackler - Dec. 4, 2015, 7:20 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1449244546 18000
#      Fri Dec 04 10:55:46 2015 -0500
# Node ID 1477b679417ada21523eeb4ae945e907e8df5f8c
# Parent  ef0693b1540667fd183fe1364dbf308f919a0fd6
# EXP-Topic cg3
changegroup: remove one special case from lookupmflinknode

In the fastpathlinkrev case, lookupmflinknode was a very complicated
way of saying mfs.__getitem__, so let's just get that case out of our
way so it's easier to understand what's going on.

Timings for bundle --all on my hg repo, tested with hgperf:
Before:
! wall 23.442445 comb 23.440000 user 23.250000 sys 0.190000 (best of 3)

After:
! wall 20.272187 comb 20.270000 user 20.190000 sys 0.080000 (best of 3)
Martin von Zweigbergk - Dec. 4, 2015, 8:38 p.m.
On Fri, Dec 4, 2015 at 11:25 AM Augie Fackler <raf@durin42.com> wrote:

> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1449244546 18000
> #      Fri Dec 04 10:55:46 2015 -0500
> # Node ID 1477b679417ada21523eeb4ae945e907e8df5f8c
> # Parent  ef0693b1540667fd183fe1364dbf308f919a0fd6
> # EXP-Topic cg3
> changegroup: remove one special case from lookupmflinknode
>
> In the fastpathlinkrev case, lookupmflinknode was a very complicated
> way of saying mfs.__getitem__, so let's just get that case out of our
> way so it's easier to understand what's going on.
>
> Timings for bundle --all on my hg repo, tested with hgperf:
> Before:
> ! wall 23.442445 comb 23.440000 user 23.250000 sys 0.190000 (best of 3)
>
> After:
> ! wall 20.272187 comb 20.270000 user 20.190000 sys 0.080000 (best of 3)
>

As we talked about on IM, the speedup seems to come from 1/6, not from this
patch. I'll move these paragraphs to that patch in flight.


>
> diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
> --- a/mercurial/changegroup.py
> +++ b/mercurial/changegroup.py
> @@ -7,6 +7,7 @@
>
>  from __future__ import absolute_import
>
> +import operator
>  import os
>  import struct
>  import tempfile
> @@ -656,21 +657,23 @@ class cg1packer(object):
>          # Callback for the manifest, used to collect linkrevs for filelog
>          # revisions.
>          # Returns the linkrev node (collected in lookupcl).
> -        def lookupmflinknode(x):
> -            """Callback for looking up the linknode for manifests.
> +        if fastpathlinkrev:
> +            lookupmflinknode = mfs.__getitem__
> +        else:
> +            def lookupmflinknode(x):
> +                """Callback for looking up the linknode for manifests.
>
> -            Returns the linkrev node for the specified manifest.
> +                Returns the linkrev node for the specified manifest.
>
> -            SIDE EFFECT:
> +                SIDE EFFECT:
>
> -              fclnodes gets populated with the list of relevant
> -              file nodes if we're not using fastpathlinkrev.
> +                  fclnodes gets populated with the list of relevant
> +                  file nodes.
>
> -            Note that this means you can't trust fclnodes until
> -            after manifests have been sent to the client.
> -            """
> -            clnode = mfs[x]
> -            if not fastpathlinkrev:
> +                Note that this means you can't trust fclnodes until
> +                after manifests have been sent to the client.
> +                """
> +                clnode = mfs[x]
>                  mdata = ml.readfast(x)
>                  for f in mfchangedfiles[x]:
>                      try:
> @@ -683,7 +686,7 @@ class cg1packer(object):
>                      fclnode = fclnodes.setdefault(n, clnode)
>                      if clrevorder[clnode] < clrevorder[fclnode]:
>                          fclnodes[n] = clnode
> -            return clnode
> +                return clnode
>
>          mfnodes = self.prune(ml, mfs, commonrevs)
>          for x in self._packmanifests(mfnodes, lookupmflinknode):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -7,6 +7,7 @@ 
 
 from __future__ import absolute_import
 
+import operator
 import os
 import struct
 import tempfile
@@ -656,21 +657,23 @@  class cg1packer(object):
         # Callback for the manifest, used to collect linkrevs for filelog
         # revisions.
         # Returns the linkrev node (collected in lookupcl).
-        def lookupmflinknode(x):
-            """Callback for looking up the linknode for manifests.
+        if fastpathlinkrev:
+            lookupmflinknode = mfs.__getitem__
+        else:
+            def lookupmflinknode(x):
+                """Callback for looking up the linknode for manifests.
 
-            Returns the linkrev node for the specified manifest.
+                Returns the linkrev node for the specified manifest.
 
-            SIDE EFFECT:
+                SIDE EFFECT:
 
-              fclnodes gets populated with the list of relevant
-              file nodes if we're not using fastpathlinkrev.
+                  fclnodes gets populated with the list of relevant
+                  file nodes.
 
-            Note that this means you can't trust fclnodes until
-            after manifests have been sent to the client.
-            """
-            clnode = mfs[x]
-            if not fastpathlinkrev:
+                Note that this means you can't trust fclnodes until
+                after manifests have been sent to the client.
+                """
+                clnode = mfs[x]
                 mdata = ml.readfast(x)
                 for f in mfchangedfiles[x]:
                     try:
@@ -683,7 +686,7 @@  class cg1packer(object):
                     fclnode = fclnodes.setdefault(n, clnode)
                     if clrevorder[clnode] < clrevorder[fclnode]:
                         fclnodes[n] = clnode
-            return clnode
+                return clnode
 
         mfnodes = self.prune(ml, mfs, commonrevs)
         for x in self._packmanifests(mfnodes, lookupmflinknode):