[PATCH 6 of 8] hidden: remove _consistencyblockers()

[Pierre-Yves David]

On 05/29/2017 07:07 PM, Martin von Zweigbergk wrote:
>
>
> On May 29, 2017 9:55 AM, "Martin von Zweigbergk" <martinvonz at google.com
> <mailto:martinvonz at google.com>> wrote:
>
>
>
>     On May 29, 2017 9:50 AM, "Pierre-Yves David"
>     <pierre-yves.david at ens-lyon.org
>     <mailto:pierre-yves.david at ens-lyon.org>> wrote:
>
>
>
>         On 05/28/2017 08:15 AM, Martin von Zweigbergk wrote:
>
>             # HG changeset patch
>             # User Martin von Zweigbergk <martinvonz at google.com
>             <mailto:martinvonz at google.com>>
>             # Date 1495950919 25200
>             #      Sat May 27 22:55:19 2017 -0700
>             # Node ID f8b3d943ff21a2b906f2806476a637aac93763c4
>             # Parent  8b91ef8f7f051165c098f3ebfc871e90afada070
>             hidden: remove  _consistencyblockers()
>
>             Roughly speaking, we currently do this to reveal hidden
>             ancestors of
>             visible revisions:
>
>              1. Iterate over all visible non-public revisions and see if
>             they have
>                 hidden parents
>
>              2. For each revision found in step (1) walk the chain of hidden
>                 commits and reveal it
>
>             We can simplify that by skipping step (1) and doing step (2)
>             from all
>             visible non-public revisions instead.
>
>             This doesn't seem to have much impact on "perfvolatilesets".
>
>             Before:
>             ! obsolete
>             ! wall 0.004616 comb 0.000000 user 0.000000 sys 0.000000
>             (best of 570)
>             ! visible
>             ! wall 0.008235 comb 0.010000 user 0.010000 sys 0.000000
>             (best of 326)
>
>             After:
>             ! obsolete
>             ! wall 0.004727 comb 0.010000 user 0.010000 sys 0.000000
>             (best of 543)
>             ! visible
>             ! wall 0.008371 comb 0.000000 user 0.000000 sys 0.000000
>             (best of 324)
>
>             diff --git a/mercurial/repoview.py b/mercurial/repoview.py
>             --- a/mercurial/repoview.py
>             +++ b/mercurial/repoview.py
>             @@ -44,20 +44,6 @@
>                      anchors.update(rev(t[0]) for t in tags.values() if
>             t[0] in nodemap)
>                  return anchors
>
>             -def _consistencyblocker(pfunc, hideable, revs):
>             -    """return non-hideable changeset blocking hideable one
>             -
>             -    For consistency, we cannot actually hide a changeset if
>             one of it children
>             -    are visible, this function find such children.
>             -    """
>             -    blockers = set()
>             -    for r in revs:
>             -        for p in pfunc(r):
>             -            if p != nullrev and p in hideable:
>             -                blockers.add(r)
>             -                break
>             -    return blockers
>             -
>              def _revealancestors(hidden, pfunc, revs):
>                  """reveals contiguous chains of hidden ancestors of
>             'revs' by removing them
>                  from 'hidden'
>             @@ -89,16 +75,12 @@
>                      mutablephases = (phases.draft, phases.secret)
>                      mutable = repo._phasecache.getrevset(repo,
>             mutablephases)
>
>             -        visible = mutable - hidden
>             -        blockers = _consistencyblocker(pfunc, hidden, visible)
>             -
>             -        # check if we have wd parents, bookmarks or tags
>             pointing to hidden
>             -        # changesets and remove those.
>             -        blockers |= (hidden & anchorrevs(repo))
>             -        if blockers:
>             +        visible = set(mutable - hidden)
>
>
>         I've not sure why we create a set from the operation result.
>         That looks like a wasteful double allocation. Am I missing
>         something?
>
>
>     as A
>
>
>         --
>         Pierre-Yves David
>
>     Ll pop p lol p ok I'll l up lm pop pop pop lol lpos lol p lol
>     opportunity to pop lolz
>
>
> What my pocket was trying to say :-) is that some operation (|= maybe)
> was not supported on the filteredset. Maybe there's a better way?\

Ho, this is not a set‽ (that is what I've been missing).

Hum, we should probably stick to pure python set for the return of 
phasecase here and only upgrade them to smartset in revset.

The hidding/obsolete logic are not using these set lazily so having a 
smart is likely to just add some overhead.

Cheers

-- 
Pierre-Yves David