[PATCH 1 of 2] revset: remove existence check from min() and max()
Durham Goode
durham at fb.com
Mon Sep 21 15:27:24 CDT 2015
On 9/21/15 11:49 AM, Pierre-Yves David wrote:
>
>
> 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?
The python max() throws ValueError "arg is an empty sequence". I can
check for that string maybe, but would that change based on localization?
More information about the Mercurial-devel
mailing list