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

Yuya Nishihara yuya at tcha.org
Wed May 31 09:11:07 EDT 2017


On Sat, 27 May 2017 13:28:43 +0200, Pierre-Yves David wrote:
> 
> 
> 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.

+1. I do that in TortoiseHg for caching changectx class wrappers. If we're
afraid of possible leak because of the class cache, we could use
WeakKeyDictionary.


More information about the Mercurial-devel mailing list