bpo-32512: Add -m option to profile for profiling modules#5132
Conversation
mariocj89
changed the title
mariocj89
changed the title
mariocj89
force-pushed
the
profile_module
branch
from
09ed95e to
06d56f4
Compare
| from optparse import OptionParser | ||
|
|
||
| usage = "profile.py [-o output_file_path] [-s sort] scriptfile [arg] ..." | ||
| usage = "profile.py [-o output_file_path] [-s sort] [-m module |escriptfile] [arg] ..." |
There was a problem hiding this comment.
Could you upgrade the documentation (Doc/library/profile.rst) and add a ..versionadded:: 3.8 ?
There was a problem hiding this comment.
Thanks! addressed 😄
|
Updated, let me know how it looks for you @matrixise. If the wording and structure looks ok we can see if any core developer is interested in this. It is a "nice to have change" as it is mainly useful for people not having cProfile available but not really urgent. |
matrixise
left a comment
matrixise
left a comment
There was a problem hiding this comment.
I am fine with this PR. Thank you so much for your job. Have a nice day.
|
Thanks for your review @matrixise ;) |
|
no problem, but I am not a core dev, you have to wait and see with a core review, maybe there will an other point to fix, but in this case, it's good adventure ;-) thanks again ;-) |
|
As a note for the reviewer, this follows the same implementation as the one in This, therefore, includes by default the functions of using The change (if desired) to move to use the source directly can be done either in this PR or in a different one, but that will require either hitting the private API in runpy, see #5474 as an example. |
mariocj89
force-pushed
the
profile_module
branch
from
2d18a74 to
a70f8b1
Compare
|
@ncoghlan we stalled this PR based on one where we were going to refactor part of the logic to extract the module details in runpy, but we agreed here that it probably makes no sense to do so. Do you want to proceed with this PR as is? Let me know if you think it is not worth or any change should be done. I am happy to close it otherwise if you want. (really, no hard feelings I promise 😊) I am also happy to have a look to the issue that this implementation brings as commented in [here]#5132 (comment)) in a latter PR. And "fix" both cProfile and profile if desired. But I'd prefer to leave that conversation for a different issue + PR. Same goes about #5134 |
|
@mariocj89 Yeah, let's go ahead with updating the existing PRs and at least offering the functionality, even if it interacts a bit differently with the interpreter level "-i" switch when compared to the directly executing a path. |
The new option in the CLI of the profile module allow to profile executable modules. This change follows the same implementation as the one already present in `cProfile`.
As the argument is now present on both modules, move the tests to the common test case to be run with profile as well.
mariocj89
force-pushed
the
profile_module
branch
from
a70f8b1 to
60a37c7
Compare
|
@ncoghlan, updated per previous comment, let me know of any change that needs to be done here :) Thanks! I've also updated #5134 😄 |
|
@ncoghlan: Please replace |
5 participants
Port of #4297 to the pure python profile module.
The patch adds a new option -m that allows the profile CLI to run modules as scripts.
https://bugs.python.org/issue32512