[PATCH 2 of 2] demandmod: avoid recusive use of __getattribute__

Gregory Szorc gregory.szorc at gmail.com
Sat Oct 3 13:43:32 CDT 2015


# HG changeset patch
# User Gregory Szorc <gregory.szorc at gmail.com>
# Date 1443897658 25200
#      Sat Oct 03 11:40:58 2015 -0700
# Node ID 8c056b5c43b94cde65e6e784db75e74a4d960c24
# Parent  faccbd648e42a08a491dc306b37fdd2886ed41cc
demandmod: avoid recusive use of __getattribute__

__getattribute__ intercepts *all* requests for attributes, even if they
already exist. In order to actually access attributes on the current
object, you need to call object.__getattribute__(self, attribute). Proxy
classes like _demandmod have a __getattribute__ handler that maintains
a whitelist of known local class attributes so it can call
object.__getattribute__ appropriately.

Since self.<attribute> lookups for known class attributes are always
going to go through object.__getattribute__ anyway and since Python
function calls are relatively expensive, we replace most self.<attribute>
lookups in the _demandmod class with direct object.__getattribute__
calls. This reduces function calls and eliminates recursive calling of
_demandmod.__getattribute__.

Profiling says this reduced the number of recursive calls of
_demandmod.__getattribute__ in an `hg up` operation on mozilla-central
by ~539,000 and this shaved ~200ms off command time. As impressive as
this is, the total number of calls to _demandmod.__getattribute__
shouldn't be so great. The modification of imports in posix.py in
224a33452ed4 somehow resulted in the "encoding" symbol on the posix
module not being replaced by the actual loaded module, meaning all
"encoding.asciilower" symbol lookups during dirstate reading go through
our proxy module, dramically increasing _demandmod.__getattribute__ call
count. I think there is a deficiency in the demand module mechanism
where symbol replacement in the locals symbol table depends on module
load order or something. For now, this patch corrects one suboptimal
aspect of the proxy module. But a larger issue remains...

diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
--- a/mercurial/demandimport.py
+++ b/mercurial/demandimport.py
@@ -76,13 +76,14 @@ class _demandmod(object):
                            (head, globals, locals, after, level))
         object.__setattr__(self, "_module", None)
     def _extend(self, name):
         """add to the list of submodules to load"""
-        self._data[3].append(name)
+        object.__getattribute__(self, '_data')[3].append(name)
     def _load(self):
-        mod = self._module
+        mod = object.__getattribute__(self, '_module')
         if not mod:
-            head, globals, locals, after, level = self._data
+            head, globals, locals, after, level = \
+                    object.__getattribute__(self, '_data')
             mod = _hgextimport(_import, head, globals, locals, None, level)
             # load submodules
             def subload(mod, p):
                 h, t = p, None
@@ -111,12 +112,12 @@ class _demandmod(object):
         raise TypeError("%s object is not callable" % repr(self))
     def __getattribute__(self, attr):
         if attr in ('_data', '_extend', '_load', '_module'):
             return object.__getattribute__(self, attr)
-        mod = self._load()
+        mod = object.__getattribute__(self, '_load')()
         return getattr(mod, attr)
     def __setattr__(self, attr, val):
-        mod = self._load()
+        mod = object.__getattribute__(self, '_load')()
         setattr(mod, attr, val)
 
 def _demandimport(name, globals=None, locals=None, fromlist=None, level=level):
     if not locals or name in ignore or fromlist == ('*',):
@@ -131,9 +132,9 @@ def _demandimport(name, globals=None, lo
                 return _import(name, globals, locals, fromlist, level)
             # if a is already demand-loaded, add b to its submodule list
             if base in locals:
                 if isinstance(locals[base], _demandmod):
-                    locals[base]._extend(rest)
+                    object.__getattribute__(locals[base], '_extend')(rest)
                 return locals[base]
         return _demandmod(name, globals, locals, level)
     else:
         # There is a fromlist.


More information about the Mercurial-devel mailing list