Patchwork [11,of,11] merge: don't fiddle with name lookups or i18n in hot loops

login
register
mail settings
Submitter Bryan O'Sullivan
Date Feb. 9, 2013, 2:06 p.m.
Message ID <994e4c0ed9a6d2ff25aa.1360418811@australite.local>
Download mbox | patch
Permalink /patch/875/
State Superseded
Commit d9ff580fcaa26199ab8d76f35f81169431ce84b3
Headers show

Comments

Bryan O'Sullivan - Feb. 9, 2013, 2:06 p.m.
# HG changeset patch
# User Bryan O'Sullivan <bryano@fb.com>
# Date 1360418465 0
# Node ID 994e4c0ed9a6d2ff25aacfcc9603ed9cef3909ac
# Parent  a278d66b8f8621ebafbb30a201000fca63983697
merge: don't fiddle with name lookups or i18n in hot loops

We perform attribute dereferences and i18n lookups before looping.
natosha@unity3d.com - Feb. 9, 2013, 10:28 p.m.
2013/2/9 Bryan O'Sullivan <bos@serpentine.com>

> # HG changeset patch
> # User Bryan O'Sullivan <bryano@fb.com>
> # Date 1360418465 0
> # Node ID 994e4c0ed9a6d2ff25aacfcc9603ed9cef3909ac
> # Parent  a278d66b8f8621ebafbb30a201000fca63983697
> merge: don't fiddle with name lookups or i18n in hot loops


I tested this entire patch series on the Unity repo

*Before the patch series, updating from null to tip (~57.000 files) on OS X:

real 1m17.607s
user 0m32.664s
sys 0m24.141s

After the patch series:

real 0m36.322s
user 0m38.497s
sys 0m30.022s*
Bryan O'Sullivan - Feb. 10, 2013, 12:28 a.m.
On Sat, Feb 9, 2013 at 2:28 PM, Na'Tosha Bard <natosha@unity3d.com> wrote:

> *Before the patch series, updating from null to tip (~57.000 files) on OS
> X:
> real 1m17.607s
>
> After the patch series:
> **real 0m36.322s*


Not bad. That matches my measurements on OS X, too - I think HFS+ is the
bottleneck (surprise!).

Patches are in crew now.
Idan Kamara - Feb. 10, 2013, 8:09 a.m.
On Sun, Feb 10, 2013 at 2:28 AM, Bryan O'Sullivan <bos@serpentine.com>
wrote:
> Patches are in crew now.

Someone takes the time to look over your work (on a Saturday..),
raises some unsettled concerns, but you decide to push it anyway?

Not cool.
Augie Fackler - Feb. 10, 2013, 11:08 a.m.
On Feb 10, 2013, at 8:09 AM, Idan Kamara <idankk86@gmail.com> wrote:

> On Sun, Feb 10, 2013 at 2:28 AM, Bryan O'Sullivan <bos@serpentine.com>
> wrote:
>> Patches are in crew now.
> 
> Someone takes the time to look over your work (on a Saturday..),
> raises some unsettled concerns, but you decide to push it anyway?
> 
> Not cool.

Relax - I reviewed this in person for Bryan, and he addressed both your and my comments before pushing.

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -321,21 +321,28 @@  def getremove(repo, mctx, overwrite, arg
 
     yields tuples for progress updates
     """
+    verbose = repo.ui.verbose
+    unlink = util.unlinkpath
+    wjoin = repo.wjoin
+    fctx = mctx.filectx
+    wwrite = repo.wwrite
     audit = repo.wopener.audit
     i = 0
     for arg in args:
         f = arg[0]
         if arg[1] == 'r':
-            repo.ui.note(_("removing %s\n") % f)
+            if verbose:
+                repo.ui.note(_("removing %s\n") % f)
             audit(f)
             try:
-                util.unlinkpath(repo.wjoin(f), ignoremissing=True)
+                unlink(wjoin(f), ignoremissing=True)
             except OSError, inst:
                 repo.ui.warn(_("update failed to remove %s: %s!\n") %
                              (f, inst.strerror))
         else:
-            repo.ui.note(_("getting %s\n") % f)
-            repo.wwrite(f, mctx.filectx(f).data(), arg[2][0])
+            if verbose:
+                repo.ui.note(_("getting %s\n") % f)
+            wwrite(f, fctx(f).data(), arg[2][0])
         if i == 100:
             yield i, f
             i = 0
@@ -414,10 +421,13 @@  def applyupdates(repo, actions, wctx, mc
     if hgsub and hgsub[0] == 'g':
         subrepo.submerge(repo, wctx, mctx, wctx, overwrite)
 
+    _updating = _('updating')
+    _files = _('files')
+    progress = repo.ui.progress
+
     for i, a in enumerate(actions):
         f, m, args, msg = a
-        repo.ui.progress(_('updating'), z + i + 1, item=f, total=numupdates,
-                         unit=_('files'))
+        progress(_updating, z + i + 1, item=f, total=numupdates, unit=_files)
         if m == "m": # merge
             if fd == '.hgsubstate': # subrepo states need updating
                 subrepo.submerge(repo, wctx, mctx, wctx.ancestor(mctx),
@@ -462,7 +472,7 @@  def applyupdates(repo, actions, wctx, mc
             util.setflags(repo.wjoin(f), 'l' in flags, 'x' in flags)
             updated += 1
     ms.commit()
-    repo.ui.progress(_('updating'), None, total=numupdates, unit=_('files'))
+    progress(_updating, None, total=numupdates, unit=_files)
 
     return updated, merged, removed, unresolved