On Aug 14, 2007, at 12:38 PM, svilen wrote:
> > > performance-wise - do u have any test/target for profiling? else i can > repeat some tests i did somewhen in february (if i remember them..) look in test/perf for some, just added a "mass-insert/mass-select" test. also the current branch im doing today will chop out about 60% of function call overhead from the abovementioned test. > ------- > database/* > get_col_spec(self) etc: > these string-formats may be better without the artificial dict, > eg. return self._extend("CHAR(%(length)s)" % {'length': self.length}) > -> return self._extend("CHAR(%s)" % self.length ) > or > -> return self._extend("CHAR(%(length)s)" % self.__dict__ ) > no idea if get_col_spec() is used that much to have crucial impact on > speed though, at least it looks simpler. im fine with that (not crucial tho) > ------- > orm.util.AliasedClauses._create_row_adapter() > class AliasedRowAdapter( object): > 1. can't this be made as standalone class, returning an instance, > initialized with the map, which is then __call__()ed ? is it faster to say "self.map" or to say "map" from locals() ? its probably not very crucial either way. > 2. this can be faster if: > a) has_key = __contains__ #instead of yet another funccall > b) __getitem__ uses try except instead of double lookup key in map im not sure try/except is faster here - im pretty sure a missing key is likely and exception throws are very expensive. would be worth a try to see if the missing key exception actually occurs here. > > ------- > orm.mapper > Mapper._instance(): > WTF is the **{'key':value, ... } ? > eg. if extension.populate_instance(self, context, row, instance, > **{'instancekey':identitykey, 'isnew':isnew}) ... > same thing is done as separate variable a page later; > btw there are several of these **{} in the file i think thats a product of a refactoring where the ** was originally not there. > > also, Mapper._options is redundant (leftover?) neverused yeah gone along with the **{} in r3303 > > > ------- > orm.attribute > AttributeManager.init_attr(): > the saving this one eventualy does is too small, compared to a > property call of ._state. i havent benched this in a while but my recollection is that the AttributeError raise is *much* slower than pre-calling this method. a single function call is always faster than an exception throw. however, i see that the exception throw is being suppressed also with a hasattr() being called every time....im not sure why thats that way now so i might change it back to throwing AttributeError. > > AttributeManager.register_attribute(): > the def _get_state(self) that is made into as property _state can > be made eventualy faster with try-except instead of 'if'. exception throws are slower. but like above says, the throw here can be prevented if init_attr() is called. > > btw: cant that ._state property be removed alltogether (i.e. made a > plain attribute? then init_attr() MUST be there seting it up as plain > dict. it should be named something non-collisionworthy such as the current _sa_attr_state. > > ------- > orm/unitofwork > UOWTask._sort_circular_dependencies(): > def get_dependency_task(obj, depprocessor): > try: > dp = dependencies[obj] > except KeyError: > dp = dependencies.setdefault(obj, {}) > isnt just the setdefault() enough? this should just say dependencies[obj] = dp = {} > > ------- > engine.url. > def translate_connect_args(self, names): > this assumes the order of passed names matches the order of > attribute_names inside... very fragile. Why not use set of kwargs > like (attr_name=replacement_name defaulting to None), then just use > the non empty ones? patches welcome > def _parse_rfc1738_args(): > the 'opts' dict is redundant, would be cleaner if args are just > passed to URL( name=value) this is an old patch we got from someone, improvement patches welcome > > ------- > topological.py > QueueDependencySorter.sort(): > 'cycles' is redundant neverused variable would look good in the patch too > > ------- > util: > ThreadLocal: > - wouldnt be faster if the key in the _tdict is tuple(id,key) and > not some formatted string off these? or the key is nonhashable? good catch, should be patched > - the engine/threadlocal.TLEngine._session() issues a hasattr() on > such object. how does it actualy work? IMO it always fails patch which includes a test case to add into test/engine/ transaction.py would be welcome --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---