[PATCH 2 of 3 V2] demandimport: avoid recusive use of __getattribute__

Gregory Szorc gregory.szorc at gmail.com
Sat Oct 3 14:58:34 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 751a5e46ab943ee93eb43c6248b878edd237e92d
# Parent  c2ee734ddd5b06715219077d372053612bc8ae44
demandimport: 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 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. This is due to suboptimal handling of references to _demandmod
instances from multiple importers and will be fixed in a subsequent
patch.

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