From patchwork Fri Oct 27 20:14:07 2017
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: D1257: dirstate: remove excess attribute lookups for dirstate.status
 (issue5714)
From: phabricator <phabricator@mercurial-scm.org>
X-Patchwork-Id: 25293
Message-Id: <differential-rev-PHID-DREV-3digpzh4ngihicyha7gb-req@phab.mercurial-scm.org>
To: mercurial-devel@mercurial-scm.org
Date: Fri, 27 Oct 2017 20:14:07 +0000

durham created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  A recent refactor added a layer of abstraction to the dirstate which makes doing
  things like 'foo in dirstate' now require some extra Python attribute lookups.
  This is causing a 100ms slow down in hg status for mozilla-central.
  
  The fix is to hoist the inner dict's functions onto the main class once the lazy
  loading it complete, as well as store the actual functions before doing the
  status loop (as is done for other such functions).
  
  In my testing, it seems to address the performance regression, but we'll
  need to see the perf run results to know for sure.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1257

AFFECTED FILES
  mercurial/dirstate.py

CHANGE DETAILS




To: durham, #hg-reviewers
Cc: mercurial-devel

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -1053,6 +1053,9 @@
         removed, deleted, clean = [], [], []
 
         dmap = self._map
+        dmap.preload()
+        dcontains = dmap.__contains__
+        dget = dmap.__getitem__
         ladd = lookup.append            # aka "unsure"
         madd = modified.append
         aadd = added.append
@@ -1074,7 +1077,7 @@
         full = listclean or match.traversedir is not None
         for fn, st in self.walk(match, subrepos, listunknown, listignored,
                                 full=full).iteritems():
-            if fn not in dmap:
+            if not dcontains(fn):
                 if (listignored or mexact(fn)) and dirignore(fn):
                     if listignored:
                         iadd(fn)
@@ -1089,7 +1092,7 @@
             # a list, but falls back to creating a full-fledged iterator in
             # general. That is much slower than simply accessing and storing the
             # tuple members one by one.
-            t = dmap[fn]
+            t = dget(fn)
             state = t[0]
             mode = t[1]
             size = t[2]
@@ -1247,6 +1250,10 @@
     def keys(self):
         return self._map.keys()
 
+    def preload(self):
+        """Loads the underlying data, if it's not already loaded"""
+        self._map
+
     def nonnormalentries(self):
         '''Compute the nonnormal dirstate entries from the dmap'''
         try:
@@ -1373,6 +1380,13 @@
         if not self._dirtyparents:
             self.setparents(*p)
 
+        # Avoid excess attribute lookups by fast pathing certain checks
+        self.__contains__ = self._map.__contains__
+        self.__getitem__ = self._map.__getitem__
+        self.__setitem__ = self._map.__setitem__
+        self.__delitem__ = self._map.__delitem__
+        self.get = self._map.get
+
     def write(self, st, now):
         st.write(parsers.pack_dirstate(self._map, self.copymap,
                                        self.parents(), now))