Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cProfile command-line should accept "-m module_name" as an alternative to script path #66061

Closed
pitrou opened this issue Jun 24, 2014 · 20 comments
Closed
Labels
3.7 easy stdlib type-feature

Comments

@pitrou
Copy link
Member

@pitrou pitrou commented Jun 24, 2014

BPO 21862
Nosy @birkenfeld, @amauryfa, @ncoghlan, @pitrou, @berkerpeksag, @CuriousLearner
PRs
  • #4297
  • Files
  • cProfile-add-new-option-module.patch: patch
  • 21862.patch
  • cProfile_module_option.patch: patch
  • cProfile_module_option_2.patch: patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2017-11-08.10:51:30.237>
    created_at = <Date 2014-06-24.20:06:54.143>
    labels = ['3.7', 'easy', 'type-feature', 'library']
    title = 'cProfile command-line should accept "-m module_name" as an alternative to script path'
    updated_at = <Date 2017-11-08.11:21:16.579>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2017-11-08.11:21:16.579>
    actor = 'CuriousLearner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-11-08.10:51:30.237>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2014-06-24.20:06:54.143>
    creator = 'pitrou'
    dependencies = []
    files = ['35775', '36094', '37749', '37885']
    hgrepos = []
    issue_num = 21862
    keywords = ['patch', 'easy']
    message_count = 20.0
    messages = ['221488', '221502', '221546', '224008', '232932', '234195', '234560', '234614', '234863', '238195', '305666', '305667', '305668', '305714', '305720', '305741', '305742', '305819', '305820', '305822']
    nosy_count = 9.0
    nosy_names = ['georg.brandl', 'amaury.forgeotdarc', 'ncoghlan', 'pitrou', 'gennad', 'berker.peksag', 'rock', 'oquanox', 'CuriousLearner']
    pr_nums = ['4297']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue21862'
    versions = ['Python 3.7']

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Jun 24, 2014

    As the title says. You should be able to type:

    $ python -m cProfile -m my.module.name

    to profile execution of my.module.name.

    @pitrou pitrou added stdlib easy type-feature labels Jun 24, 2014
    @Rock
    Copy link
    Mannequin

    @Rock Rock mannequin commented Jun 24, 2014

    I tweaked the Lib/cProfile.py a little bit to get the feature done, please review the patch attached.

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Jun 25, 2014

    That's not how -m should work. It should use the runpy facilities (see the runpy module). Otherwise it probably won't work properly with qualified module names ("-m pkg.subpkg.mod").

    @gennad
    Copy link
    Mannequin

    @gennad gennad mannequin commented Jul 25, 2014

    I added runpy import mechanism

    @amauryfa
    Copy link
    Member

    @amauryfa amauryfa commented Dec 19, 2014

    The patch does not seem to allow parameters after the -m option.
    I'm sure this restriction can be lifted.

    @oquanox
    Copy link
    Mannequin

    @oquanox oquanox mannequin commented Jan 17, 2015

    Now allows parameters after the -m option.

    @amauryfa
    Copy link
    Member

    @amauryfa amauryfa commented Jan 23, 2015

    This change will cause the module to be imported twice:
    progname = runpy.run_module(args[0])['__file__']
    ... and then the runctx() call.

    What about something like:
    code = "runpy.run_module(modname, run_name='__main__')"
    globs = { 'runpy': runpy, 'modname': args[0] }

    @ncoghlan
    Copy link
    Contributor

    @ncoghlan ncoghlan commented Jan 24, 2015

    I haven't looked into the details of how cProfile handles command line arguments, but potentially relevant is issue bpo-19982, which proposes adding a "target" parameter that lets runpy run in the context of an existing module object, rather than always creating its own.

    However, if this can be implemented without that, go ahead - it can always be refactored later.

    @oquanox
    Copy link
    Mannequin

    @oquanox oquanox mannequin commented Jan 27, 2015

    Updated the patch and added docs and tests.

    @oquanox
    Copy link
    Mannequin

    @oquanox oquanox mannequin commented Mar 16, 2015

    Could someone please review the patch. Thanks.

    @pitrou pitrou added the 3.7 label Nov 6, 2017
    @CuriousLearner
    Copy link
    Member

    @CuriousLearner CuriousLearner commented Nov 6, 2017

    Hey, seems like this bug is not updated for a while. Can I work on this?

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Nov 6, 2017

    Sanyam, you definitely can. Step #1 would probably be to port the patch to git master and turn it into a GitHub PR.

    @CuriousLearner
    Copy link
    Member

    @CuriousLearner CuriousLearner commented Nov 6, 2017

    Thanks for the heads up! I'll do that ;)

    @ncoghlan
    Copy link
    Contributor

    @ncoghlan ncoghlan commented Nov 7, 2017

    Interesting - I'd never looked at how cProfile works before, and the fact it already doesn't support the "-i" (interactive) switch makes it much simpler to handle than the pdb case.

    So from a runpy perspective, this approach gets a +1 from me, but I'd prefer if there was someone more familiar with the cProfile module that was willing to handle the merge.

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Nov 7, 2017

    Le 07/11/2017 à 08:48, Nick Coghlan a écrit :

    So from a runpy perspective, this approach gets a +1 from me, but I'd prefer if there was someone more familiar with the cProfile module that was willing to handle the merge.

    Thanks Nick. Can you confirm the runpy invocation in the PR is sane?

    @ncoghlan
    Copy link
    Contributor

    @ncoghlan ncoghlan commented Nov 7, 2017

    I added an inline comment on the PR - I think what's there now would work fine, but I also suggested a slightly shorter and clearer (at least to me) alternative.

    @ncoghlan
    Copy link
    Contributor

    @ncoghlan ncoghlan commented Nov 7, 2017

    Also, based on reviewing this, I suspect the same approach would also work for the pure Python profile module.

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Nov 8, 2017

    New changeset 7973e27 by Antoine Pitrou (Sanyam Khurana) in branch 'master':
    bpo-21862: Add -m option to cProfile for profiling modules (bpo-4297)
    7973e27

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Nov 8, 2017

    This is done. Thank you Sanyam!

    @pitrou pitrou closed this Nov 8, 2017
    @CuriousLearner
    Copy link
    Member

    @CuriousLearner CuriousLearner commented Nov 8, 2017

    Thanks a lot Antoine and Nick :)

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 easy stdlib type-feature
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants