[PATCH 2 of 5 V1 REBASED] obsolete: drop successors sets which are subset of another one

Augie Fackler raf at durin42.com
Sat Dec 8 17:53:56 CST 2012


On Dec 7, 2012, at 6:25 PM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:

> On Fri, Dec 07, 2012 at 03:07:32PM -0600, Augie Fackler wrote:
>> 
>> On Nov 30, 2012, at 3:12 PM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
>> 
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david at logilab.fr>
>>> # Date 1352509019 -3600
>>> # Node ID a174b9f3d76e9458e1bfbc590cb9d550dd478ad6
>>> # Parent  d58f0a5976a0e663e2afff58ccd4883eb4d9b99b
>>> obsolete: drop successors sets which are subset of another one
>>> 
>>> If both "(B,)" and "(B, C)" are successors set of "A", "(B,)" is dropped.
>>> We won't be interrested in detection such divergence scenario.
>>> 
>>> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
>>> --- a/mercurial/obsolete.py
>>> +++ b/mercurial/obsolete.py
>>> @@ -516,11 +516,28 @@ def successorssets(repo, initialnode, ca
>>>                break
>>>            else:
>>>                # computation was succesful for *all* marker.
>>>                # Add computed successors set to the cache
>>>                # (will be poped from to proceeed on the next iteration)
>>> -                cache[node] = list(set(tuple(r) for r in lss if r))
>>> +                #
>>> +                # We remove successors set that are subset of another one
>>> +                # this fil
>>> +                candsucset = sorted(((len(ss), set(ss), ss) for ss in lss),
>>> +                                    reverse=True)
>> 
>> Is there a reason to not use sorted() with they key= kwarg here? Is decorate-sort-undecorate really faster here?
> 
> Not really, but we need the `set(ss)` and `ss` anyway. Computing the len(ss) at
> the same time that everything else seemed simpler.
> 
> I have not strong opinion on keep this forms.

I guess I would have written this:

finalsucset = sorted(((set(css), css) for css in lss if lss), key=len, reverse=True)

> 
>>> +                finalsucset = []
>>> +                for cl, cs, css in candsucset:
>>> +                    if not css:
>>> +                        # remove empty successors set
>>> +                        continue
>>> +                    for fs, fss in finalsucset:
>>> +                        if cs.issubset(fs):
>>> +                            break
>>> +                    else:
>>> +                        finalsucset.append((cs, css))
>> 
>> I'm not sure why you're saving cs in the finalsucset, since you immediately trim it off on the next line as soon as the loop was over.
> 
> Because I'm trimming it *after* the loop while using it in the loop itself.
> Three lines above.

Oh, that's really subtle. I think I might have made a parallel set object or something, just for clarity and avoiding looping over the dataset an extra time.

> 
> -- 
> Pierre-Yves David



More information about the Mercurial-devel mailing list