D1620: revset: use phasecache.getrevset to calculate public()

[quark (Jun Wu)]

quark created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Other revsets like secret(), draft(), _nonpublic() are using
  phasescache.getrevset already. The latter is more efficient after https://phab.mercurial-scm.org/D1606.
  So let's migrate the public() revset function too.
  
  Tested using:
  
    $ hg debugshell --hidden --cwd hg-committed`
    In [1]: %timeit len(repo.revs('public()'))
  
  - Before https://phab.mercurial-scm.org/D1606: 10 loops, best of 3: 22.5 ms per loop
  - Before this change, after https://phab.mercurial-scm.org/D1606: 10 loops, best of 3: 28.6 ms per loop
  - After this change: 10 loops, best of 3: 20.2 ms per loop
  
  Therefore `public()` revset becomes even slightly faster after the data
  structure change by https://phab.mercurial-scm.org/D1606. A similar performance win could also be observed
  on a large repo.
  
  A side effect is `phasecache.getrevset` needs to take a `subset` parameter.
  That was added with a default value so it won't cause BC issues.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1620

AFFECTED FILES
  mercurial/phases.py
  mercurial/revset.py

CHANGE DETAILS

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1504,8 +1504,7 @@
 
 def _phase(repo, subset, *targets):
     """helper to select all rev in <targets> phases"""
-    s = repo._phasecache.getrevset(repo, targets)
-    return subset & s
+    return repo._phasecache.getrevset(repo, targets, subset)
 
 @predicate('draft()', safe=True)
 def draft(repo, subset, x):
@@ -1612,11 +1611,7 @@
     """Changeset in public phase."""
     # i18n: "public" is a keyword
     getargs(x, 0, 0, _("public takes no arguments"))
-    phase = repo._phasecache.phase
-    target = phases.public
-    condition = lambda r: phase(repo, r) == target
-    return subset.filter(condition, condrepr=('<phase %r>', target),
-                         cache=False)
+    return _phase(repo, subset, phases.public)
 
 @predicate('remote([id [,path]])', safe=False)
 def remote(repo, subset, x):
diff --git a/mercurial/phases.py b/mercurial/phases.py
--- a/mercurial/phases.py
+++ b/mercurial/phases.py
@@ -208,7 +208,7 @@
             self.filterunknown(repo)
             self.opener = repo.svfs
 
-    def getrevset(self, repo, phases):
+    def getrevset(self, repo, phases, subset=None):
         """return a smartset for the given phases"""
         self.loadphaserevs(repo) # ensure phase's sets are loaded
         phases = set(phases)
@@ -222,7 +222,10 @@
                 revs = set.union(*[self._phasesets[p] for p in phases])
             if repo.changelog.filteredrevs:
                 revs = revs - repo.changelog.filteredrevs
-            return smartset.baseset(revs)
+            if subset is None:
+                return smartset.baseset(revs)
+            else:
+                return subset & smartset.baseset(revs)
         else:
             phases = set(allphases).difference(phases)
             if not phases:
@@ -232,9 +235,11 @@
                 revs = self._phasesets[p]
             else:
                 revs = set.union(*[self._phasesets[p] for p in phases])
+            if subset is None:
+                subset = smartset.fullreposet(repo)
             if not revs:
-                return smartset.fullreposet(repo)
-            return smartset.fullreposet(repo).filter(lambda r: r not in revs)
+                return subset
+            return subset.filter(lambda r: r not in revs)
 
     def copy(self):
         # Shallow copy meant to ensure isolation in



To: quark, #hg-reviewers
Cc: mercurial-devel