Patchwork [10,of,12] candidates-groups: pass revision number to the function

login
register
mail settings
Submitter Boris Feld
Date Aug. 18, 2018, 9:27 a.m.
Message ID <e21fd074f202f62ddde2.1534584445@FB-lair>
Download mbox | patch
Permalink /patch/33875/
State Accepted
Headers show

Comments

Boris Feld - Aug. 18, 2018, 9:27 a.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1534570325 -7200
#      Sat Aug 18 07:32:05 2018 +0200
# Node ID e21fd074f202f62ddde2ea03aa5e2e8cc4073df2
# Parent  8e08a733133d84db3587e2d780acbb654ade7371
# EXP-Topic sparse-snapshot
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r e21fd074f202
candidates-groups: pass revision number to the function

The `_candidates_groups` logic works on revisions, so the nodes are currently
just converted to revs at the top of the function. It seems cleaner to instead
pass revision numbers to the function.

Having this conversion out of the way will help for further cleanups.
Gregory Szorc - Aug. 24, 2018, 5:57 p.m.
On Sat, Aug 18, 2018 at 2:27 AM Boris Feld <boris.feld@octobus.net> wrote:

> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1534570325 -7200
> #      Sat Aug 18 07:32:05 2018 +0200
> # Node ID e21fd074f202f62ddde2ea03aa5e2e8cc4073df2
> # Parent  8e08a733133d84db3587e2d780acbb654ade7371
> # EXP-Topic sparse-snapshot
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> e21fd074f202
> candidates-groups: pass revision number to the function
>

Please use modules instead of low-level feature names in commit messages.
This can be changed in flight easily enough. But since you will be sending
a revised series, it would be appreciated if you changed this.


>
> The `_candidates_groups` logic works on revisions, so the nodes are
> currently
> just converted to revs at the top of the function. It seems cleaner to
> instead
> pass revision numbers to the function.
>
> Having this conversion out of the way will help for further cleanups.
>
> diff --git a/mercurial/revlogutils/deltas.py
> b/mercurial/revlogutils/deltas.py
> --- a/mercurial/revlogutils/deltas.py
> +++ b/mercurial/revlogutils/deltas.py
> @@ -576,7 +576,6 @@ def _candidate_groups(revlog, p1, p2, ca
>      gdelta = revlog._generaldelta
>      curr = len(revlog)
>      prev = curr - 1
> -    p1r, p2r = revlog.rev(p1), revlog.rev(p2)
>
>      # should we try to build a delta?
>      if prev != nullrev and revlog.storedeltachains:
> @@ -593,7 +592,7 @@ def _candidate_groups(revlog, p1, p2, ca
>
>          if gdelta:
>              # exclude already lazy tested base if any
> -            parents = [p for p in (p1r, p2r)
> +            parents = [p for p in (p1, p2)
>                         if p != nullrev and p not in tested]
>
>              if not revlog._deltabothparents and len(parents) == 2:
> @@ -736,7 +735,8 @@ class deltacomputer(object):
>
>          deltainfo = None
>          deltas_limit = revinfo.textlen * LIMIT_DELTA2TEXT
> -        candidate_groups = _candidate_groups(self.revlog, p1, p2,
> cachedelta)
> +        p1r, p2r = revlog.rev(p1), revlog.rev(p2)
> +        candidate_groups = _candidate_groups(self.revlog, p1r, p2r,
> cachedelta)
>          for candidaterevs in candidate_groups:
>              # filter out delta base that will never produce good delta
>              candidaterevs = [r for r in candidaterevs
>

Patch

diff --git a/mercurial/revlogutils/deltas.py b/mercurial/revlogutils/deltas.py
--- a/mercurial/revlogutils/deltas.py
+++ b/mercurial/revlogutils/deltas.py
@@ -576,7 +576,6 @@  def _candidate_groups(revlog, p1, p2, ca
     gdelta = revlog._generaldelta
     curr = len(revlog)
     prev = curr - 1
-    p1r, p2r = revlog.rev(p1), revlog.rev(p2)
 
     # should we try to build a delta?
     if prev != nullrev and revlog.storedeltachains:
@@ -593,7 +592,7 @@  def _candidate_groups(revlog, p1, p2, ca
 
         if gdelta:
             # exclude already lazy tested base if any
-            parents = [p for p in (p1r, p2r)
+            parents = [p for p in (p1, p2)
                        if p != nullrev and p not in tested]
 
             if not revlog._deltabothparents and len(parents) == 2:
@@ -736,7 +735,8 @@  class deltacomputer(object):
 
         deltainfo = None
         deltas_limit = revinfo.textlen * LIMIT_DELTA2TEXT
-        candidate_groups = _candidate_groups(self.revlog, p1, p2, cachedelta)
+        p1r, p2r = revlog.rev(p1), revlog.rev(p2)
+        candidate_groups = _candidate_groups(self.revlog, p1r, p2r, cachedelta)
         for candidaterevs in candidate_groups:
             # filter out delta base that will never produce good delta
             candidaterevs = [r for r in candidaterevs