Patchwork [3,of,8,V2] perf: avoid actual writing branch cache out correctly

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Oct. 8, 2016, 4:18 p.m.
Message ID <33038811b6e57e0fb743.1475943503@feefifofum>
Download mbox | patch
Permalink /patch/16944/
State Accepted
Headers show

Comments

Katsunori FUJIWARA - Oct. 8, 2016, 4:18 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1475942597 -32400
#      Sun Oct 09 01:03:17 2016 +0900
# Node ID 33038811b6e57e0fb743223db4c0c99a7c69c652
# Parent  ab2d4d5e7821c321017cdbdae7743824c0da76ff
perf: avoid actual writing branch cache out correctly

Mercurial 2.5 (or 9b6ae29d4801) introduced "perfbranchmap" command,
and tried to avoid actual writing branch cache out by replacing
write() of branchcache class in branchmap.py with no-op function
(probably, for elimination of noisy and heavy file I/O factor).

But its implementation isn't correct, because 9b6ae29d4801 replaced
not branchmap.branchcache.write() but branchmap.write(). The latter
doesn't exist, even at that change.

To avoid actual writing branch cache out correctly, this patch
replaces branchmap.branchcache.write() with no-op function.

To detect mistake of replacement or change of API in the future
quickly, this patch uses safeattrsetter() instead of direct attribute
assignment. For similarity between replacements, this patch also
changes replacement of branchmap.read().

In this patch, replacement of read()/write() can run safely outside
"try" block, because two safeattrsetter() invocations ensure that
replacement doesn't cause exception.

FYI, the table below compares "base" filter wall time of perfbranchmap
on recent mozilla-central repo with each Mercurial version between
before and after this patch.

  ==== ========= =========
  ver  before    after
  ==== ========= =========
  2.5  18.492334 18.232455
  2.6  18.733858 18.156702
  2.7  18.245598 18.349210
  2.8  18.289070 18.528422
  2.9  17.572742 16.989655
  3.0  17.406953 17.615012
  3.1  17.228419 17.689805
  3.2  17.862961 17.718367
  3.3   2.632110  2.707960
  3.4   3.285683  3.272060
  3.5   3.370141  3.352176
  3.6   3.366939  3.242455
  3.7   3.300778  3.367328
  3.8   3.300132  3.267298
  3.9   3.418996  3.370265
  ==== ========= =========

IMHO, there is no serious overlooking performance regression.

Patch

diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -884,16 +884,17 @@  def perfbranchmap(ui, repo, full=False, 
             repo.filtered(name).branchmap()
     # add unfiltered
     allfilters.append(None)
-    oldread = branchmap.read
-    oldwrite = branchmap.branchcache.write
+
+    branchcacheread = safeattrsetter(branchmap, 'read')
+    branchcachewrite = safeattrsetter(branchmap.branchcache, 'write')
+    branchcacheread.set(lambda repo: None)
+    branchcachewrite.set(lambda bc, repo: None)
     try:
-        branchmap.read = lambda repo: None
-        branchmap.write = lambda repo: None
         for name in allfilters:
             timer(getbranchmap(name), title=str(name))
     finally:
-        branchmap.read = oldread
-        branchmap.branchcache.write = oldwrite
+        branchcacheread.restore()
+        branchcachewrite.restore()
     fm.end()
 
 @command('perfloadmarkers')