Commits

Mike Bayer  committed 50d531f

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.

  • Participants
  • Parent commits 65e1e7c

Comments (0)

Files changed (6)

 ========
 
 - 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.
 

File lib/sqlalchemy/orm/properties.py

         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)
         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
         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()"""

File lib/sqlalchemy/orm/strategies.py

     
     def init(self):
         super(EagerLoader, self).init()
-        self.clauses = {}
         self.join_depth = self.parent_property.join_depth
 
     def init_class_attribute(self):
     
         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:

File lib/sqlalchemy/util.py

         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."""

File test/base/utils.py

 from testlib import TestBase
 from testlib.testing import eq_, is_, ne_
 
-
 class OrderedDictTest(TestBase):
     def test_odict(self):
         o = util.OrderedDict()
         eq_(set(util.class_hierarchy(Mixin)), set())
         eq_(set(util.class_hierarchy(A)), set((A, B, object)))
 
+        
 if __name__ == "__main__":
     testenv.main()

File test/profiling/memusage.py

             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)