Bugs item #1573394, was opened at 2006-10-08 19:37
Message generated for change (Comment added) made by etrepum
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1573394&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Python Library
Group: Python 2.5
Status: Closed
Resolution: Wont Fix
Priority: 5
Submitted By: Mark Flacy (markaflacy)
Assigned to: Bob Ippolito (etrepum)
Summary: struct module doesn't use weakref for cache

Initial Comment:
At the moment, when you attempt to add your 101st
different Struct object to the cache, all the other 100
entries are tossed into the garbage.  That seems a
trifle odd.

The Struct cache would appear to be a perfect use for a
weakref dictionary.

----------------------------------------------------------------------

>Comment By: Bob Ippolito (etrepum)
Date: 2006-10-16 19:57

Message:
Logged In: YES 
user_id=139309

The flush strategy is a silly thing to rattle on about.

The idea is just to pick a cache size that's big enough that
any sane application will never have to flush it. The
current cache size is plenty big enough for anything I threw
at it. If there's an application out there that needs a
bigger cache I'd like to see it, and then perhaps we could
adjust the size in some minor release to accommodate it.

A FIFO is probably a bad strategy because there are a lot of
common structs that are used a lot (the ones with just a few
characters).

If there was an obviously better solution here, surely it
would have already been proposed or implemented for the re
module.

----------------------------------------------------------------------

Comment By: Josiah Carlson (josiahcarlson)
Date: 2006-10-16 18:41

Message:
Logged In: YES 
user_id=341410

Add a deque and a bit of logic, and one would get a fifo
implementation of the cache.

from collections import deque
_toss = deque()

def _compile(fmt):
    # Internal: compile struct pattern
    if len(_cache) >= _MAXCACHE:
        del _cache[_toss.popleft()]
    s = Struct(fmt)
    _cache[fmt] = s
    _toss.append(fmt)
    return s

Race conditions from multiple threads could result in
#threads-1 more entries in the cache than _MAXCACHE, but
this could be trimmed in later calls by replacing 'if' with
'while'.

----------------------------------------------------------------------

Comment By: Bob Ippolito (etrepum)
Date: 2006-10-12 13:54

Message:
Logged In: YES 
user_id=139309

Yes, that code is different. You haven't shown that it's
better though. Using popitem probably doesn't have very good
cache effects.

----------------------------------------------------------------------

Comment By: �iga Seilnacht (zseil)
Date: 2006-10-12 13:36

Message:
Logged In: YES 
user_id=1326842

I was thinking abot something like this:

Index: Lib/struct.py
===================================================================
--- Lib/struct.py       (revision 52316)
+++ Lib/struct.py       (working copy)
@@ -35,7 +35,7 @@
 def _compile(fmt):
     # Internal: compile struct pattern
     if len(_cache) >= _MAXCACHE:
-        _cache.clear()
+        _cache.popitem()
     s = Struct(fmt)
     _cache[fmt] = s
     return s

(sorry, I don't have the rights to attach a file)


----------------------------------------------------------------------

Comment By: Bob Ippolito (etrepum)
Date: 2006-10-12 12:56

Message:
Logged In: YES 
user_id=139309

Weakrefs would slow it down.. probably to the point where
the cache wouldn't be an improvement at all.

The re module does the same thing with the regular
expression cache. This isn't a bug until someone presents a
patch that proves another strategy is better for performance.

----------------------------------------------------------------------

Comment By: �iga Seilnacht (zseil)
Date: 2006-10-12 11:52

Message:
Logged In: YES 
user_id=1326842

WeakValueDictionary would be useless here; the cache is
the only thing that holds references to Struct objects.
Replacing the _cache.clear() call with _cache.popitem()
seems like a better solution.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1573394&group_id=5470
_______________________________________________
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to