bpo-21862: Add -m option to cProfile for profiling modules #4297
Conversation
Lib/cProfile.py
Outdated
| code = "runpy.run_module(modname, run_name='__main__')" | ||
| globs = { | ||
| 'runpy': runpy, | ||
| 'modname': args[0] |
There was a problem hiding this comment.
A potential simplification here:
code = f"run_module({modname}, run_name='__main__')"
globs = {
'run_module': runpy.run_module
}
What's currently there isn't wrong, but I think the above would be slightly clearer.
Setting alter_sys=True would also be an option, but cProfile doesn't do that for regular scripts, so I don't see any particular reason for it to do it for modules.
There was a problem hiding this comment.
Sure, I'll fix that and also add NEWS entry :)
There was a problem hiding this comment.
Hey @ncoghlan !
So, one of the thing I tried from your code snippet was this:
if options.module:
modname = args[0]
code = f"run_module({modname}, run_name='__main__')"
globs = {
'run_module': runpy.run_module
}
and if I run ./python.exe -m cProfile -m timeit -n 1 to test it, it throws NameError for timeit. But this is not the case without f-strings. I'm unable to understand this behaviour. Can you please help?
There was a problem hiding this comment.
This is not the case if I avoid using f-string and provide the context of running the module as globals to the runctx method. So, I'm not sure, what is wrong. Help would be much appreciated.
There was a problem hiding this comment.
I think modname would need to be fed through repr(), e.g. f"run_module({modname!r}, run_name='__main__')".
That said, I think your original code is fine and I don't find @ncoghlan's suggestion more readable :-)
There was a problem hiding this comment.
@pitrou is right, and the missing !r was just a bug in my suggested code. However, there's merit in the idea of using just one mechanism for passing values in (the globs dict) rather than two (string interpolation and the globs dict).
So while I still think it makes sense to pass in a reference to the run_module itself, rather than one to the runpy module, I withdraw the suggestion of using string interpolation to pass in the module name to run.
|
|
||
| ``-o`` writes the profile results to a file instead of to stdout | ||
|
|
||
| ``-s`` specifies one of the :func:`~pstats.Stats.sort_stats` sort values to sort | ||
| the output by. This only applies when ``-o`` is not supplied. | ||
|
|
||
| ``-m`` specifies that a module is being profiled instead of a script. | ||
|
|
There was a problem hiding this comment.
You can add a versionadded tag just after this for the -m option.
Doc/library/profile.rst
Outdated
| @@ -123,13 +123,15 @@ them in various ways. | |||
| The file :mod:`cProfile` can also be invoked as a script to profile another | |||
| script. For example:: | |||
|
|
|||
| python -m cProfile [-o output_file] [-s sort_order] myscript.py | |||
| python -m cProfile [-o output_file] [-s sort_order] [-m module] myscript.py | |||
There was a problem hiding this comment.
Note that -m module and myscript.py are exclusive. So the doc could/should be spelled (-m module | myscript.py).
Lib/test/test_cprofile.py
Outdated
| assert_python_failure('-m', 'cProfile', '-m') | ||
|
|
||
| # Test failure for not-existent module | ||
| assert_python_failure('-m', 'cProfile', 'random_module_xyz.py') |
There was a problem hiding this comment.
This should be:
assert_python_failure('-m', 'cProfile', '-m', 'random_module_xyz')
|
Thanks for fixing the tidbits @pitrou |
|
You're welcome. Sometimes it's faster than having to go back and forth :-) |
* bpo-21862: Add -m option to cProfile for profiling modules
Solves issue https://bugs.python.org/issue21862
https://bugs.python.org/issue21862