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 sqlalchemy@googlegroups.com 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):