D6207: branchcache: add functions to validate changelog nodes
yuja added a comment. > 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. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6207 To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D6207: branchcache: add functions to validate changelog nodes
> 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. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6207: branchcache: add functions to validate changelog nodes
pulkit added a comment. In https://phab.mercurial-scm.org/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 https://phab.mercurial-scm.org/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 (https://phab.mercurial-scm.org/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? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6207 To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6207: branchcache: add functions to validate changelog nodes
This revision was automatically updated to reflect the committed changes. Closed by commit rHG2f8147521e59: branchcache: add functions to validate changelog nodes (authored by pulkit, committed by ). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D6207?vs=14676=14750 REVISION DETAIL https://phab.mercurial-scm.org/D6207 AFFECTED FILES mercurial/branchmap.py CHANGE DETAILS diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py --- a/mercurial/branchmap.py +++ b/mercurial/branchmap.py @@ -126,6 +126,10 @@ def clear(self): 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): """A dict like object that hold branches heads cache. @@ -173,6 +177,32 @@ if self._hasnode is None: 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): return iter(self._entries) To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6207: branchcache: add functions to validate changelog nodes
yuja added a comment. 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 https://phab.mercurial-scm.org/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. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6207 To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D6207: branchcache: add functions to validate changelog nodes
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. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D6207: branchcache: add functions to validate changelog nodes
pulkit created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION 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. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6207 AFFECTED FILES mercurial/branchmap.py CHANGE DETAILS diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py --- a/mercurial/branchmap.py +++ b/mercurial/branchmap.py @@ -126,6 +126,10 @@ def clear(self): 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): """A dict like object that hold branches heads cache. @@ -173,6 +177,32 @@ if self._hasnode is None: 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): return iter(self._entries) To: pulkit, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
