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

# 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)

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

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.

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

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

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>