Patchwork D7492: localrepo: also fastpath access to working copy parents when possible

login
register
mail settings
Submitter phabricator
Date Nov. 22, 2019, 9:23 a.m.
Message ID <differential-rev-PHID-DREV-6fxmaw33upreusbq7bbz-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/43435/
State Superseded
Headers show

Comments

phabricator - Nov. 22, 2019, 9:23 a.m.
marmoute created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  If the filter level garantee that the workig copy parents will be visible, we
  allow fast path access to them. With this change multiple commands can now run
  without triggering filtering.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/context.py
  mercurial/localrepo.py
  mercurial/scmutil.py
  tests/test-repo-filters-tiptoe.t

CHANGE DETAILS




To: marmoute, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 10, 2019, 7:38 a.m.
martinvonz added a comment.


  Hmm, I thought this entire series had been queued. I'd like to queue it. Can you include perf numbers?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7492/new/

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

To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
phabricator - Dec. 12, 2019, 12:42 p.m.
marmoute added a comment.


  In D7492#111595 <https://phab.mercurial-scm.org/D7492#111595>, @martinvonz wrote:
  
  > Hmm, I thought this entire series had been queued. I'd like to queue it. Can you include perf numbers?
  
  Now that we have a reference filtered repository, I am planning to use the benchmark suite to gather timing (probably next week). We already track status and diff. addign `hg export` is easy. However I am not sure what's your criteria for annotate ? Can you clarify them and offer a file to test for each for the reference repositories ?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7492/new/

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

To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
phabricator - Dec. 12, 2019, 2:31 p.m.
martinvonz added a comment.


  In D7492#111988 <https://phab.mercurial-scm.org/D7492#111988>, @marmoute wrote:
  
  > In D7492#111595 <https://phab.mercurial-scm.org/D7492#111595>, @martinvonz wrote:
  >
  >> Hmm, I thought this entire series had been queued. I'd like to queue it. Can you include perf numbers?
  >
  > Now that we have a reference filtered repository, I am planning to use the benchmark suite to gather timing (probably next week). We already track status and diff. addign `hg export` is easy. However I am not sure what's your criteria for annotate ? Can you clarify them and offer a file to test for each for the reference repositories ?
  
  Try the one I used as an example in my patch. I don't think I thought very hard about finding a particularly good example. You just want one that's fast to annotate so the time to load obsmarkers is a significant part of the total time.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7492/new/

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

To: marmoute, #hg-reviewers
Cc: martinvonz, mercurial-devel
phabricator - Jan. 9, 2020, 2:26 p.m.
pulkit added a comment.


  How's the perf number mining going? It will be nice to get this pushed before upcoming release candidate.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7492/new/

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

To: marmoute, #hg-reviewers
Cc: pulkit, martinvonz, mercurial-devel
phabricator - Jan. 16, 2020, 10:24 a.m.
marmoute added a comment.


  Here is raw perf comparison for status, diff and export. We don't have a benchmark for annottate yet. (not too long to write, but I did not had the time to do that yet)
  
    All benchmarks:
    
           before           after         ratio
         [8e095512]       [36b2f659]
    -       711±0.8ms       60.7±0.2ms     0.09  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
    -       712±0.8ms       61.6±0.2ms     0.09  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
    -         690±1ms       93.5±0.3ms     0.14  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
    -         688±1ms       93.8±0.3ms     0.14  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
    -         714±1ms       60.7±0.8ms     0.09  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
    -         713±1ms       60.9±0.3ms     0.09  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
    -         689±1ms       93.7±0.2ms     0.14  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
    -         687±2ms       92.8±0.2ms     0.14  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
    -         799±2ms       98.1±0.6ms     0.12  simple_command.read.export.bare.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
    -       800±0.8ms      100.0±0.4ms     0.12  simple_command.read.export.bare.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
    -       711±0.9ms        111±0.2ms     0.16  simple_command.read.export.bare.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
    -         711±1ms        112±0.3ms     0.16  simple_command.read.export.bare.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
    -         760±1ms       59.8±0.1ms     0.08  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
    -         763±2ms       62.2±0.3ms     0.08  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
    -         689±1ms       93.1±0.3ms     0.14  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
    -         688±1ms       94.3±0.3ms     0.14  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
    -         763±1ms       60.1±0.2ms     0.08  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
    -         763±1ms       62.1±0.4ms     0.08  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
    -       689±0.8ms       93.2±0.2ms     0.14  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
    -       687±0.9ms       94.1±0.3ms     0.14  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7492/new/

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

