[PATCH] store: break up reference cycle introduced in 9cbff8a39a2a

[Adrian Buehlmann]

On 2011-05-05 09:36, Martin Geisler wrote:
> Adrian Buehlmann <adrian at cadifra.com> writes:
> 
>> I think the reference cycle in 9cbff8a39a2a goes like this:
>>
>>       --> fncacheopener object
>>      |        |
>>      |    local scope of function fncachestore.__init__
>>      |        |
>>       --- fncache object bound to name fnc
>>
>> Not sure if that's really a problem though.
>>
>> With my patch, the references go like this:
>>
>>        -- fncachestore object
>>       |       |
>>       |   _fncacheopener object  --
>>       |       |                    |
>>        -> fncache object           |
>>               |                    |
>>           object of openertype  <--
> 
> I think that very nice drawing should go into the commit message.

You're right. I should have put it into the message.

But the horse is already out of the barn now (my egg is in the main
repo, and it's derived from Dan's hen :-)

> But more generally: Python *can* garbage collect some reference cycles,
> namely the cycles where none of the object define a destructor. This is
> sort of described here:
> 
>   http://docs.python.org/library/gc.html#gc.garbage
> 
> I just wanted to mention this since it seems that people are hunting
> reference cycles just for the sake of hunting them. If removing a cycle
> makes the code architecture cleaner then that's perfect -- go ahead! But
> if not, then I think we can leave a couple of cycles behind.

IIRC, we had some issues with memory usage going up (and staying) in the
past (around that time when Simon made 996c1cd8f530).

I know that the Python GC is supposed to break up cycles. But I think if
we can easily avoid cycles -- like in this case -- we should do it.

If a data structure/algorithm provides some significant benefit by
allowing cycles, then I agree with you.