From: Mike Bayer Date: Sat, 13 Jul 2013 20:28:42 +0000 (-0400) Subject: A performance fix related to the usage of the :func:`.defer` option X-Git-Tag: rel_0_8_3~93 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5b65af098324a1852c1ca885f5c79b68cd2733c5;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git A performance fix related to the usage of the :func:`.defer` option when loading mapped entities. The function overhead of applying a per-object deferred callable to an instance at load time was significantly higher than that of just loading the data from the row (note that ``defer()`` is meant to reduce DB/network overhead, not necessarily function call count); the function call overhead is now less than that of loading data from the column in all cases. There is also a reduction in the number of "lazy callable" objects created per load from N (total deferred values in the result) to 1 (total number of deferred cols). [ticket:2778] --- diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst index 55fd7949a1..ded97c8d84 100644 --- a/doc/build/changelog/changelog_08.rst +++ b/doc/build/changelog/changelog_08.rst @@ -6,6 +6,21 @@ .. changelog:: :version: 0.8.3 + .. change:: + :tags: bug, orm + :tickets: 2778 + + A performance fix related to the usage of the :func:`.defer` option + when loading mapped entities. The function overhead of applying + a per-object deferred callable to an instance at load time was + significantly higher than that of just loading the data from the row + (note that ``defer()`` is meant to reduce DB/network overhead, not + necessarily function call count); the function call overhead is now + less than that of loading data from the column in all cases. There + is also a reduction in the number of "lazy callable" objects created + per load from N (total deferred values in the result) to 1 (total + number of deferred cols). + .. change:: :tags: bug, sqlite :tickets: 2781 diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index 7f1b5e58c4..b95677f0ba 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -608,7 +608,7 @@ class AttributeImpl(object): if key in state.callables: callable_ = state.callables[key] - value = callable_(passive) + value = callable_(state, passive) elif self.callable_: value = self.callable_(state, passive) else: diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py index 5937197fd9..1006d45c1a 100644 --- a/lib/sqlalchemy/orm/loading.py +++ b/lib/sqlalchemy/orm/loading.py @@ -161,7 +161,7 @@ def get_from_identity(session, key, passive): # expired state will be checked soon enough, if necessary return instance try: - state(passive) + state(state, passive) except orm_exc.ObjectDeletedError: session._remove_newly_deleted([state]) return None diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py index 6ade91b3e0..c479d880d8 100644 --- a/lib/sqlalchemy/orm/state.py +++ b/lib/sqlalchemy/orm/state.py @@ -305,14 +305,19 @@ class InstanceState(interfaces._InspectionAttr): dict_.pop(key, None) self.callables[key] = self - def _set_callable(self, dict_, key, callable_): - """Remove the given attribute and set the given callable - as a loader.""" - - old = dict_.pop(key, None) - if old is not None and self.manager[key].impl.collection: - self.manager[key].impl._invalidate_collection(old) - self.callables[key] = callable_ + @classmethod + def _row_processor(cls, manager, fn, key): + impl = manager[key].impl + if impl.collection: + def _set_callable(state, dict_, row): + old = dict_.pop(key, None) + if old is not None: + impl._invalidate_collection(old) + state.callables[key] = fn + else: + def _set_callable(state, dict_, row): + state.callables[key] = fn + return _set_callable def _expire(self, dict_, modified_set): self.expired = True @@ -359,7 +364,7 @@ class InstanceState(interfaces._InspectionAttr): self.manager.dispatch.expire(self, attribute_names) - def __call__(self, passive): + def __call__(self, state, passive): """__call__ allows the InstanceState to act as a deferred callable for loading expired attributes, which is also serializable (picklable). diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 6984f4127a..7535a8c881 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -14,6 +14,7 @@ from . import ( attributes, interfaces, exc as orm_exc, loading, unitofwork, util as orm_util ) +from .state import InstanceState from .util import _none_set from .interfaces import ( LoaderStrategy, StrategizedOption, MapperOption, PropertyOption, @@ -181,9 +182,9 @@ class DeferredColumnLoader(LoaderStrategy): context, path, mapper, row, adapter) elif not self.is_class_level: - def set_deferred_for_local_state(state, dict_, row): - state._set_callable( - dict_, key, LoadDeferredColumns(state, key)) + set_deferred_for_local_state = InstanceState._row_processor( + mapper.class_manager, + LoadDeferredColumns(key), key) return set_deferred_for_local_state, None, None else: def reset_col_for_deferred(state, dict_, row): @@ -256,12 +257,11 @@ log.class_logger(DeferredColumnLoader) class LoadDeferredColumns(object): """serializable loader object used by DeferredColumnLoader""" - def __init__(self, state, key): - self.state = state + def __init__(self, key): self.key = key - def __call__(self, passive=attributes.PASSIVE_OFF): - state, key = self.state, self.key + def __call__(self, state, passive=attributes.PASSIVE_OFF): + key = self.key localparent = state.manager.mapper prop = localparent._props[key] @@ -602,16 +602,18 @@ class LazyLoader(AbstractRelationshipLoader): mapper, row, adapter): key = self.key if not self.is_class_level: - def set_lazy_callable(state, dict_, row): - # we are not the primary manager for this attribute - # on this class - set up a - # per-instance lazyloader, which will override the - # class-level behavior. - # this currently only happens when using a - # "lazyload" option on a "no load" - # attribute - "eager" attributes always have a - # class-level lazyloader installed. - state._set_callable(dict_, key, LoadLazyAttribute(state, key)) + # we are not the primary manager for this attribute + # on this class - set up a + # per-instance lazyloader, which will override the + # class-level behavior. + # this currently only happens when using a + # "lazyload" option on a "no load" + # attribute - "eager" attributes always have a + # class-level lazyloader installed. + set_lazy_callable = InstanceState._row_processor( + mapper.class_manager, + LoadLazyAttribute(key), key) + return set_lazy_callable, None, None else: def reset_for_lazy_callable(state, dict_, row): @@ -634,12 +636,11 @@ log.class_logger(LazyLoader) class LoadLazyAttribute(object): """serializable loader object used by LazyLoader""" - def __init__(self, state, key): - self.state = state + def __init__(self, key): self.key = key - def __call__(self, passive=attributes.PASSIVE_OFF): - state, key = self.state, self.key + def __call__(self, state, passive=attributes.PASSIVE_OFF): + key = self.key instance_mapper = state.manager.mapper prop = instance_mapper._props[key] strategy = prop._strategies[LazyLoader] diff --git a/test/aaa_profiling/test_orm.py b/test/aaa_profiling/test_orm.py index b9eeb83610..0bafd7f04a 100644 --- a/test/aaa_profiling/test_orm.py +++ b/test/aaa_profiling/test_orm.py @@ -2,7 +2,7 @@ from sqlalchemy.testing import eq_, assert_raises, \ assert_raises_message from sqlalchemy import exc as sa_exc, util, Integer, String, ForeignKey from sqlalchemy.orm import exc as orm_exc, mapper, relationship, \ - sessionmaker, Session + sessionmaker, Session, defer from sqlalchemy import testing from sqlalchemy.testing import profiling from sqlalchemy.testing import fixtures @@ -257,4 +257,56 @@ class MergeBackrefsTest(fixtures.MappedTest): ]: s.merge(a) +class DeferOptionsTest(fixtures.MappedTest): + @classmethod + def define_tables(cls, metadata): + Table('a', metadata, + Column('id', Integer, primary_key=True), + Column('x', String(5)), + Column('y', String(5)), + Column('z', String(5)), + Column('q', String(5)), + Column('p', String(5)), + Column('r', String(5)), + ) + + @classmethod + def setup_classes(cls): + class A(cls.Basic): + pass + + @classmethod + def setup_mappers(cls): + A = cls.classes.A + a = cls.tables.a + mapper(A, a) + + @classmethod + def insert_data(cls): + A = cls.classes.A + s = Session() + s.add_all([ + A(id=i, + **dict((letter, "%s%d" % (letter, i)) for letter in + ['x', 'y', 'z', 'p', 'q', 'r']) + ) for i in range(1, 1001) + ]) + s.commit() + + @profiling.function_call_count(variance=.10) + def test_baseline(self): + # as of [ticket:2778], this is at 39025 + A = self.classes.A + s = Session() + s.query(A).all() + + @profiling.function_call_count(variance=.10) + def test_defer_many_cols(self): + # with [ticket:2778], this goes from 50805 to 32817, + # as it should be fewer function calls than the baseline + A = self.classes.A + s = Session() + s.query(A).options( + *[defer(letter) for letter in ['x', 'y', 'z', 'p', 'q', 'r']]).\ + all() diff --git a/test/orm/test_attributes.py b/test/orm/test_attributes.py index 7b2bf0e3ee..dbe82f5851 100644 --- a/test/orm/test_attributes.py +++ b/test/orm/test_attributes.py @@ -11,12 +11,19 @@ from sqlalchemy.util import jython from sqlalchemy import event from sqlalchemy import testing from sqlalchemy.testing.mock import Mock, call +from sqlalchemy.orm.state import InstanceState # global for pickling tests MyTest = None MyTest2 = None + +def _set_callable(state, dict_, key, callable_): + fn = InstanceState._row_processor(state.manager, callable_, key) + fn(state, dict_, None) + + class AttributeImplAPITest(fixtures.MappedTest): def _scalar_obj_fixture(self): class A(object): @@ -602,8 +609,10 @@ class AttributesTest(fixtures.ORMTest): """ - class Post(object):pass - class Blog(object):pass + class Post(object): + pass + class Blog(object): + pass instrumentation.register_class(Post) instrumentation.register_class(Blog) @@ -618,10 +627,10 @@ class AttributesTest(fixtures.ORMTest): # create objects as if they'd been freshly loaded from the database (without history) b = Blog() p1 = Post() - attributes.instance_state(b)._set_callable(attributes.instance_dict(b), - 'posts', lambda passive:[p1]) - attributes.instance_state(p1)._set_callable(attributes.instance_dict(p1), - 'blog', lambda passive:b) + _set_callable(attributes.instance_state(b), attributes.instance_dict(b), + 'posts', lambda state, passive:[p1]) + _set_callable(attributes.instance_state(p1), attributes.instance_dict(p1), + 'blog', lambda state, passive:b) p1, attributes.instance_state(b)._commit_all(attributes.instance_dict(b)) # no orphans (called before the lazy loaders fire off) @@ -2628,7 +2637,7 @@ class TestUnlink(fixtures.TestBase): coll = a1.bs a1.bs.append(B()) state = attributes.instance_state(a1) - state._set_callable(state.dict, "bs", lambda: B()) + _set_callable(state, state.dict, "bs", lambda: B()) assert_raises( Warning, coll.append, B() diff --git a/test/profiles.txt b/test/profiles.txt index 3c22397bbb..7e5c808c17 100644 --- a/test/profiles.txt +++ b/test/profiles.txt @@ -1,15 +1,15 @@ # /Users/classic/dev/sqlalchemy/test/profiles.txt # This file is written out on a per-environment basis. -# For each test in aaa_profiling, the corresponding function and +# For each test in aaa_profiling, the corresponding function and # environment is located within this file. If it doesn't exist, # the test is skipped. -# If a callcount does exist, it is compared to what we received. +# If a callcount does exist, it is compared to what we received. # assertions are raised if the counts do not match. -# -# To add a new callcount test, apply the function_call_count -# decorator and re-run the tests using the --write-profiles +# +# To add a new callcount test, apply the function_call_count +# decorator and re-run the tests using the --write-profiles # option - this file will be rewritten including the new count. -# +# # TEST: test.aaa_profiling.test_compiler.CompileTest.test_insert @@ -59,6 +59,24 @@ test.aaa_profiling.test_compiler.CompileTest.test_update_whereclause 2.7_postgre test.aaa_profiling.test_compiler.CompileTest.test_update_whereclause 2.7_sqlite_pysqlite_cextensions 130 test.aaa_profiling.test_compiler.CompileTest.test_update_whereclause 2.7_sqlite_pysqlite_nocextensions 130 +# TEST: test.aaa_profiling.test_orm.DeferOptionsTest.test_baseline + +test.aaa_profiling.test_orm.DeferOptionsTest.test_baseline 2.7_mysql_mysqldb_cextensions 30052 +test.aaa_profiling.test_orm.DeferOptionsTest.test_baseline 2.7_mysql_mysqldb_nocextensions 39069 +test.aaa_profiling.test_orm.DeferOptionsTest.test_baseline 2.7_postgresql_psycopg2_cextensions 42032 +test.aaa_profiling.test_orm.DeferOptionsTest.test_baseline 2.7_postgresql_psycopg2_nocextensions 51049 +test.aaa_profiling.test_orm.DeferOptionsTest.test_baseline 2.7_sqlite_pysqlite_cextensions 30008 +test.aaa_profiling.test_orm.DeferOptionsTest.test_baseline 2.7_sqlite_pysqlite_nocextensions 39025 + +# TEST: test.aaa_profiling.test_orm.DeferOptionsTest.test_defer_many_cols + +test.aaa_profiling.test_orm.DeferOptionsTest.test_defer_many_cols 2.7_mysql_mysqldb_cextensions 29994 +test.aaa_profiling.test_orm.DeferOptionsTest.test_defer_many_cols 2.7_mysql_mysqldb_nocextensions 32867 +test.aaa_profiling.test_orm.DeferOptionsTest.test_defer_many_cols 2.7_postgresql_psycopg2_cextensions 29830 +test.aaa_profiling.test_orm.DeferOptionsTest.test_defer_many_cols 2.7_postgresql_psycopg2_nocextensions 32835 +test.aaa_profiling.test_orm.DeferOptionsTest.test_defer_many_cols 2.7_sqlite_pysqlite_cextensions 29812 +test.aaa_profiling.test_orm.DeferOptionsTest.test_defer_many_cols 2.7_sqlite_pysqlite_nocextensions 32817 + # TEST: test.aaa_profiling.test_orm.LoadManyToOneFromIdentityTest.test_many_to_one_load_identity test.aaa_profiling.test_orm.LoadManyToOneFromIdentityTest.test_many_to_one_load_identity 2.5_sqlite_pysqlite_nocextensions 17987