This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6207

branchcache: add functions to validate changelog nodes
ClosedPublic

Authored by pulkit on Apr 5 2019, 9:31 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG2f8147521e59: branchcache: add functions to validate changelog nodes
Summary

This patch adds functions to validate closed nodes, validate nodes for a certain
branch and for all the branches. These functions will be used in upcoming
patches.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.Apr 5 2019, 9:31 AM
yuja added a subscriber: yuja.Apr 15 2019, 7:16 PM

Queued, thanks.

+ def _verifybranch(self, branch):
+ """ verify head nodes for the given branch. If branch is None, verify
+ for all the branches """

"If branch is None, ..." appears wrong.

+ if branch not in self._entries or branch in self._verifiedbranches:
+ return
+ for n in self._entries[branch]:
+ if not self._hasnode(n):
+ _unknownnode(n)
+
+ self._verifiedbranches.add(branch)

Regarding D6236, _verifiedbranches could be inverted (i.e. a set of branches
to be verified) so that _verifyall() can return early. I don't know which
will be faster, but in principle, fewer loops and Python gives a better result.

Closed by commit rHG2f8147521e59: branchcache: add functions to validate changelog nodes (authored by pulkit). · Explain WhyApr 15 2019, 7:41 PM
This revision was automatically updated to reflect the committed changes.
pulkit added a comment.Apr 16 2019, 8:35 AM
In D6207#90778, @yuja wrote:

Queued, thanks.

+ def _verifybranch(self, branch):
+ """ verify head nodes for the given branch. If branch is None, verify
+ for all the branches """

"If branch is None, ..." appears wrong.

+ if branch not in self._entries or branch in self._verifiedbranches:
+ return
+ for n in self._entries[branch]:
+ if not self._hasnode(n):
+ _unknownnode(n)
+
+ self._verifiedbranches.add(branch)

Regarding D6236, _verifiedbranches could be inverted (i.e. a set of branches
to be verified) so that _verifyall() can return early. I don't know which
will be faster, but in principle, fewer loops and Python gives a better result.

In _verifyall(), I changed code to calculate a needverification set and then iterate over it.

The motivation behind iteritems() change (D6236) is optimizing https://www.mercurial-scm.org/repo/hg-committed/file/70b71421fd33/mercurial/commands.py#l1128.

Maybe we can iterate over verified nodes first, and then iterate over unverified nodes?

yuja added a comment.Apr 16 2019, 7:08 PM
The motivation behind iteritems() change (https://phab.mercurial-scm.org/D6236) is optimizing https://www.mercurial-scm.org/repo/hg-committed/file/70b71421fd33/mercurial/commands.py#l1128.

Okay, got that, thanks.

Maybe we can iterate over verified nodes first, and then iterate over unverified nodes?

Maybe no. I don't think it's more likely for callers to get any verified node
and stop the iteration.

Revision Contents
Changeset List

PathPackages
Mmercurial/branchmap.py (30 lines)

Diff 14676

mercurial/branchmap.py

if cache.validfor(rview): if cache.validfor(rview):
self._per_filter[candidate] = cache self._per_filter[candidate] = cache
cache.write(rview) cache.write(rview)
return return
def clear(self): def clear(self):
self._per_filter.clear() self._per_filter.clear()
def _unknownnode(node):
""" raises ValueError when branchcache found a node which does not exists
"""
raise ValueError(r'node %s does not exist' % pycompat.sysstr(hex(node)))
class branchcache(object): class branchcache(object):
"""A dict like object that hold branches heads cache. """A dict like object that hold branches heads cache.
This cache is used to avoid costly computations to determine all the This cache is used to avoid costly computations to determine all the
branch heads of a repo. branch heads of a repo.
The cache is serialized on disk in the following format: The cache is serialized on disk in the following format:
# whether closed nodes are verified or not # whether closed nodes are verified or not
self._closedverified = False self._closedverified = False
# branches for which nodes are verified # branches for which nodes are verified
self._verifiedbranches = set() self._verifiedbranches = set()
self._hasnode = hasnode self._hasnode = hasnode
if self._hasnode is None: if self._hasnode is None:
self._hasnode = lambda x: True self._hasnode = lambda x: True
def _verifyclosed(self):
""" verify the closed nodes we have """
if self._closedverified:
return
for node in self._closednodes:
if not self._hasnode(node):
_unknownnode(node)
self._closedverified = True
def _verifybranch(self, branch):
""" verify head nodes for the given branch. If branch is None, verify
for all the branches """
if branch not in self._entries or branch in self._verifiedbranches:
return
for n in self._entries[branch]:
if not self._hasnode(n):
_unknownnode(n)
self._verifiedbranches.add(branch)
def _verifyall(self):
""" verifies nodes of all the branches """
for b in self._entries:
self._verifybranch(b)
def __iter__(self): def __iter__(self):
return iter(self._entries) return iter(self._entries)
def __setitem__(self, key, value): def __setitem__(self, key, value):
self._entries[key] = value self._entries[key] = value
def __getitem__(self, key): def __getitem__(self, key):
return self._entries[key] return self._entries[key]