To: marmoute, #hg-reviewers
Cc: pulkit, martinvonz, mercurial-devel
phabricator - Jan. 17, 2020, 2:57 p.m.
This revision is now accepted and ready to land.
pulkit added a comment.
pulkit accepted this revision.


  Amending the perf numbers in commit message and fixed some spellings.

REPOSITORY
  rHG Mercurial

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7492/new/

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

To: marmoute, #hg-reviewers, pulkit
Cc: pulkit, martinvonz, mercurial-devel
phabricator - Jan. 17, 2020, 3 p.m.
marmoute added a comment.


  Thank you, much appreciated.

REPOSITORY
  rHG Mercurial

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7492/new/

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

To: marmoute, #hg-reviewers, pulkit
Cc: pulkit, martinvonz, mercurial-devel

Patch

diff --git a/tests/test-repo-filters-tiptoe.t b/tests/test-repo-filters-tiptoe.t
--- a/tests/test-repo-filters-tiptoe.t
+++ b/tests/test-repo-filters-tiptoe.t
@@ -62,7 +62,6 @@ 
 Getting status of working copy
 
   $ hg status
-  debug.filters: computing revision filter for "visible"
   M c
   A d
   R a
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -1463,6 +1463,7 @@ 
         if src not in newctx or dst in newctx or ds[dst] != b'a':
             src = None
         ds.copy(src, dst)
+    repo._quick_access_changeid_invalidate()
 
 
 def writerequires(opener, requirements):
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1514,18 +1514,51 @@ 
         narrowspec.save(self, newincludes, newexcludes)
         self.invalidate(clearfilecache=True)
 
-    @util.propertycache
+    @unfilteredpropertycache
+    def _quick_access_changeid_null(self):
+        return {
+            b'null': (nullrev, nullid),
+            nullrev: (nullrev, nullid),
+            nullid: (nullrev, nullid),
+        }
+
+    @unfilteredpropertycache
+    def _quick_access_changeid_wc(self):
+        # also fast path access to the working copy parents
+        # however, only do it for filter that ensure wc is visible.
+        quick = {}
+        cl = self.unfiltered().changelog
+        for node in self.dirstate.parents():
+            if node == nullid:
+                continue
+            rev = cl.index.get_rev(node)
+            if rev is None:
+                # unknown working copy parent case:
+                #
+                #   skip the fast path and let higher code deal with it
+                continue
+            pair = (rev, node)
+            quick[rev] = pair
+            quick[node] = pair
+        return quick
+
+    @unfilteredmethod
+    def _quick_access_changeid_invalidate(self):
+        if '_quick_access_changeid_wc' in vars(self):
+            del self.__dict__['_quick_access_changeid_wc']
+
+    @property
     def _quick_access_changeid(self):
         """an helper dictionnary for __getitem__ calls
 
         This contains a list of symbol we can recognise right away without
         further processing.
         """
-        return {
-            b'null': (nullrev, nullid),
-            nullrev: (nullrev, nullid),
-            nullid: (nullrev, nullid),
-        }
+        mapping = self._quick_access_changeid_null
+        if self.filtername in repoview.filter_has_wc:
+            mapping = mapping.copy()
+            mapping.update(self._quick_access_changeid_wc)
+        return mapping
 
     def __getitem__(self, changeid):
         # dealing with special cases
@@ -1897,6 +1930,7 @@ 
                 for f, s in sorted(self.dirstate.copies().items()):
                     if f not in pctx and s not in pctx:
                         self.dirstate.copy(None, f)
+            self._quick_access_changeid_invalidate()
 
     def filectx(self, path, changeid=None, fileid=None, changectx=None):
         """changeid must be a changeset revision, if specified.
@@ -2495,6 +2529,7 @@ 
     def invalidatevolatilesets(self):
         self.filteredrevcache.clear()
         obsolete.clearobscaches(self)
+        self._quick_access_changeid_invalidate()
 
     def invalidatedirstate(self):
         '''Invalidates the dirstate, causing the next call to dirstate
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -1957,6 +1957,7 @@ 
             for f in self.removed():
                 self._repo.dirstate.drop(f)
             self._repo.dirstate.setparents(node)
+            self._repo._quick_access_changeid_invalidate()
 
         # write changes out explicitly, because nesting wlock at
         # runtime may prevent 'wlock.release()' in 'repo.commit()'