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
-~----------~----~----~----~------~----~------~--~---

Reply via email to