instant team mailing list archive

["Garth N. Wells" <gnw20@xxxxxxxxx>]

Anders Logg wrote:
> On Sun, Feb 07, 2010 at 12:34:52PM +0000, Garth N. Wells wrote:
>> Instant gurus: Take a look at this change. I hope that it's not evil.
>>
>> Instant was inserting an object into the memory cache even though it was
>>  already found in the cache. It was inserting the object based on its
>> address, and not its signature. This meant that a element or form
>> declared in a loop would be repeatedly inserted into the memory cache,
>> despite the signature being the same.
>>
>> Garth
> 
> I don't know much about the internals of Instant, but did you consider
> that there are two different "signatures"? One is the thing returned
> by __hash__ in jitobject.py and the other is the thing returned by
> signature in the same file. The first is used for in-memory cache and
> the second for disk cache.
> 

The problem is that there is some code that inserts things on the basis
if the address. Some names in the code make it unclear what's intended.
The below is from check_memory_cache. I've added some comments.

    moduleids = [moduleid]
    module = memory_cached_module(moduleid)
    if module: return module, moduleids

    if hasattr(moduleid, "signature"):
	# GNW: Why not signature = moduleid.signature()?	
        moduleid = moduleid.signature()
        module = memory_cached_module(moduleid)
        if module:
	    # GNW: Is moduleid meant to be 'moduleid =
            #      moduleid.signature()' or from 'moduleids =
            #      [moduleid]'. The below uses the address of moduleid
            #      as passed to the function.
            for moduleid in moduleids:
                place_module_in_memory_cache(moduleid, module)
            return module, moduleids
        moduleids.append(moduleid)

Also, I don't see what a function called 'check_memory_cache' should be
inserting something into the cache. It's confusing.

Garth



> --
> Anders
> 
> 
>>
>> -------- Original Message --------
>> Subject: [Branch ~instant-core/instant/main] Rev 288: Fix suspected
>> leak in	Instant.
>> Date: Sun, 07 Feb 2010 12:01:13 -0000
>> From: noreply@xxxxxxxxxxxxx
>> Reply-To: noreply@xxxxxxxxxxxxx
>> To: Garth Wells <gnw20@xxxxxxxxx>
>>
>> ------------------------------------------------------------
>> revno: 288
>> committer: Garth N. Wells <gnw20@xxxxxxxxx>
>> branch nick: instant
>> timestamp: Sun 2010-02-07 11:51:12 +0000
>> message:
>>   Fix suspected leak in Instant.
>> modified:
>>   src/instant/cache.py
>>
>>
> 
>> === modified file 'src/instant/cache.py'
>> --- src/instant/cache.py	2009-03-20 14:10:01 +0000
>> +++ src/instant/cache.py	2010-02-07 11:51:12 +0000
>> @@ -83,8 +83,8 @@
>>                        "'%s' from moduleid.signature()." % moduleid)
>>          module = memory_cached_module(moduleid)
>>          if module:
>> -            for moduleid in moduleids:
>> -                place_module_in_memory_cache(moduleid, module)
>> +            #for moduleid in moduleids:
>> +            #    place_module_in_memory_cache(moduleid, module)
>>              return module, moduleids
>>          moduleids.append(moduleid)
>>
>>
>>
> 
>> _______________________________________________
>> Mailing list: https://launchpad.net/~instant
>> Post to     : instant@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~instant
>> More help   : https://help.launchpad.net/ListHelp
>