[PATCH stable] revset: don't cache abstractsmartset min/max invocations infinitely

Mads Kiilerich mads at kiilerich.com
Wed Oct 26 08:09:17 EDT 2016


On 10/26/2016 01:21 PM, Yuya Nishihara wrote:
> On Tue, 25 Oct 2016 18:57:26 +0200, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski at unity3d.com>
>> # Date 1477414587 -7200
>> #      Tue Oct 25 18:56:27 2016 +0200
>> # Branch stable
>> # Node ID c2fe58cd4235fc6c8cabea882794303d620bec3a
>> # Parent  76c57e1fe79b0980b377b4f305635dea393d6315
>> revset: don't cache abstractsmartset min/max invocations infinitely
>>
>> There was a "leak", apparently introduced in ab66c1dee405. When running:
>>
>>      hg = hglib.open('repo')
>>      while True:
>>          hg.log("max(branch('default'))")
>>
>> all filteredset instances from branch() would be cached indefinitely by the
>> @util.cachefunc annotation on the max() implementation.
> Indeed. We've cached {self: v} pair every time max() was called.
> Queued this for stable, thanks.
>
>> -    @util.cachefunc
>>       def min(self):
>>           """return the minimum element in the set"""
>> -        if self.fastasc is not None:
>> -            for r in self.fastasc():
>> -                return r
>> -            raise ValueError('arg is an empty sequence')
>> -        return min(self)
>> -
>> -    @util.cachefunc
>> +        if self.fastasc is None:
>> +            v = min(self)
>> +        else:
>> +            for v in self.fastasc():
>> +                break
>> +            else:
>> +                raise ValueError('arg is an empty sequence')
>> +        self.min = lambda: v
>> +        return v
> I slightly prefer using propertycache() and wrap it by a function, which
> is a common pattern seen in context.py, but that's just a nitpicking.

Exactly what do you mean with wrapping the property by a function? I 
don't see a clear pattern of that in context.py?

Instead, I would perhaps prefer to introduce a 'gettercache' that works 
on methods that only have self as parameter (and thus can set a simple 
instance method as I do here)  ... or a 'methodcache' that would be like 
cachefunc but store the cache on the first 'self' parameter.

/Mads



More information about the Mercurial-devel mailing list