changeset 30143:2d858c771760

perf: introduce safeattrsetter to replace direct attribute assignment Referring not-existing attribute immediately causes failure, but assigning a value to such attribute doesn't. For example, perf.py has code paths below, which assign a value to not-existing attribute. This causes incorrect performance measurement, but these code paths are executed successfully. - "repo._tags = None" in perftags() recent Mercurial has tags cache information in repo._tagscache - "branchmap.write = lambda repo: None" in perfbranchmap() branchmap cache is written out by branchcache.write() in branchmap.py "util.safehasattr() before assignment" can avoid this issue, but might increase mistake at "copy & paste" attribute name or so. To centralize (1) examining existence of, (2) assigning a value to, and (3) restoring an old value to the attribute, this patch introduces safeattrsetter(). This is used to replace direct attribute assignment in subsequent patches. Encapsulation of restoring is needed to completely remove direct attribute assignment from perf.py, even though restoring isn't needed so often.
author FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
date Sun, 09 Oct 2016 01:03:16 +0900
parents 3dcaf1c4e90d
children 14031d183048
files contrib/perf.py
diffstat 1 files changed, 34 insertions(+), 0 deletions(-) [+]
line wrap: on
line diff
--- a/contrib/perf.py	Sat Oct 08 00:59:41 2016 +0200
+++ b/contrib/perf.py	Sun Oct 09 01:03:16 2016 +0900
@@ -182,6 +182,40 @@
     fm.write('count',  ' (best of %d)', count)
     fm.plain('\n')
 
+# utilities for historical portability
+
+def safeattrsetter(obj, name, ignoremissing=False):
+    """Ensure that 'obj' has 'name' attribute before subsequent setattr
+
+    This function is aborted, if 'obj' doesn't have 'name' attribute
+    at runtime. This avoids overlooking removal of an attribute, which
+    breaks assumption of performance measurement, in the future.
+
+    This function returns the object to (1) assign a new value, and
+    (2) restore an original value to the attribute.
+
+    If 'ignoremissing' is true, missing 'name' attribute doesn't cause
+    abortion, and this function returns None. This is useful to
+    examine an attribute, which isn't ensured in all Mercurial
+    versions.
+    """
+    if not util.safehasattr(obj, name):
+        if ignoremissing:
+            return None
+        raise error.Abort(("missing attribute %s of %s might break assumption"
+                           " of performance measurement") % (name, obj))
+
+    origvalue = getattr(obj, name)
+    class attrutil(object):
+        def set(self, newvalue):
+            setattr(obj, name, newvalue)
+        def restore(self):
+            setattr(obj, name, origvalue)
+
+    return attrutil()
+
+# perf commands
+
 @command('perfwalk', formatteropts)
 def perfwalk(ui, repo, *pats, **opts):
     timer, fm = gettimer(ui, opts)