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

[Pierre-Yves David]

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?

(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