Patchwork [6,of,6] hidden: alert hidden store when obsmarkers are created

login
register
mail settings
Submitter Durham Goode
Date May 18, 2017, 6:24 p.m.
Message ID <b09fa310c34bdbad4fc2.1495131840@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/20680/
State Deferred
Headers show

Comments

Durham Goode - May 18, 2017, 6:24 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1495129620 25200
#      Thu May 18 10:47:00 2017 -0700
# Node ID b09fa310c34bdbad4fc2b5b914e84e0fc9dc4b68
# Parent  0979ce9d40803103441b6df1ebefafe80bb9a057
hidden: alert hidden store when obsmarkers are created

This calls the new updatevisibility function from obsolete.createmarkers(). This
let's us update the currently stored hidden information when markers change.
Durham Goode - May 18, 2017, 6:34 p.m.
On 5/18/17 11:24 AM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1495129620 25200
> #      Thu May 18 10:47:00 2017 -0700
> # Node ID b09fa310c34bdbad4fc2b5b914e84e0fc9dc4b68
> # Parent  0979ce9d40803103441b6df1ebefafe80bb9a057
> hidden: alert hidden store when obsmarkers are created
>
> This calls the new updatevisibility function from obsolete.createmarkers(). This
> let's us update the currently stored hidden information when markers change.
>
> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
> --- a/mercurial/obsolete.py
> +++ b/mercurial/obsolete.py
> @@ -1226,6 +1226,7 @@ def createmarkers(repo, relations, flag=
>          metadata['user'] = repo.ui.username()
>      tr = repo.transaction('add-obsolescence-marker')
>      try:
> +        affectednodes = set()
>          markerargs = []
>          for rel in relations:
>              prec = rel[0]
> @@ -1258,6 +1259,11 @@ def createmarkers(repo, relations, flag=
>              repo.obsstore.create(tr, nprec, nsucs, flag, parents=npare,
>                                   date=date, metadata=localmetadata)
>              repo.filteredrevcache.clear()
> +            affectednodes.add(nprec)
> +
> +        cl = repo.unfiltered().changelog
> +        affectedrevs = (cl.rev(n) for n in affectednodes if n in cl.nodemap)
> +        repo.hidden.updatevisibility(affectedrevs)
>          tr.close()
>      finally:
>          tr.release()

This is the first half of a series that introduces the concept of an 
explicit hidden store.  It is represented as a set, where you can test 
if a given commit is hidden or not, and is serialized as a set of roots 
of the hidden part of the graph.

It is currently just a cache of the current hidden algorithm, and the 
final patch in the series (not sent yet) adds validation that the hidden 
store always matches the computehidden result.

Other parts of Mercurial communicate with the hidden store simply by 
telling it what nodes they've touched 
(`repo.hidden.updatevisibility(affectedrevs)`), so knowledge about the 
exact hidden semantics remains confined to hidden.py.

This direction has several benefits:
- moves the computation of hiddeness to write-time instead of read-time, 
so we only have to pay it once
- enables external extensions to interact with hidden computations more 
easily (they can now observe hidden affecting changes by wrapping the 
updatevisibility function, without having to individually monkey patch 
into bookmarks/tags/obsmarkers/etc)
- potentially gives us future flexibility on the defining and storing 
what is hidden and what is not
Gregory Szorc - May 18, 2017, 7:10 p.m.
> On May 18, 2017, at 11:34, Durham Goode <durham@fb.com> wrote:
> 
>> On 5/18/17 11:24 AM, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode <durham@fb.com>
>> # Date 1495129620 25200
>> #      Thu May 18 10:47:00 2017 -0700
>> # Node ID b09fa310c34bdbad4fc2b5b914e84e0fc9dc4b68
>> # Parent  0979ce9d40803103441b6df1ebefafe80bb9a057
>> hidden: alert hidden store when obsmarkers are created
>> 
>> This calls the new updatevisibility function from obsolete.createmarkers(). This
>> let's us update the currently stored hidden information when markers change.
>> 
>> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
>> --- a/mercurial/obsolete.py
>> +++ b/mercurial/obsolete.py
>> @@ -1226,6 +1226,7 @@ def createmarkers(repo, relations, flag=
>>         metadata['user'] = repo.ui.username()
>>     tr = repo.transaction('add-obsolescence-marker')
>>     try:
>> +        affectednodes = set()
>>         markerargs = []
>>         for rel in relations:
>>             prec = rel[0]
>> @@ -1258,6 +1259,11 @@ def createmarkers(repo, relations, flag=
>>             repo.obsstore.create(tr, nprec, nsucs, flag, parents=npare,
>>                                  date=date, metadata=localmetadata)
>>             repo.filteredrevcache.clear()
>> +            affectednodes.add(nprec)
>> +
>> +        cl = repo.unfiltered().changelog
>> +        affectedrevs = (cl.rev(n) for n in affectednodes if n in cl.nodemap)
>> +        repo.hidden.updatevisibility(affectedrevs)
>>         tr.close()
>>     finally:
>>         tr.release()
> 
> This is the first half of a series that introduces the concept of an explicit hidden store.  It is represented as a set, where you can test if a given commit is hidden or not, and is serialized as a set of roots of the hidden part of the graph.
> 
> It is currently just a cache of the current hidden algorithm, and the final patch in the series (not sent yet) adds validation that the hidden store always matches the computehidden result.

Do you have the full series (even if not in sendable state) available to pull from? I have a few questions that I think could be answered by seeing the final state...

Only glancing at this, I like the general direction. I do have several nits. But I'll hold off unless you ask for them privately because higher-level discussion is more important.

> 
> Other parts of Mercurial communicate with the hidden store simply by telling it what nodes they've touched (`repo.hidden.updatevisibility(affectedrevs)`), so knowledge about the exact hidden semantics remains confined to hidden.py.
> 
> This direction has several benefits:
> - moves the computation of hiddeness to write-time instead of read-time, so we only have to pay it once
> - enables external extensions to interact with hidden computations more easily (they can now observe hidden affecting changes by wrapping the updatevisibility function, without having to individually monkey patch into bookmarks/tags/obsmarkers/etc)
> - potentially gives us future flexibility on the defining and storing what is hidden and what is not
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Durham Goode - May 18, 2017, 7:26 p.m.
On 5/18/17 12:10 PM, Gregory Szorc wrote:
>
>
>> On May 18, 2017, at 11:34, Durham Goode <durham@fb.com> wrote:
>>
>>> On 5/18/17 11:24 AM, Durham Goode wrote:
>>> # HG changeset patch
>>> # User Durham Goode <durham@fb.com>
>>> # Date 1495129620 25200
>>> #      Thu May 18 10:47:00 2017 -0700
>>> # Node ID b09fa310c34bdbad4fc2b5b914e84e0fc9dc4b68
>>> # Parent  0979ce9d40803103441b6df1ebefafe80bb9a057
>>> hidden: alert hidden store when obsmarkers are created
>>>
>>> This calls the new updatevisibility function from obsolete.createmarkers(). This
>>> let's us update the currently stored hidden information when markers change.
>>>
>>> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
>>> --- a/mercurial/obsolete.py
>>> +++ b/mercurial/obsolete.py
>>> @@ -1226,6 +1226,7 @@ def createmarkers(repo, relations, flag=
>>>         metadata['user'] = repo.ui.username()
>>>     tr = repo.transaction('add-obsolescence-marker')
>>>     try:
>>> +        affectednodes = set()
>>>         markerargs = []
>>>         for rel in relations:
>>>             prec = rel[0]
>>> @@ -1258,6 +1259,11 @@ def createmarkers(repo, relations, flag=
>>>             repo.obsstore.create(tr, nprec, nsucs, flag, parents=npare,
>>>                                  date=date, metadata=localmetadata)
>>>             repo.filteredrevcache.clear()
>>> +            affectednodes.add(nprec)
>>> +
>>> +        cl = repo.unfiltered().changelog
>>> +        affectedrevs = (cl.rev(n) for n in affectednodes if n in cl.nodemap)
>>> +        repo.hidden.updatevisibility(affectedrevs)
>>>         tr.close()
>>>     finally:
>>>         tr.release()
>>
>> This is the first half of a series that introduces the concept of an explicit hidden store.  It is represented as a set, where you can test if a given commit is hidden or not, and is serialized as a set of roots of the hidden part of the graph.
>>
>> It is currently just a cache of the current hidden algorithm, and the final patch in the series (not sent yet) adds validation that the hidden store always matches the computehidden result.
>
> Do you have the full series (even if not in sendable state) available to pull from? I have a few questions that I think could be answered by seeing the final state...
>
> Only glancing at this, I like the general direction. I do have several nits. But I'll hold off unless you ask for them privately because higher-level discussion is more important.
>
The full series can be seen here: 
https://bitbucket.org/DurhamG/hg/commits/branch/hidden

All the core test pass at the tip. The tip of the series includes 
warning output if the cache ever doesn't match the old hidden result, so 
the fact that the tests pass without giving that warning means the cache 
always matches the current hidden result.

Patch

diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
--- a/mercurial/obsolete.py
+++ b/mercurial/obsolete.py
@@ -1226,6 +1226,7 @@  def createmarkers(repo, relations, flag=
         metadata['user'] = repo.ui.username()
     tr = repo.transaction('add-obsolescence-marker')
     try:
+        affectednodes = set()
         markerargs = []
         for rel in relations:
             prec = rel[0]
@@ -1258,6 +1259,11 @@  def createmarkers(repo, relations, flag=
             repo.obsstore.create(tr, nprec, nsucs, flag, parents=npare,
                                  date=date, metadata=localmetadata)
             repo.filteredrevcache.clear()
+            affectednodes.add(nprec)
+
+        cl = repo.unfiltered().changelog
+        affectedrevs = (cl.rev(n) for n in affectednodes if n in cl.nodemap)
+        repo.hidden.updatevisibility(affectedrevs)
         tr.close()
     finally:
         tr.release()