| Submitter | Jun Wu |
|---|---|
| Date | Nov. 1, 2016, 8:51 a.m. |
| Message ID | <f6c6598addaa1854f855.1477990265@x1c> |
| Download | mbox | patch |
| Permalink | /patch/17234/ |
| State | Changes Requested |
| Headers | show |
Comments
On 11/01/2016 09:51 AM, Jun Wu wrote: > # HG changeset patch > # User Jun Wu <quark@fb.com> > # Date 1477885221 0 > # Mon Oct 31 03:40:21 2016 +0000 > # Node ID f6c6598addaa1854f8556bacb732347256964e3e > # Parent eead0ed124cbc59b9bb5183035aa481af3547677 > # Available At https://bitbucket.org/quark-zju/hg-draft > # hg pull https://bitbucket.org/quark-zju/hg-draft -r f6c6598addaa > adjustlinkrev: remove the inclusive parameter > > The parameter is only set to False when srcrev=fctx._descendantrev, in which > case inclusive could also be True with little performance impact. linkrev works in two steps: 1) find a revision that touch that file 2) check if the file version is the one we are looking for If we drop inclusive here we'll alway have an initial match for (1) leading to a false check for (2). (2) is not cheap and we would rather avoid that. > Drop this parameter to make the interface cleaner. > > diff --git a/mercurial/context.py b/mercurial/context.py > --- a/mercurial/context.py > +++ b/mercurial/context.py > @@ -809,5 +809,5 @@ class basefilectx(object): > return True > > - def _adjustlinkrev(self, path, filelog, fnode, srcrev, inclusive=False): > + def _adjustlinkrev(self, path, filelog, fnode, srcrev): > """return the first ancestor of <srcrev> introducing <fnode> > > @@ -821,5 +821,4 @@ class basefilectx(object): > :filelog: the filelog of this path > :srcrev: the changeset revision we search ancestors from > - :inclusive: if true, the src revision will also be checked > """ > repo = self._repo > @@ -834,5 +833,4 @@ class basefilectx(object): > # wctx case, used by workingfilectx during mergecopy > revs = [p.rev() for p in self._repo[None].parents()] > - inclusive = True # we skipped the real (revless) source > else: > revs = [srcrev] > @@ -846,5 +844,5 @@ class basefilectx(object): > return lkr > # fallback to walk through the changelog > - iteranc = cl.ancestors(revs, lkr, inclusive=inclusive) > + iteranc = cl.ancestors(revs, lkr, inclusive=True) > for a in iteranc: > ac = cl.read(a) # get changeset data (we avoid object creation) > @@ -875,5 +873,5 @@ class basefilectx(object): > return self.linkrev() > return self._adjustlinkrev(self._path, self._filelog, self._filenode, > - self.rev(), inclusive=True) > + self.rev()) > > def _parentfilectx(self, path, fileid, filelog): > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel >
Excerpts from Pierre-Yves David's message of 2016-11-02 00:34:19 +0100: > > On 11/01/2016 09:51 AM, Jun Wu wrote: > > # HG changeset patch > > # User Jun Wu <quark@fb.com> > > # Date 1477885221 0 > > # Mon Oct 31 03:40:21 2016 +0000 > > # Node ID f6c6598addaa1854f8556bacb732347256964e3e > > # Parent eead0ed124cbc59b9bb5183035aa481af3547677 > > # Available At https://bitbucket.org/quark-zju/hg-draft > > # hg pull https://bitbucket.org/quark-zju/hg-draft -r f6c6598addaa > > adjustlinkrev: remove the inclusive parameter > > > > The parameter is only set to False when srcrev=fctx._descendantrev, in which > > case inclusive could also be True with little performance impact. > > linkrev works in two steps: > > 1) find a revision that touch that file > 2) check if the file version is the one we are looking for > > If we drop inclusive here we'll alway have an initial match for (1) > leading to a false check for (2). (2) is not cheap and we would rather > avoid that. Good catch. I was focused on changelog.i walk but didn't think too much about the changelog.d walk. Feel free to drop this one while I'm thinking about other solutions. > > > Drop this parameter to make the interface cleaner. > > > > diff --git a/mercurial/context.py b/mercurial/context.py > > --- a/mercurial/context.py > > +++ b/mercurial/context.py > > @@ -809,5 +809,5 @@ class basefilectx(object): > > return True > > > > - def _adjustlinkrev(self, path, filelog, fnode, srcrev, inclusive=False): > > + def _adjustlinkrev(self, path, filelog, fnode, srcrev): > > """return the first ancestor of <srcrev> introducing <fnode> > > > > @@ -821,5 +821,4 @@ class basefilectx(object): > > :filelog: the filelog of this path > > :srcrev: the changeset revision we search ancestors from > > - :inclusive: if true, the src revision will also be checked > > """ > > repo = self._repo > > @@ -834,5 +833,4 @@ class basefilectx(object): > > # wctx case, used by workingfilectx during mergecopy > > revs = [p.rev() for p in self._repo[None].parents()] > > - inclusive = True # we skipped the real (revless) source > > else: > > revs = [srcrev] > > @@ -846,5 +844,5 @@ class basefilectx(object): > > return lkr > > # fallback to walk through the changelog > > - iteranc = cl.ancestors(revs, lkr, inclusive=inclusive) > > + iteranc = cl.ancestors(revs, lkr, inclusive=True) > > for a in iteranc: > > ac = cl.read(a) # get changeset data (we avoid object creation) > > @@ -875,5 +873,5 @@ class basefilectx(object): > > return self.linkrev() > > return self._adjustlinkrev(self._path, self._filelog, self._filenode, > > - self.rev(), inclusive=True) > > + self.rev()) > > > > def _parentfilectx(self, path, fileid, filelog): > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > > >
Patch
diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -809,5 +809,5 @@ class basefilectx(object): return True - def _adjustlinkrev(self, path, filelog, fnode, srcrev, inclusive=False): + def _adjustlinkrev(self, path, filelog, fnode, srcrev): """return the first ancestor of <srcrev> introducing <fnode> @@ -821,5 +821,4 @@ class basefilectx(object): :filelog: the filelog of this path :srcrev: the changeset revision we search ancestors from - :inclusive: if true, the src revision will also be checked """ repo = self._repo @@ -834,5 +833,4 @@ class basefilectx(object): # wctx case, used by workingfilectx during mergecopy revs = [p.rev() for p in self._repo[None].parents()] - inclusive = True # we skipped the real (revless) source else: revs = [srcrev] @@ -846,5 +844,5 @@ class basefilectx(object): return lkr # fallback to walk through the changelog - iteranc = cl.ancestors(revs, lkr, inclusive=inclusive) + iteranc = cl.ancestors(revs, lkr, inclusive=True) for a in iteranc: ac = cl.read(a) # get changeset data (we avoid object creation) @@ -875,5 +873,5 @@ class basefilectx(object): return self.linkrev() return self._adjustlinkrev(self._path, self._filelog, self._filenode, - self.rev(), inclusive=True) + self.rev()) def _parentfilectx(self, path, fileid, filelog):