Page MenuHomePhabricator

branchcache: add functions to validate changelog nodes
ClosedPublic

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

Details

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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.

This revision was automatically updated to reflect the committed changes.
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.