[PATCH 2 of 6 V3] hidden: use _domainancestors to compute revs revealed by dynamic blocker

[Pierre-Yves David]

On 05/26/2017 10:43 PM, Martin von Zweigbergk wrote:
> +mercurial-devel again (I clicked just "Reply" by mistake yet again)
>
> On Fri, May 26, 2017 at 1:19 PM, Martin von Zweigbergk
> <martinvonz at google.com> wrote:
>> On Fri, May 26, 2017 at 11:34 AM, Pierre-Yves David
>> <pierre-yves.david at ens-lyon.org> wrote:
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david at octobus.net>
>>> # Date 1495373721 -7200
>>> #      Sun May 21 15:35:21 2017 +0200
>>> # Node ID c95d2c1ff6128ab5580d8e28ef8632ae87ecccc5
>>> # Parent  95b7279b3b2ec46607dbc22ebe4e8a27381c5ace
>>> # EXP-Topic fast-compute-hidden
>>> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>>> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r c95d2c1ff612
>>> hidden: use _domainancestors to compute revs revealed by dynamic blocker
>>>
>>> The complexity of computing the revealed changesets is now 'O(revealed)'.
>>> This massively speeds up the computation on large repository. Moving it to the
>>> millisecond range.
>>>
>>> Below are timing from two Mozilla repositories with different contents:
>>>
>>> 1) mozilla repository with:
>>>  * 400667 changesets
>>>  * 35 hidden changesets (first rev-268334)
>>>  * 288 visible drafts
>>>  * obsolete working copy (dynamicblockers),
>>>
>>> Before:
>>> ! visible
>>> ! wall 0.030247 comb 0.030000 user 0.030000 sys 0.000000 (best of 100)
>>>
>>> After:
>>> ! visible
>>> ! wall 0.000585 comb 0.000000 user 0.000000 sys 0.000000 (best of 4221)
>>>
>>> The timing above include the computation of obsolete changeset:
>>> ! obsolete
>>> ! wall 0.000396 comb 0.000000 user 0.000000 sys 0.000000 (best of 6816)
>>>
>>> So adjusted time give 30ms before versus 0.2ms after. A 150x speedup.
>>>
>>> 2) mozilla repository with:
>>>  * 405645 changesets
>>>  * 4312 hidden changesets (first rev-326004)
>>>  * 264 visible drafts
>>>  * obsolete working copy (dynamicblockers),
>>>
>>> Before:
>>> ! visible
>>> ! wall 0.168658 comb 0.170000 user 0.170000 sys 0.000000 (best of 48)
>>>
>>> After
>>> ! visible
>>> ! wall 0.008612 comb 0.010000 user 0.010000 sys 0.000000 (best of 325)
>>>
>>> The timing above include the computation of obsolete changeset:
>>> ! obsolete
>>> ! wall 0.006408 comb 0.010000 user 0.010000 sys 0.000000 (best of 404)
>>>
>>> So adjusted time give 160ms before versus 2ms after. A 75x speedup.
>>
>> I've said before that I'm more interested in effect on real-life
>> commands (which is not to say that everyone has to agree and share
>> timings for real-life commands). 160ms is a pretty good win, but not
>> so relevant if the command still takes 10s. Roughly how long does a
>> "hg log -r ." (?) take with the setup above?

The hidden code is run for every single command since it is on the path 
to retrieve a "visible" filtered changelog. Any win here is a win for 
all commands. (and the win is is asymptotic).

Example timing below.

Setup (1)
   hg id -r .
     before: 0.205 total
     after:  0.165 total

     full series combined with obscache: 0.103 total

Setup (2)
   hg id -r .
     before: 0.328 total
     after:  0.192 total

     full series combined with obscache: 0.098 total

This works in the same area as my obscache series: 
https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-May/098136.html

>>> diff --git a/mercurial/repoview.py b/mercurial/repoview.py
>>> --- a/mercurial/repoview.py
>>> +++ b/mercurial/repoview.py
>>> @@ -224,8 +224,10 @@ def computehidden(repo):
>>>          # changesets and remove those.
>>>          dynamic = hidden & revealedrevs(repo)
>>>          if dynamic:
>>> -            blocked = cl.ancestors(dynamic, inclusive=True)
>>> -            hidden = frozenset(r for r in hidden if r not in blocked)
>>> +            pfunc = cl.parentrevs
>>> +            mutablephases = (phases.draft, phases.secret)
>>> +            mutable = repo._phasecache.getrevset(repo, mutablephases)
>>> +            hidden = hidden - _domainancestors(pfunc, dynamic, mutable)
>>
>> It looks like we're adding more code that assumes public commits can
>> not be hidden. That seems fine to me, but I wanted to mention it to
>> see if anyone else has objections.

The old code already assume this[1]. Before this series we have two 
functions (one assuming non-public) and one on-disk caching layer. After 
this series we have one (simpler) function (assuming non-public). So we 
do not adds new assumption.

(note: the new code have simpler code that would make it simpler to 
change that assumption (by extending the "domain" set))

[1] https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/repoview.py#l68

Cheers,

-- 
Pierre-Yves David