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

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Aug. 8, 2016, 9:59 a.m.
Message ID <246936fc48ebc95a6f82.1470650382@juju>
Download mbox | patch
Permalink /patch/16193/
State Changes Requested
Delegated to: Yuya Nishihara
Headers show

Comments

Katsunori FUJIWARA - Aug. 8, 2016, 9:59 a.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1470649697 -32400
#      Mon Aug 08 18:48:17 2016 +0900
# Node ID 246936fc48ebc95a6f820589cc8a0d765fcf1967
# Parent  31662cb0696bf10444244d8749a2fb6ba485ad77
perf: avoid actual writing branch cache out correctly

9b6ae29d4801 (or 2.5) 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.

In this patch, replacement by "replace*" functions 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 17.994683  18.232455
  2.6 18.292930  18.156702
  2.7 18.728485  18.349210
  2.8 18.254513  18.528422
  2.9 17.303533  16.989655
  3.0 17.236667  17.615012
  3.1 17.139160  17.689805
  3.2 17.538039  17.718367
  3.3  2.652611   2.707960
  3.4  3.260117   3.272060
  3.5  3.375696   3.352176
  3.6  3.301309   3.242455
  3.7  3.331894   3.367328
  3.8  3.365343   3.267298
  3.9  3.332564   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
@@ -843,16 +843,17 @@  def perfbranchmap(ui, repo, full=False, 
             repo.filtered(name).branchmap()
     # add unfiltered
     allfilters.append(None)
-    oldread = branchmap.read
-    oldwrite = branchmap.branchcache.write
+
+    replaceread = safeattrsetter(branchmap, 'read')
+    replacewrite = safeattrsetter(branchmap.branchcache, 'write')
+    restoreread = replaceread(lambda repo: None)
+    restorewrite = replacewrite(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
+        restoreread()
+        restorewrite()
     fm.end()
 
 @command('perfloadmarkers')