[PATCH 1 of 2] revset: remove existence check from min() and max()

[Pierre-Yves David]

On 09/20/2015 08:51 PM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham at fb.com>
> # Date 1442802473 25200
> #      Sun Sep 20 19:27:53 2015 -0700
> # Node ID 9deecd57915da9ed217abafb81cf8cba506b2849
> # Parent  a0eff7ebc2aebb32cf4c8da276104102ff37d786
> revset: remove existence check from min() and max()
>
> min() and max() would first do an existence check. Unfortunately existence
> checks can be slow in certain situations (like if the smartset is a list, and
> quickly iterable in both ascending and descending directions, then doing an
> existence check will start from the bottom, even if you want to check the
> max()).
>
> The fix is to not do the check, and just handle the error if it happens. In a
> large repo, this speeds up:
>
> hg log -r 'max(parents(. + .^) - (. + .^)  & ::master)'
>
> from 3.5s to 0.85s. That revset is contrived and just for testing. In our
> real case we used 'bundle()' in place of '. + .^'
>
> Interesting perf numbers for the revset benchmarks:
>
> max(draft() and ::tip) =>  0.027s to 0.0005s
> max(author(lmoscovicz)) => 2.48s to 0.57s
>
> min doesn't show any perf changes, but changing it as well will prevent a perf
> regression in my next patch.
>
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -1277,10 +1277,13 @@ def maxrev(repo, subset, x):
>       Changeset with highest revision number in set.
>       """
>       os = getset(repo, fullreposet(repo), x)
> -    if os:
> +    try:
>           m = os.max()
>           if m in subset:
>               return baseset([m])
> +    except ValueError:
> +        # os was an empty set
> +        pass
>       return baseset()
>
>   def merge(repo, subset, x):
> @@ -1316,10 +1319,13 @@ def minrev(repo, subset, x):
>       Changeset with lowest revision number in set.
>       """
>       os = getset(repo, fullreposet(repo), x)
> -    if os:
> +    try:
>           m = os.min()
>           if m in subset:
>               return baseset([m])
> +    except ValueError:

The Value error catching scares me a bit as too wide. Could we find a 
way to narrow that?

-- 
Pierre-Yves David