Patchwork [2,of,6] scmutil.addremove: use the dirstate's map directly

login
register
mail settings
Submitter Siddharth Agarwal
Date April 2, 2013, 7:36 p.m.
Message ID <891827de9c10b0bbedd2.1364931393@sid0x220>
Download mbox | patch
Permalink /patch/1240/
State Superseded
Headers show

Comments

Siddharth Agarwal - April 2, 2013, 7:36 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1364888347 25200
#      Tue Apr 02 00:39:07 2013 -0700
# Node ID 891827de9c10b0bbedd2e74ef48189dfb4af77e1
# Parent  d723353c8a0ab2ced8606014abee43aa0b1e3fa4
scmutil.addremove: use the dirstate's map directly

On a large repository with 170,000 files, this speeds up perfaddremove from
2.78 seconds to 2.29.
Siddharth Agarwal - April 2, 2013, 8:35 p.m.
On 04/02/2013 12:49 PM, Kevin Bullock wrote:
> Boo, hiss. If dirstate's map needs to be a public object (and it looks like there's several places where it does), let's refactor it into a proper external collaborator of the dirstate object.

I was expecting this :)

So I went back and decided to try just pulling the repo.dirstate fetch 
out of the loop. Turns out that gets us most of the win:
Before: 2.78s
repo.dirstate out of the loop: 2.40s
directly accessing the dirstate map: 2.29s

Would just pulling the dirstate fetch out of the loop be fine for now?
Bryan O'Sullivan - April 2, 2013, 11:02 p.m.
On Tue, Apr 2, 2013 at 1:35 PM, Siddharth Agarwal <sid0@fb.com> wrote:

> Would just pulling the dirstate fetch out of the loop be fine for now?
>

What does "the dirstate fetch" mean here?
Siddharth Agarwal - April 2, 2013, 11:03 p.m.
On 04/02/2013 04:02 PM, Bryan O'Sullivan wrote:
> On Tue, Apr 2, 2013 at 1:35 PM, Siddharth Agarwal <sid0@fb.com 
> <mailto:sid0@fb.com>> wrote:
>
>     Would just pulling the dirstate fetch out of the loop be fine for now?
>
>
> What does "the dirstate fetch" mean here?

Fetching the dirstate attribute from the repo object, i.e. repo.dirstate.

Note that I already sent a V2 which does do that.

Patch

diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -676,10 +676,11 @@  def addremove(repo, pats=[], opts={}, dr
     m.bad = lambda x, y: rejected.append(x)
 
     ctx = repo[None]
+    dget = repo.dirstate._map.get
     walkresults = repo.dirstate.walk(m, sorted(ctx.substate), True, False)
     for abs in sorted(walkresults):
         st = walkresults[abs]
-        dstate = repo.dirstate[abs]
+        dstate = dget(abs, ('?',))[0]
         if dstate == '?' and audit_path.check(abs):
             unknown.append(abs)
             if repo.ui.verbose or not m.exact(abs):