As attachement, here's the patch (against rev 2132). It's local to
sqlalchemy/orm/, so:
cd ..../sqlalchemy/orm
patch -p0 < attribute_cache.patch
About the patch itself:
> 1. the cache should be a WeakKeyDictionary
OK, done. Performances are still OK, differences between built-in dict are
negligible.
> 2. the cache needs to be cleared for a particular cache (class ?)
If I understand right, while calling register_attribute(), the cache will be
cleared for the given class, if this one has been cached. Is there anywhere
else the cache needs to cleared ?
> 3. the cache should *probably* be at the module level, and not within
> the AttributeManager itself; SA uses only a single AttributeManager per
> application so it probably doesnt matter
I let the cache within the AttributeManager... Things can be moved at the
module level, but if it's not necessary right now, it might not be a good
idea to do it right now (when it will be necessary, the design may have been
changed and modifications not valid anymore).
> 4. the raise should use an exception class of some kind
I've made it raise a TypeError... Yes, string based exceptions are very a bad
thing !
Attributes are cached while using the managed_attributes() *and*
noninherited_managed_attributes() funcs. Those are very similar. I hesitated
refactor them, but centralize cache management is probably a good thing. This
is the purpose of the second patch, which include this refactoring. As
expected, performances decrease. Here's the traditionnal benchmark results:
SA 0.3.1, rev 2127:
total time 4.29376721382
real 0m5.420s
user 0m4.088s
sys 0m0.108s
SA with attr. cache, no refactoring (~2X faster, back closed to 0.2.8
speed):
total time 2.34819602966
real 0m3.013s
user 0m2.344s
sys 0m0.088s
SA with attr. cache, with refactoring (30% slower than without
refactoring)
total time 3.05679416656
real 0m3.747s
user 0m2.988s
sys 0m0.068s
It's up to you choosing the patch ! IMO, I'm *not* in favor to use refactoring
in this case :).
Finally, I've put a clear_attribute_cache func which, ... clear the attribute
cache. While client code may not have to worry about caching, it may need to
clear it...
> thanks much, this is the kind of user involvement i like.
Well, you're very welcome ! Glad to help !
Cheers,
Seb
--
Sébastien LELONG
sebastien.lelong[at]sirloon.net
P.S:
> im beginning
> to suspect that yield introduces overhead vs. a straight list ?
> (python interpreter storing stack frames ? dunno).
dunnotoo :)
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"sqlalchemy" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/sqlalchemy?hl=en
-~----------~----~----~----~------~----~------~--~---
Index: attributes.py
===================================================================
--- attributes.py (révision 2132)
+++ attributes.py (copie de travail)
@@ -609,8 +609,18 @@
class AttributeManager(object):
"""allows the instrumentation of object attributes. AttributeManager is stateless, but can be
- overridden by subclasses to redefine some of its factory operations."""
+ overridden by subclasses to redefine some of its factory operations. Also be aware AttributeManager
+ will cache attributes for a given class, allowing not to determine those for each objects (used
+ in managed_attributes() and noninherited_managed_attributes()). This cache is cleared for a given class
+ while calling register_attribute(), and can be cleared using clear_attribute_cache()"""
+ def __init__(self):
+ # will cache attributes, indexed by class objects
+ self._attribute_cache = weakref.WeakKeyDictionary()
+
+ def clear_attribute_cache(self):
+ self._attribute_cache.clear()
+
def rollback(self, *obj):
"""retrieves the committed history for each object in the given list, and rolls back the attributes
each instance to their original value."""
@@ -639,21 +649,31 @@
def managed_attributes(self, class_):
"""returns an iterator of all InstrumentedAttribute objects associated with the given class."""
+ if self._attribute_cache.has_key(class_):
+ return self._attribute_cache[class_]
+
+ self._attribute_cache[class_] = []
if not isinstance(class_, type):
- raise repr(class_) + " is not a type"
+ raise TypeError(repr(class_) + " is not a type")
for key in dir(class_):
value = getattr(class_, key, None)
if isinstance(value, InstrumentedAttribute):
- yield value
+ self._attribute_cache[class_].append(value)
+ return self._attribute_cache[class_]
def noninherited_managed_attributes(self, class_):
+ if self._attribute_cache.has_key(class_):
+ return self._attribute_cache[class_]
+
+ self._attribute_cache[class_] = []
if not isinstance(class_, type):
- raise repr(class_) + " is not a type"
+ raise TypeError(repr(class_) + " is not a type")
for key in list(class_.__dict__):
value = getattr(class_, key, None)
if isinstance(value, InstrumentedAttribute):
- yield value
-
+ self._attribute_cache[class_].append(value)
+ return self._attribute_cache[class_]
+
def is_modified(self, object):
for attr in self.managed_attributes(object.__class__):
if attr.check_mutable_modified(object):
@@ -733,6 +753,10 @@
def register_attribute(self, class_, key, uselist, callable_=None, **kwargs):
"""registers an attribute at the class level to be instrumented for all instances
of the class."""
+ # firt invalidate the cache for the given class
+ # (will be reconstituted as needed, while getting managed attributes)
+ self._attribute_cache.pop(class_,None)
+
#print self, "register attribute", key, "for class", class_
if not hasattr(class_, '_state'):
def _get_state(self):
Index: attributes.py
===================================================================
--- attributes.py (révision 2132)
+++ attributes.py (copie de travail)
@@ -609,8 +609,18 @@
class AttributeManager(object):
"""allows the instrumentation of object attributes. AttributeManager is stateless, but can be
- overridden by subclasses to redefine some of its factory operations."""
+ overridden by subclasses to redefine some of its factory operations. Also be aware AttributeManager
+ will cache attributes for a given class, allowing not to determine those for each objects (used
+ in managed_attributes() and noninherited_managed_attributes()). This cache is cleared for a given class
+ while calling register_attribute(), and can be cleared using clear_attribute_cache()"""
+ def __init__(self):
+ # will cache attributes, indexed by class objects
+ self._attribute_cache = weakref.WeakKeyDictionary()
+
+ def clear_attribute_cache(self):
+ self._attribute_cache.clear()
+
def rollback(self, *obj):
"""retrieves the committed history for each object in the given list, and rolls back the attributes
each instance to their original value."""
@@ -636,24 +646,27 @@
for o in obj:
o._state['original'] = CommittedState(self, o)
o._state['modified'] = False
+
+ def _managed_attributes(self, class_, class_attrs):
+ if self._attribute_cache.has_key(class_):
+ return self._attribute_cache[class_]
+
+ self._attribute_cache[class_] = []
+ if not isinstance(class_, type):
+ raise TypeError(repr(class_) + " is not a type")
+ for key in class_attrs:
+ value = getattr(class_, key, None)
+ if isinstance(value, InstrumentedAttribute):
+ self._attribute_cache[class_].append(value)
+ return self._attribute_cache[class_]
def managed_attributes(self, class_):
"""returns an iterator of all InstrumentedAttribute objects associated with the given class."""
- if not isinstance(class_, type):
- raise repr(class_) + " is not a type"
- for key in dir(class_):
- value = getattr(class_, key, None)
- if isinstance(value, InstrumentedAttribute):
- yield value
+ return self._managed_attributes(class_,dir(class_))
def noninherited_managed_attributes(self, class_):
- if not isinstance(class_, type):
- raise repr(class_) + " is not a type"
- for key in list(class_.__dict__):
- value = getattr(class_, key, None)
- if isinstance(value, InstrumentedAttribute):
- yield value
-
+ return self._managed_attributes(class_,list(class_.__dict__))
+
def is_modified(self, object):
for attr in self.managed_attributes(object.__class__):
if attr.check_mutable_modified(object):
@@ -733,6 +746,10 @@
def register_attribute(self, class_, key, uselist, callable_=None, **kwargs):
"""registers an attribute at the class level to be instrumented for all instances
of the class."""
+ # firt invalidate the cache for the given class
+ # (will be reconstituted as needed, while getting managed attributes)
+ self._attribute_cache.pop(class_,None)
+
#print self, "register attribute", key, "for class", class_
if not hasattr(class_, '_state'):
def _get_state(self):
