Patchwork D6156: branchcache: add attributes to track which nodes are verified

login
register
mail settings
Submitter phabricator
Date March 19, 2019, 1:40 p.m.
Message ID <differential-rev-PHID-DREV-2mck4jba564xzfubx4vj-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39333/
State Superseded
Headers show

Comments

phabricator - March 19, 2019, 1:40 p.m.
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Half of the cost of loading branchcache comes from verifiying all the nodes it
  has. We don't need to verify all the nodes in all the cases. Sometimes we need
  to verify only a set of nodes for a set of branches. We can ignore nodes of
  other branches as we are not going to read them.
  
  This patch introduces two attributes to branchcache class. _verifiedbranches is
  a set which will tell the branches for which it's head nodes are verified.
  _closedverified is a boolean which will tell whether all closednodes are
  verified or not.
  
  Another idea which I had was to keep a set of nodes which are verified, but it
  will be more ugly and difficult to track things on node level.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/branchmap.py

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - March 24, 2019, 1:25 a.m.
> @@ -231,8 +235,10 @@
>                  raise ValueError(
>                      r'node %s does not exist' % pycompat.sysstr(hex(node)))
>              self._entries.setdefault(label, []).append(node)
> +            self._verifiedbranches.add(label)

Can we expect that we'll rewrite this `load()` logic to handle verification
state of multiple nodes (i.e. heads) correctly? It's unclear to me how lazy
verification will be implemented.
phabricator - March 24, 2019, 1:26 a.m.
yuja added a comment.


  > @@ -231,8 +235,10 @@
  > 
  >       raise ValueError(
  >           r'node %s does not exist' % pycompat.sysstr(hex(node)))
  >   self._entries.setdefault(label, []).append(node)
  > 
  > +            self._verifiedbranches.add(label)
  
  Can we expect that we'll rewrite this `load()` logic to handle verification
  state of multiple nodes (i.e. heads) correctly? It's unclear to me how lazy
  verification will be implemented.

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
--- a/mercurial/branchmap.py
+++ b/mercurial/branchmap.py
@@ -162,6 +162,10 @@ 
         else:
             self._closednodes = closednodes
         self._entries = dict(entries)
+        # whether closed nodes are verified or not
+        self._closedverified = False
+        # branches for which nodes are verified
+        self._verifiedbranches = set()
 
     def __iter__(self):
         return iter(self._entries)
@@ -231,8 +235,10 @@ 
                 raise ValueError(
                     r'node %s does not exist' % pycompat.sysstr(hex(node)))
             self._entries.setdefault(label, []).append(node)
+            self._verifiedbranches.add(label)
             if state == 'c':
                 self._closednodes.add(node)
+        self._closedverified = True
 
     @staticmethod
     def _filename(repo):