Patchwork [8,of,8] hidden: clarfiy set operations a little

login
register
mail settings
Submitter via Mercurial-devel
Date May 28, 2017, 6:15 a.m.
Message ID <53691b5174a6c892d50d.1495952136@martinvonz.svl.corp.google.com>
Download mbox | patch
Permalink /patch/20973/
State Changes Requested
Headers show

Comments

via Mercurial-devel - May 28, 2017, 6:15 a.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1495951805 25200
#      Sat May 27 23:10:05 2017 -0700
# Node ID 53691b5174a6c892d50de7e68479cc94be5eee2c
# Parent  55c2e0b946ab4b00c8cb481b92881d4c9ccb0707
hidden: clarfiy set operations a little

Just a small refactoring that I think makes it a little clearer what
effect the anchor revs have.
Pierre-Yves David - May 28, 2017, 4:27 p.m.
On 05/28/2017 08:15 AM, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1495951805 25200
> #      Sat May 27 23:10:05 2017 -0700
> # Node ID 53691b5174a6c892d50de7e68479cc94be5eee2c
> # Parent  55c2e0b946ab4b00c8cb481b92881d4c9ccb0707
> hidden: clarfiy set operations a little

The overall direction in this series seems good. I did not had time to 
dive into the details, my main worries would be around the complexity of 
operations (it seems fine) and the number of operation on largish set we 
end up doing (they can affect the timing significantly from what I recall).

I'll try to have a detailed look at this shortly.
Pierre-Yves David - May 29, 2017, 4:50 p.m.
On 05/28/2017 08:15 AM, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1495951805 25200
> #      Sat May 27 23:10:05 2017 -0700
> # Node ID 53691b5174a6c892d50de7e68479cc94be5eee2c
> # Parent  55c2e0b946ab4b00c8cb481b92881d4c9ccb0707
> hidden: clarfiy set operations a little

I'm not too convinced this one clarify things either, so -0 with a weak 
opinion.

Overall the series looks good and the new set operation should not be 
problematic. I ran some timing on my side and I saw no impact either.

Thanks for the further cleanup

Patch

diff --git a/mercurial/repoview.py b/mercurial/repoview.py
--- a/mercurial/repoview.py
+++ b/mercurial/repoview.py
@@ -74,10 +74,10 @@ 
         mutablephases = (phases.draft, phases.secret)
         mutable = repo._phasecache.getrevset(repo, mutablephases)
 
-        visible = set(mutable - hidden)
-        visible |= (hidden & anchorrevs(repo))
+        effectiveanchors = (hidden & anchorrevs(repo))
+        visible = set(mutable - hidden) | effectiveanchors
         if visible:
-            hidden = hidden - visible
+            hidden = hidden - effectiveanchors
             _revealancestors(hidden, pfunc, visible)
     return frozenset(hidden)