[PATCH 3 of 3 STABLE V2] changectx: do not include hidden revisions on short node lookup (issue4964)

Jun Wu quark at fb.com
Tue Nov 1 15:56:45 UTC 2016


I think the final solution to teach the C radix tree code about what are
hidden. I will take a look some time in the future.

Excerpts from Yuya Nishihara's message of 2016-10-28 21:48:10 +0900:
> On Fri, 28 Oct 2016 10:31:40 +0200, Pierre-Yves David wrote:
> > On 10/26/2016 03:17 PM, Yuya Nishihara wrote:
> > > On Tue, 25 Oct 2016 23:17:14 +0900, Yuya Nishihara wrote:
> > >> # HG changeset patch
> > >> # User Yuya Nishihara <yuya at tcha.org>
> > >> # Date 1477199774 -32400
> > >> #      Sun Oct 23 14:16:14 2016 +0900
> > >> # Branch stable
> > >> # Node ID 242b7a856495179795ee5662f298029c4b492563
> > >> # Parent  ecbce2fe4dea116c925a2fecd1b7b50df0a62589
> > >> changectx: do not include hidden revisions on short node lookup (issue4964)
> > >>
> > >> It was changed at dc25ed84bee8, but which seems wrong since we filtered out
> > >> hidden nodes by _partialmatch() before that change. This patch makes
> > >> changectx() be consistent with the filtered changelog, and detect a hidden
> > >> short node only if it has no unique match in the filtered changelog.
> > >>
> > >> Though we've made shortest(node) use unfiltered changelog, I think this is
> > >> a separate issue worth fixing.
> > >>
> > >> diff --git a/mercurial/context.py b/mercurial/context.py
> > >> --- a/mercurial/context.py
> > >> +++ b/mercurial/context.py
> > >> @@ -481,11 +481,16 @@ class changectx(basectx):
> > >>              except error.RepoLookupError:
> > >>                  pass
> > >>
> > >> -            self._node = repo.unfiltered().changelog._partialmatch(changeid)
> > >> +            self._node = repo.changelog._partialmatch(changeid)
> > >>              if self._node is not None:
> > >>                  self._rev = repo.changelog.rev(self._node)
> > >>                  return
> > >>
> > >> +            # lookup hidden node to provide a better error indication
> > >> +            n = repo.unfiltered().changelog._partialmatch(changeid)
> > >> +            if n is not None:
> > >> +                repo.changelog.rev(n)  # must raise FilteredLookupError
> > >
> > > Please feel free to drop this, the last patch, if the performance is important.
> > 
> > The behavior update seems good, however, you are mentionning performance
> > impact but it is unclear of the extend of it. Can you mention any number?
> 
> I haven't measure it, but the number would be similar to the patch 1. If an
> ambiguous identifier is specified, and if hidden revisions exist, new code
> would be about 10-100x (a few milliseconds) slower.
> 
> The question is whether we should care the cost needed only when an ambiguous
> identifier is given.
> 
> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089657.html 


More information about the Mercurial-devel mailing list