[PATCH 3 of 3 v2] repoview: cache repoview class instances (issue5043)

[Gregory Szorc]

> On Jan 17, 2016, at 17:24, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
> 
> 
> 
>> On 01/17/2016 05:19 PM, Gregory Szorc wrote:
>> 
>> 
>>> On Jan 17, 2016, at 17:10, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
>>> 
>>> 
>>> 
>>>> On 01/17/2016 02:18 PM, Gregory Szorc wrote:
>>>> # HG changeset patch
>>>> # User Gregory Szorc <gregory.szorc at gmail.com>
>>>> # Date 1453069024 28800
>>>> #      Sun Jan 17 14:17:04 2016 -0800
>>>> # Node ID 3a418d20ca9d0345416d65b82fd3f49101bcc5b2
>>>> # Parent  91969eee20a1dcd52470258164bc44528861a4c6
>>>> repoview: cache repoview class instances (issue5043)
>>>> 
>>>> The Python garbage collector was reporting the proxycls instances
>>>> created on every invocation of localrepository.filtered() were
>>>> uncollectable. This led to leaking memory.
>>>> 
>>>> We fix the leak by introducing a cache of the proxy classes. After
>>>> this patch, the garbage collector no longer reports uncollectable
>>>> objects after each iteration of `hg convert`. This likely prevents
>>>> memory leaks in a number of other operations as well (pretty much
>>>> anything relying heavily on repo.filtered() calls).
>>>> 
>>>> We stop short of caching the instantiated proxycls instances, which
>>>> we /should/ be able to do safely. This feels a bit too risky for
>>>> a change just before code freeze, however.
>>> 
>>> I'm either missing something or the behavior of this patch is incorrect.
>>> 
>>> If the class of the repo objet changes, we won't invalidate the cache, and the next call to "filtered" will return an object with an outdated proxy class.
>> 
>> That's a valid observation. But the class should be static after reposetup functions are called. And we shouldn't be obtaining a repoview before reposetup is called, right?
> 
> I would not bet on class being static (and I'am pretty sure it is not).
> 
> What's make the class object uncollectable, could we not fix this instead?

I, uh, have no idea. Either classes aren't garbage collected or the ref to repo.__class__ is somehow making it not collectible. This is a minor leak compared to the first 2 patches. So if you could queue those, it would be appreciated.

> 
> (You are right in the fact that I don't think there is anything that would obtains a repoview before reposetup is called)
> 
> Cheers,
> 
> -- 
> Pierre-Yves David