Patchwork [V2] dirstate: test normalize is truthy instead of using a no-op lambda

login
register
mail settings
Submitter Siddharth Agarwal
Date Dec. 4, 2012, 6:29 p.m.
Message ID <6688ba567bcd57d05e88.1354645786@sid0x220>
Download mbox | patch
Permalink /patch/13/
State Accepted
Commit a9e623bb440edc298f2bd46aaa8c0941bcc5503f
Headers show

Comments

Siddharth Agarwal - Dec. 4, 2012, 6:29 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0 at fb.com>
# Date 1354645758 28800
# Node ID 6688ba567bcd57d05e8867ba3c3560d38dafbac4
# Parent  11191f1c3d088cc8af01afda410a47928e551683
dirstate: test normalize is truthy instead of using a no-op lambda

hg perfstatus -u on a working directory with 170,000 files, without this
change:
! wall 1.869404 comb 1.850000 user 1.170000 sys 0.680000 (best of 6)

With this change:
! wall 1.839561 comb 1.830000 user 1.120000 sys 0.710000 (best of 6)
Dirkjan Ochtman - Dec. 4, 2012, 6:38 p.m.
On Tue, Dec 4, 2012 at 7:29 PM, Siddharth Agarwal <sid0 at fb.com> wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0 at fb.com>
> # Date 1354645758 28800
> # Node ID 6688ba567bcd57d05e8867ba3c3560d38dafbac4
> # Parent  11191f1c3d088cc8af01afda410a47928e551683
> dirstate: test normalize is truthy instead of using a no-op lambda
>
> hg perfstatus -u on a working directory with 170,000 files, without this
> change:
> ! wall 1.869404 comb 1.850000 user 1.170000 sys 0.680000 (best of 6)
>
> With this change:
> ! wall 1.839561 comb 1.830000 user 1.120000 sys 0.710000 (best of 6)
>
> diff -r 11191f1c3d08 -r 6688ba567bcd mercurial/dirstate.py
> --- a/mercurial/dirstate.py     Mon Dec 03 14:21:45 2012 -0800
> +++ b/mercurial/dirstate.py     Tue Dec 04 10:29:18 2012 -0800
> @@ -605,7 +605,7 @@
>              normalize = self._normalize
>              skipstep3 = False
>          else:
> -            normalize = lambda x, y, z: x
> +            normalize = None
>
>          files = sorted(match.files())
>          subrepos.sort()
> @@ -626,7 +626,10 @@
>
>          # step 1: find all explicit files
>          for ff in files:
> -            nf = normalize(normpath(ff), False, True)
> +            if normalize:
> +                nf = normalize(normpath(ff), False, True)
> +            else:
> +                nf = normpath(ff)

Did you also test with "if normalize is None:"?

Cheers,

Dirkjan

>              if nf in results:
>                  continue
>
> @@ -676,7 +679,10 @@
>                      continue
>                  raise
>              for f, kind, st in entries:
> -                nf = normalize(nd and (nd + "/" + f) or f, True, True)
> +                if normalize:
> +                    nf = normalize(nd and (nd + "/" + f) or f, True, True)
> +                else:
> +                    nf = nd and (nd + "/" + f) or f
>                  if nf not in results:
>                      if kind == dirkind:
>                          if not ignore(nf):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Siddharth Agarwal - Dec. 4, 2012, 7 p.m.
On 12/04/2012 10:38 AM, Dirkjan Ochtman wrote:
> Did you also test with "if normalize is None:"?
>
> Cheers,
>
> Dirkjan

That was V1 :)

"if normalize:" is faster in microbenchmarks, but I didn't see a 
difference either way in actual use. However mpm asked me to switch.
Dirkjan Ochtman - Dec. 4, 2012, 7:20 p.m.
On Tue, Dec 4, 2012 at 8:00 PM, Siddharth Agarwal <sid0 at fb.com> wrote:
> That was V1 :)

Ah, sorry. Thanks for elaborating.

Cheers,

Dirkjan
Kevin Bullock - Dec. 4, 2012, 8:32 p.m.
On 4 Dec 2012, at 12:29 PM, Siddharth Agarwal wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0 at fb.com>
> # Date 1354645758 28800
> # Node ID 6688ba567bcd57d05e8867ba3c3560d38dafbac4
> # Parent  11191f1c3d088cc8af01afda410a47928e551683
> dirstate: test normalize is truthy instead of using a no-op lambda
> 
> hg perfstatus -u on a working directory with 170,000 files, without this
> change:
> ! wall 1.869404 comb 1.850000 user 1.170000 sys 0.680000 (best of 6)
> 
> With this change:
> ! wall 1.839561 comb 1.830000 user 1.120000 sys 0.710000 (best of 6)

I don't have a big enough repo handy to stress-test this on, but on smallish repos it certainly doesn't harm performance (Mac OS X here). LGTM.

pacem in terris / ??? / ?????? / ????????? / ??
Kevin R. Bullock
Bryan O'Sullivan - Dec. 4, 2012, 8:48 p.m.
On Tue, Dec 4, 2012 at 10:29 AM, Siddharth Agarwal <sid0 at fb.com> wrote:

> dirstate: test normalize is truthy instead of using a no-op lambda
>

Pushed to crew, thanks.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20121204/25322dc3/attachment.html>

Patch

diff -r 11191f1c3d08 -r 6688ba567bcd mercurial/dirstate.py
--- a/mercurial/dirstate.py	Mon Dec 03 14:21:45 2012 -0800
+++ b/mercurial/dirstate.py	Tue Dec 04 10:29:18 2012 -0800
@@ -605,7 +605,7 @@ 
             normalize = self._normalize
             skipstep3 = False
         else:
-            normalize = lambda x, y, z: x
+            normalize = None
 
         files = sorted(match.files())
         subrepos.sort()
@@ -626,7 +626,10 @@ 
 
         # step 1: find all explicit files
         for ff in files:
-            nf = normalize(normpath(ff), False, True)
+            if normalize:
+                nf = normalize(normpath(ff), False, True)
+            else:
+                nf = normpath(ff)
             if nf in results:
                 continue
 
@@ -676,7 +679,10 @@ 
                     continue
                 raise
             for f, kind, st in entries:
-                nf = normalize(nd and (nd + "/" + f) or f, True, True)
+                if normalize:
+                    nf = normalize(nd and (nd + "/" + f) or f, True, True)
+                else:
+                    nf = nd and (nd + "/" + f) or f
                 if nf not in results:
                     if kind == dirkind:
                         if not ignore(nf):