msg117373 - (view) |
Author: Armin Ronacher (aronacher) *  |
Date: 2010-09-25 14:32 |
findCaller() on loses case information on the files on Windows and has in general a really bad performance. The attached patch does not depend on filename comparisions and instead compares the object identity of the caller's global namespace against the one from the logging module. |
|
|
msg117425 - (view) |
Author: Vinay Sajip (vinay.sajip) *  |
Date: 2010-09-26 17:04 |
Your basic fix looks fine, but there's a couple of other issues to consider: 1. Users can use _srcFile = None to avoid calling findCaller() altogether, so I can't do away with the _srcFile altogether as it may cause some issues with existing code. 2. If using e.g. a LoggerAdapter derived class, there needs to be some way in which you can skip (in addition to the logging package itself) other user-defined modules until you get to the appropriate stack frame. I'm thinking about these, but if you have any ideas about these, I'm listening :-) |
|
|
msg117428 - (view) |
Author: Armin Ronacher (aronacher) *  |
Date: 2010-09-26 20:02 |
> 1. Users can use _srcFile = None to avoid calling findCaller() > altogether, so I can't do away with the _srcFile altogether as it may > cause some issues with existing code. That is very undocumented behaviour and relying on that sounds like a terrible idea. > 2. If using e.g. a LoggerAdapter derived class, there needs to be > some way in which you can skip (in addition to the logging package > itself) other user-defined modules until you get to the appropriate > stack frame. First of all that should not be a global setting, secondly this is currently not possible in logging either. A better solution would be to do what warnings does and making it possible provide a stacklevel to the caller finder. |
|
|
msg117429 - (view) |
Author: Vinay Sajip (vinay.sajip) *  |
Date: 2010-09-26 21:41 |
> That is very undocumented behaviour and relying on that sounds like a terrible idea. Agreed, I'm just fretting about it ... > First of all that should not be a global setting, secondly this is currently not possible in logging either. True, but you have pointed out in the past that this *is* a problem - so it would be good to solve it. > A better solution would be to do what warnings does and making it possible provide a stacklevel to the caller finder. I'm not that familiar with how warnings does it, but I will take a look. Thanks for the suggestions. BTW re. your patch - where you say "os.co_filename", do you mean "co.co_filename"? |
|
|
msg117528 - (view) |
Author: Vinay Sajip (vinay.sajip) *  |
Date: 2010-09-28 15:12 |
I made a temporary change to logging to time the two different approaches: see this gist: http://gist.github.com/601162 According to these findings, the use of module globals seems to make only a marginal difference: vinay@eta-jaunty:~/projects/scratch$ python3.2 timefc.py filename_comparison 50.61 microseconds module_globals 49.56 microseconds vinay@eta-jaunty:~/projects/scratch$ python2.7 timefc.py filename_comparison 50.79 microseconds module_globals 50.01 microseconds Have I missed something obvious? |
|
|
msg118346 - (view) |
Author: Vinay Sajip (vinay.sajip) *  |
Date: 2010-10-10 21:09 |
Partial fix checked into py3k and release27-maint branches (for losing filename case information) - r85361. Regarding performance changes - awaiting feedback from Armin re. my performance measurements showing only marginal differences. |
|
|
msg185422 - (view) |
Author: Georg Brandl (georg.brandl) *  |
Date: 2013-03-28 10:20 |
Closing due to lack of response. |
|
|