Patchwork [5,of,8] merge: separate worker functions for batch remove and batch get

login
register
mail settings
Submitter Mads Kiilerich
Date May 1, 2014, 11:42 p.m.
Message ID <9d786e6be4319bf31467.1398987773@mk-desktop>
Download mbox | patch
Permalink /patch/4470/
State Superseded
Headers show

Comments

Mads Kiilerich - May 1, 2014, 11:42 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1398985754 -7200
#      Fri May 02 01:09:14 2014 +0200
# Branch stable
# Node ID 9d786e6be4319bf31467eb715fb432c68ccd4507
# Parent  feac4ee682bea8ebb19ead775bdb9ef413da8e3e
merge: separate worker functions for batch remove and batch get
Pierre-Yves David - May 7, 2014, 9:09 p.m.
On 05/01/2014 04:42 PM, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1398985754 -7200
> #      Fri May 02 01:09:14 2014 +0200
> # Branch stable
> # Node ID 9d786e6be4319bf31467eb715fb432c68ccd4507
> # Parent  feac4ee682bea8ebb19ead775bdb9ef413da8e3e
> merge: separate worker functions for batch remove and batch get


Why is this necessary//useful?
Mads Kiilerich - May 8, 2014, 11:39 a.m.
On 05/07/2014 11:09 PM, Pierre-Yves David wrote:
>
>
> On 05/01/2014 04:42 PM, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1398985754 -7200
>> #      Fri May 02 01:09:14 2014 +0200
>> # Branch stable
>> # Node ID 9d786e6be4319bf31467eb715fb432c68ccd4507
>> # Parent  feac4ee682bea8ebb19ead775bdb9ef413da8e3e
>> merge: separate worker functions for batch remove and batch get
>
>
> Why is this necessary//useful?

The old code had one function that could do 2 different things. First, 
is was called a bunch of times to do one thing. Next, it was called a 
bunch of times to do the other thing. That gave unnecessary complexity 
and a dispatch overhead. Having separate functions is "obviously" better 
than having a function that can do two things, depending on its parameters.

And again: it prepares for the 8th patch.

/Mads
Pierre-Yves David - May 8, 2014, 7:57 p.m.
On 05/08/2014 04:39 AM, Mads Kiilerich wrote:
> On 05/07/2014 11:09 PM, Pierre-Yves David wrote:
>>
>>
>> On 05/01/2014 04:42 PM, Mads Kiilerich wrote:
>>> # HG changeset patch
>>> # User Mads Kiilerich <madski@unity3d.com>
>>> # Date 1398985754 -7200
>>> #      Fri May 02 01:09:14 2014 +0200
>>> # Branch stable
>>> # Node ID 9d786e6be4319bf31467eb715fb432c68ccd4507
>>> # Parent  feac4ee682bea8ebb19ead775bdb9ef413da8e3e
>>> merge: separate worker functions for batch remove and batch get
>>
>>
>> Why is this necessary//useful?
>
> The old code had one function that could do 2 different things. First,
> is was called a bunch of times to do one thing. Next, it was called a
> bunch of times to do the other thing. That gave unnecessary complexity
> and a dispatch overhead. Having separate functions is "obviously" better
> than having a function that can do two things, depending on its parameters.
>
> And again: it prepares for the 8th patch.

Please apply update your changeset description with this convincing 
explanation.

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -557,23 +557,19 @@  actionpriority = dict((m, p) for p, m in
 def actionkey(a):
     return actionpriority[a[1]], a
 
-def getremove(repo, mctx, overwrite, args):
-    """apply usually-non-interactive updates to the working directory
-
-    mctx is the context to be merged into the working copy
+def batchremove(repo, actions):
+    """apply removes to the working directory
 
     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 f, m, args, msg in args:
-        repo.ui.debug(" %s: %s -> %s\n" % (f, msg, m))
-        if m == 'r':
+    for f, m, args, msg in actions:
+        repo.ui.debug(" %s: %s -> r\n" % (f, msg))
+        if True:
             if verbose:
                 repo.ui.note(_("removing %s\n") % f)
             audit(f)
@@ -582,7 +578,27 @@  def getremove(repo, mctx, overwrite, arg
             except OSError, inst:
                 repo.ui.warn(_("update failed to remove %s: %s!\n") %
                              (f, inst.strerror))
-        else:
+        if i == 100:
+            yield i, f
+            i = 0
+        i += 1
+    if i > 0:
+        yield i, f
+
+def batchget(repo, mctx, actions):
+    """apply gets to the working directory
+
+    mctx is the context to get from
+
+    yields tuples for progress updates
+    """
+    verbose = repo.ui.verbose
+    fctx = mctx.filectx
+    wwrite = repo.wwrite
+    i = 0
+    for f, m, args, msg in actions:
+        repo.ui.debug(" %s: %s -> g\n" % (f, msg))
+        if True:
             if verbose:
                 repo.ui.note(_("getting %s\n") % f)
             wwrite(f, fctx(f).data(), args[0])
@@ -654,15 +670,13 @@  def applyupdates(repo, actions, wctx, mc
 
     # remove in parallel (must come first)
     z = 0
-    prog = worker.worker(repo.ui, 0.001, getremove, (repo, mctx, overwrite),
-                         removeactions)
+    prog = worker.worker(repo.ui, 0.001, batchremove, (repo,), removeactions)
     for i, item in prog:
         z += i
         progress(_updating, z, item=item, total=numupdates, unit=_files)
 
     # get in parallel
-    prog = worker.worker(repo.ui, 0.001, getremove, (repo, mctx, overwrite),
-                         updateactions)
+    prog = worker.worker(repo.ui, 0.001, batchget, (repo, mctx), updateactions)
     for i, item in prog:
         z += i
         progress(_updating, z, item=item, total=numupdates, unit=_files)