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

Jun Wu quark at fb.com
Sat May 27 04:59:53 EDT 2017


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.

> [...]
> +        # 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)

> [...]


More information about the Mercurial-devel mailing list