[PATCH 3 of 3] localrepo: cache types for filtered repos (issue5043)

Pierre-Yves David pierre-yves.david at ens-lyon.org
Sat May 27 07:28:43 EDT 2017



On 05/27/2017 10:59 AM, Jun Wu wrote:
> Patch 1 looks good to me.
>
> Note: Working around Python class cycle bug [1] is hard and I have given up.
> I think it's safe to assume every class ever defined won't be freed by
> refcount in Python 2. So reducing the number of classes seems good.
>
> [1]: http://bugs.python.org/issue17950
>
> Excerpts from Gregory Szorc's message of 2017-05-26 17:42:36 -0700:
>> [...]
>> As the inline comment explains, there is danger to caching the type
>> if the __class__ of the repo is updated after first call. Code to
>> validate this doesn't happen has been added. It is possible this may
>> break some extensions. If it does, I think they deserve to break,
>> as mutating the __class__ of a repo is dangerous and should be done
>> as early as possible and only in reposetup().
>
> Mutating __class__  is a common pattern in Mercurial world. wrapfunction
> docstring explicitly recommends this pattern. I don't think we should break
> extensions here.

+1, patch 3 as it is is a bit scary. I think jun proposed solution 
(using source class as cache key) is a good one.

>> [...]
>> +        # Cache of types representing filtered repos.
>> +        self._filteredexpectedbasetype = None
>> +        self._filteredrepotypes = {}
>
> (also to patch 2): Different filters could reuse a same filteredrepo class
> to reduce class count (since they cannot be freed). Maybe we can implement
> __repr__ to make debugging easier.
>
> We can use self.unfiltered().__class__ as the cache keys. Since class won't
> be freed, it does not matter if we add new references. And that solves the
> correctness issue.
>
>> [...]
>>          self.names = namespaces.namespaces()
>>
>> @@ -493,11 +497,31 @@ class localrepository(object):
>>
>>      def filtered(self, name):
>
> Maybe something like this:
>
>           key = self._unfiltered().__class__
>           if key not in self._filteredrepotypes:
>               class filteredrepo(repoview.repoview, key):
>                   pass
>               self._filteredrepotypes[key] = filteredrepo
>           return self._filteredrepotypes[key](self, name)
>
>> [...]
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list