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

Pierre-Yves David pierre-yves.david at ens-lyon.org
Sat Jun 3 21:28:30 EDT 2017



On 06/03/2017 10:08 PM, Jun Wu wrote:
> Excerpts from Pierre-Yves David's message of 2017-06-03 21:38:47 +0200:
>>
>> On 06/03/2017 07:38 PM, Jun Wu wrote:
>>> Excerpts from Martin von Zweigbergk's message of 2017-06-03 10:05:06 -0700:
>>>> [...]
>>>> If all current code paths use the full set (and don't care about order), I
>>>
>>> I think we are talking about phase revsets.
>>
>> No. we are talking about the low level set computation, like in
>> '_computeobsoleteset' and 'computehidden'. The revsets are out of the
>> scope for that discussion.
>
> I understand we want to continue use Python sets in obsolete.py. I didn't
> say that's a problem.
>
>> The function aboves performs unordered operation on python 'set' object
>> and return a python 'set' object.
>>
>> The phase logic itself is computing and cache python 'set' object.
>> Having the phase logic explicitly covert that set to a smartset before
>> making it available to this low level function is not necessary and end
>> up being harmful for performance.
>>
>> I just made a small series that allow to skip the smartset conversion
>> for these function. It pass tests and reveals a quite significant impact
>
> My point is we can just convert a smartset to Python set in an efficient way
> (ex. "[PATCH] smartset: add a "toset" method"). Then the change will just be
> something like:
>
>   # in obsolete.py
>   - s = set(nonpublic)
>   + s = nonpublic.toset()
>
> I'd expect it to be as efficient as accessing "phasecache._phasesets"
> directly. It is more flexible.

I see you point, that is something to consider.

I'm not fully convinced yet because the following chain  of 
usage/transformation seems a bit strange at the place where it is used"

   set → smartset (→ check if smartset has a 'toset' method) → set

But I'll think about it when we get to the point where is it worthwhile 
to update that parts of the code and I submit my series to switch to 
python sets

Cheers,

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list