From: Mike Bayer Date: Sat, 17 Jan 2009 06:27:02 +0000 (+0000) Subject: WeakCompositeKey was coded incorrectly and was not weakly referencing anything. ... X-Git-Tag: rel_0_5_1~8 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=345eaeed74588d97fc614a396dd6cfe5f8ece938;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git WeakCompositeKey was coded incorrectly and was not weakly referencing anything. However when repaired, the usage within RelationLoader._create_joins() still creates cycles between key elements and the value placed in the dict. In the interests of risk reduction, WCK is now removed and the two caches it was used for are now non-cached. Speed comparisons with one join/eager-heavy web application show no noticeable effect in response time. --- diff --git a/CHANGES b/CHANGES index ac92141434..0feb2fdc2d 100644 --- a/CHANGES +++ b/CHANGES @@ -8,6 +8,10 @@ CHANGES ======== - orm + - Removed an internal join cache which could potentially leak + memory when issuing query.join() repeatedly to ad-hoc + selectables. + - Modernized the "no mapped table" exception and added a more explicit __table__/__tablename__ exception to declarative. diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 7198cc830f..c83e03599d 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -381,7 +381,6 @@ class RelationProperty(StrategizedProperty): self.join_depth = join_depth self.local_remote_pairs = _local_remote_pairs self.extension = extension - self.__join_cache = {} self.comparator_factory = comparator_factory or RelationProperty.Comparator self.comparator = self.comparator_factory(self, None) util.set_creation_order(self) @@ -1004,12 +1003,6 @@ class RelationProperty(StrategizedProperty): return self.mapper.common_parent(self.parent) def _create_joins(self, source_polymorphic=False, source_selectable=None, dest_polymorphic=False, dest_selectable=None, of_type=None): - key = util.WeakCompositeKey(source_polymorphic, source_selectable, dest_polymorphic, dest_selectable, of_type) - try: - return self.__join_cache[key] - except KeyError: - pass - if source_selectable is None: if source_polymorphic and self.parent.with_polymorphic: source_selectable = self.parent._with_polymorphic_selectable @@ -1076,10 +1069,9 @@ class RelationProperty(StrategizedProperty): else: target_adapter = None - self.__join_cache[key] = ret = (primaryjoin, secondaryjoin, + return (primaryjoin, secondaryjoin, (source_selectable or self.parent.local_table), (dest_selectable or self.mapper.local_table), secondary, target_adapter) - return ret def _get_join(self, parent, primary=True, secondary=True, polymorphic_parent=True): """deprecated. use primary_join_against(), secondary_join_against(), full_join_against()""" diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 5f820565f1..7195310cdf 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -594,7 +594,6 @@ class EagerLoader(AbstractRelationLoader): def init(self): super(EagerLoader, self).init() - self.clauses = {} self.join_depth = self.parent_property.join_depth def init_class_attribute(self): @@ -669,14 +668,8 @@ class EagerLoader(AbstractRelationLoader): towrap = context.eager_joins.setdefault(entity_key, default_towrap) - # create AliasedClauses object to build up the eager query. this is cached after 1st creation. - # this also allows ORMJoin to cache the aliased joins it produces since we pass the same - # args each time in the typical case. - path_key = util.WeakCompositeKey(*path) - try: - clauses = self.clauses[path_key] - except KeyError: - self.clauses[path_key] = clauses = mapperutil.ORMAdapter(mapperutil.AliasedClass(self.mapper), + # create AliasedClauses object to build up the eager query. + clauses = mapperutil.ORMAdapter(mapperutil.AliasedClass(self.mapper), equivalents=self.mapper._equivalent_columns) if adapter: diff --git a/lib/sqlalchemy/util.py b/lib/sqlalchemy/util.py index 619888135d..110ef21d5e 100644 --- a/lib/sqlalchemy/util.py +++ b/lib/sqlalchemy/util.py @@ -1178,38 +1178,6 @@ class _TLocalRegistry(ScopedRegistry): except AttributeError: pass -class WeakCompositeKey(object): - """an weak-referencable, hashable collection which is strongly referenced - until any one of its members is garbage collected. - - """ - keys = set() - - __slots__ = 'args', '__weakref__' - - def __init__(self, *args): - self.args = [self.__ref(arg) for arg in args] - WeakCompositeKey.keys.add(self) - - def __ref(self, arg): - if isinstance(arg, type): - return weakref.ref(arg, self.__remover) - else: - return lambda: arg - - def __remover(self, wr): - WeakCompositeKey.keys.discard(self) - - def __hash__(self): - return hash(tuple(self)) - - def __cmp__(self, other): - return cmp(tuple(self), tuple(other)) - - def __iter__(self): - return iter(arg() for arg in self.args) - - class _symbol(object): def __init__(self, name): """Construct a new named symbol.""" diff --git a/test/base/utils.py b/test/base/utils.py index b3e11f230b..0b7762b7d6 100644 --- a/test/base/utils.py +++ b/test/base/utils.py @@ -4,7 +4,6 @@ from sqlalchemy import util, sql, exc from testlib import TestBase from testlib.testing import eq_, is_, ne_ - class OrderedDictTest(TestBase): def test_odict(self): o = util.OrderedDict() @@ -943,5 +942,6 @@ class TestClassHierarchy(TestBase): eq_(set(util.class_hierarchy(Mixin)), set()) eq_(set(util.class_hierarchy(A)), set((A, B, object))) + if __name__ == "__main__": testenv.main() diff --git a/test/profiling/memusage.py b/test/profiling/memusage.py index ab6a5e2c0a..3e5db8ae42 100644 --- a/test/profiling/memusage.py +++ b/test/profiling/memusage.py @@ -300,6 +300,44 @@ class MemUsageTest(EnsureZeroed): metadata.drop_all() assert_no_mappers() + def test_join_cache(self): + metadata = MetaData(testing.db) + + table1 = Table("table1", metadata, + Column('id', Integer, primary_key=True), + Column('data', String(30)) + ) + + table2 = Table("table2", metadata, + Column('id', Integer, primary_key=True), + Column('data', String(30)), + Column('t1id', Integer, ForeignKey('table1.id')) + ) + + class Foo(object): + pass + + class Bar(object): + pass + + mapper(Foo, table1, properties={ + 'bars':relation(mapper(Bar, table2)) + }) + metadata.create_all() + + session = sessionmaker() + + @profile_memory + def go(): + s = table2.select() + session().query(Foo).join((s, Foo.bars)).all() + + try: + go() + finally: + metadata.drop_all() + + def test_mutable_identity(self): metadata = MetaData(testing.db)