ENH: auto-profile module execution

[TTsangSC]

Motivation

In issue #29, it was requested that support for running modules be added to kernprof via an -m flag. While it was noted that explicit decoration with the LINE_PROFILE=1 environment-variable switch can mitigate having to call kernprof off the command line, this does not cover the use-case of auto-profiling module execution, like how the existing invocation kernprof -l -p... script.py covers script execution.

Code changes

This PR adds the requested (but see Caveats) functionality by:

• Adding the -m/--module EDIT 13 Apr: long-option form no longer supported boolean flag to kernprof.py::main()
  • Convenience: when -m is passed without specifying -p/--prof-mod, it defaults to -p {<the_specified_module>} EDIT 12 Apr: this is no longer the case, users should supply -p<module> explicitly
  • (EDIT 13 Apr) Syntactics for kernprof -m updated to behave more like python -m; kernprof -m now:
    • Complains if there isn't a trailing argument (the module name) which isn't --
    • Terminates parsing for the arguments after the module name, passing them to the module as positional args
• Adding kernprof.py::find_module_script(), which leverages line_profiler/autoprofile/util_static.py::modname_to_modpath() to convert module/package dotted paths to their corresponding file paths; packages are detected and referred to the appropriate __main__.py
• Adding a line_profiler/autoprofile/run_module.py::AstTreeModuleProfiler, which rewrites relative imports into absolute imports, so that the resultant ASTs can be compile()-ed and subsequently exec()-ed as-is.
• Adding the parametrized tests tests/test_autoprofile.py:: test_autoprofile_exec_{package,module}() for the new functionality (kernprof -l ... -m {<some_package_with_a_main_py>} and kernprof -l ... -m {<some_module>})
• (EDIT 17 Mar) Adding the parametrized test tests/test_explicit_profile.py::test_explicit_profile_with_kernprof_m() for the corresponding cases without the -l/--line-by-line flag
• (EDIT 12 Apr) Made kernprof.py::main() clean up after itself by restoring sys.path and sys.argv after its execution, as well as undoing the line_profiler.profile._kernprof_overwrite() with the LineProfiler instance created inside main()
• (EDIT 13 Apr) Adding the parametrized test tests/test_kernprof.py::test_kernprof_m_parsing() to test the special-casing of the -m flag (which must follow all other kernprof options and and be followed by a module name)

Caveats

While this PR technically covers #29's use-case, in that kernprof -l -m pytest {<tests>} does profile whatever is already decorated with @profile in the tests, we still aren't auto-profiling tests, since it is the pytest module that would have been auto-profiled. Actually implementing auto-profiling for pytest tests (which pytest-line-profiler does not handle, owing to how auto-profiling did not exist when the package was last updated) may be nontrivial, due to how pytest also does its own AST rewrites.

[Erotemic]

Still need to look at this in more detail. But I'm liking what I see so far.

[Erotemic]

suiifx

typo?

[TTsangSC]

... yikes. Thanks for the note; fixed just now.

On the topic of typos though... Just noticed that the files line_profiler/autoprofile/ast_profle_transformer.py[i] might have missed an i in the name. That could probably be fixed while retaining backwards compatibility by:

• Migrating ast_profle_transformer.py[i] to ast_profile_transformer.py[i]
• Adding module-level __getattr__() and __dir__() at ast_profle_transformer.py to alias to the relocated module, and also to issue DeprecationWarnings or FutureWarnings when it is used.
  • The former is probably enough, since kernprof is a dev-tool, and DeprecationWarnings not being visible to end-users by default is probably not a concern.
  • And maybe the typo-ed module alias can be dropped in v4.5.0 or something.
• Rewriting ast_profle_transformer.pyi accordingly.

If you deem this worthy of a fix, I'll spin it off into its own issue and submit another PR for that.

One concern though is how module-level attribute access customization (PEP 562) is Python 3.7+, while according to the pyproject.toml and requirements/*.txt we may be supporting as far down to Python 3.6. That said, anything beneath 3.9 has already been EoL-ed, so maybe it isn't an issue? (Apropos, maybe we can also have a CONTRIBUTING.txt for the expected scope of support for new features, among other things?)

[Erotemic]

We can remove support for <3.9. I tend to support EOL python versions until there is any friction, and then I drop them. I'll take care of removing older python support as I have scripts for that. I'll put up a PR. (EDIT: I only see support up to 3.8 in pyproject.toml and setup.py, not sure where you see 3.6)

ast_profle_transformer

Ooph, I had to read your comment about 4 times before I saw the spelling error. I don't think anyone is explicitly importing the ast_profle_transformer, so we be backwards incompatible in that corner case. If you want to fix that, a separate PR would be best.

With all these changes we can bump to 4.3.0, add a note in the CHANGELOG and call it good enough.

[TTsangSC]

I see, thanks for the clarification. Was a bit confused because pyproject.toml::[build-system.requires] and the requirements/*.txt still have lines reading {<pkg>}; python_version < '3.8' and python_version >= '3.6', but I missed the most important python_requires in setup.py.

Just submitted #325 for the typo.

Thanks for taking care of matters.

[TTsangSC]

Hi, I noticed that you've assigned the PR to yourself. Does this mean you're still in the progress of reviewing it? If you're done, should I just rebase -> add changelog -> force-push now? Cheers.

[Erotemic]

Please do the rebase, changelog, etc...

I assigned myself so I don't lose track of it. I've given it a baseline read, and it looks good, but I need to find time to step through it in detail and try it out myself.

[TTsangSC]

Hi @Erotemic, I need some help with this... weird stuff going on here.

• Somewhere like two commits ago (after rebasing) I noticed that the existing -m flag doesn't cover when we don't use autoprofile (i.e. if we kernprof -m without -l), so I went ahead and reduplicated the non-autoprofile logics in kernprof.py by replacing kernprof.execfile(script_file, ...) with the equivalent runpy.run_module(options.script, ...) calls.
• Currently tests/test_explicit_profile.py::test_explicit_profile_with_kernprof_m[False-*] (a new test) is the only test going thru this code path. Strangely enough, it passes on my machine but fails in CI (run #1, run #2), saying that "Another profiling tool is already active" in the ContextualProfile.enable_by_count() call.
  • I tried various solutions (like swapping out globals() with None as above), but in both cases the test invariably passed on local and failed on CI. This is also weird, given how the line_profiler.profile._kernprof_overwrite(prof) call some 40 lines above is supposed to (I assume) have replaced the global cProfile.Profile object with prof.
  • Also tried both editable and non-editable installs, but more of the same.

So I figured that perhaps its just my new code that is subtly wrong, and I just needed to try harder to resolve it. Out of curiosity though, I tried to crash the else: branch below (which again I borrowed the code from) by inserting a raise Exception. To my horror though nothing changed – all tests still passed on local, which means that the block

else:
    prof.runctx('execfile_(%r, globals())' % (script_file,), ns, ns)

has actually not been covered by any test:

• line_profiler.py only has a coverage of 55% on CI, which is lower than the 63% I got locally (see screenshot below) – which didn't cover runctx().
• After explicitly combing thru the test suite, the only other tests which "uses" both explicit profiling and kernprof.py seem to be:
  • tests/test_explicit_profile.py::test_explicit_profile_with_kernprof()
  • tests/test_complex_case.py::test_varied_complex_invocations()

Unfortunately, both the above tests use kernprof -l, which sets options.builtin = True and thus don't actually go to that branch.

Given the above, I suspect there's something wrong with line_profiler.LineProfiler.runctx() that we didn't know of and never tested until now. But given that the test does pass on local, it's incredibly hard for me to debug on my own. Any pointers? And since you're already testing out this PR (presumably on your machine), can you also take a closer look at this in particular? Cheers.

[Erotemic]

To confirm: you're saying that there is an existing bug

I probably won't have time to review in depth in the near future. It's going to be on a weekend when I don't have other things going on.

It does look like some of the python versions are passing, but it's failing on 3.12. Is that the version you are testing with locally?

Try using cibuildwheel locally to run the 3.12 build / tests to see if it reproduces:

CIBW_BUILD="cp312-*" cibuildwheel --config-file pyproject.toml --platform linux --arch x86_64

Oh, but now I see in your comment an sdist build also failed, which shouldn't be using cibuildwheel..., but that's also using 3.13, which might have other issues. I just switched my local env to 3.13, and I've found a bunch of oddities come up from changes due to PEP 667.

You can try disabling python versions to test in the CI to make things faster. You can also likely use docker to get a clean local state an mimic the CI environment. I haven't used it in awhile, but I think act can run github action pipelines locally.

[TTsangSC]

(EDITS 19 Mar): typos

you're saying that there is an existing bug

Could be. Maybe I can even try to jury-rig a test for the aforementioned, currently-untested runctx() code path just to see what happens... if that works out, we'd also have more coverage.

won't have time to review in depth in the near future

Understandable, please take your time. I apologize if at any point of the PR I was a bit pushy/overbearing/etc.

Is that the version you are testing with locally?

Locally I'm on 3.13, which worked like a charm. But I do have everything from 8 to 13 (except 10 for whatever reason), so maybe I'll use each of them to fuzz the tests.

PEP 667

Thanks for the tip, that could very well be the issue. locals() did show some expected (EDIT 19 Mar: unexpected) behaviors when I tinkered with the PR and maybe that was (part of) what went wrong.

cibuildwheel
docker

Not familiar with those but will try to look into them. Seem like good tools to have in the box...

Anyhow, thanks for the suggestions. Let's keep ourselves update (EDIT 19 Mar: updated) here if anything new comes up.

[Erotemic]

Could be. Maybe I can even try to jury-rig a test for the aforementioned, currently-untested runctx() code path just to see what happens... if that works out, we'd also have more coverage.

If you find one and can add it independently as a separate PR, that would be valuable. You don't have to fix it in a separate PR, but something minimal that just shows it is an existing problem might be a good first step. If it is a non-trivial amount of work, then don't worry about it, but that's what I would do if I saw this behavior.

cibuildwheel will let you test all of the python versions on your local machine using docker. I also have a helper script in my xdev package, that can help. If you pip install xdev, then cd into the line profiler directory and run:

xdev freshpyenv --image=python:3.12

Which will run this script.

It will drop you into a docker minimal container with 3.12 as the main python. You will be in a clone of your local repo state (i.e. if you commit something on your local host, and then pull in the docker container you can get the changes without having to restart the image).

From there you can:

pip install -e .[all-strict] -v

To install line-profiler and minimal test dependencies in development mode, then you can run pytest. As previously mentioned, if you make a change, then just commit it (no need to push). The repo in the container points to a volume mount of your local repo, so it can pull in the changes.

I typically develop in a local environment on my host system, but when I get tricky things like these, using docker can help isolate the problem from any confounding factors on my host system.

EDIT: I was able to reproduce the error on 3.12 this way.

[TTsangSC]

Hi, please forgive me for the tangent, but I've been working on some tool of my own that uses this new AstTreeModuleProfiler added here. One corner case that I've noticed is that it still errors on relative imports in __init__.py, because the resolved module name there is that of the containing package, without the trailing .__init__ (so if foo/bar/__init__.py does a from . import baz, it is rewritten to from foo import baz instead of the intended from foo.bar import baz).

Now the fix is super easy, we can always just special-case __init__.py files in the code to resolve to the correct namespace, like we're already doing for __main__.py to support running packages. However, it may also be gratuitous for our use-case, because __init__.pys are normally not the file run with python -m, except if someone tries to be cute and do python -m something.__init__, which is probably a bad idea anyway.

Do you reckon that I should also patch this corner case up in this PR, or leave the feature as-is, and go back to fix the important bug that we've been discussing? Cheers.

[Erotemic]

Do the modname_to_modpath and modpath_to_modname functions in line_profiler/autoprofile/util_static.py not handle this?

I don't think there are any ambiguities in implementing this correctly, so might as well do it.

[TTsangSC]

I just rebased on #326 (which itself has been rebased on the newest main), and implemented the fix for the __init__.py. Is it safe for me to force-push now or should I wait for that PR to be merged and rebase again?

[TTsangSC]

Update: to facilitate review I've pushed the rebased changes to a new branch at https://github.com/TTsangSC/line_profiler/commits/autoprofile-module-review/. Here's a summary:

• 0352568: Fixes the __init__.py corner case.
• 982aa23: Fixes the docstring of line_profiler/autoprofile/ast_profile_transformer.py::ast_create_profile_node(), which has documented the wrong arguments since 257afa7.

The two commits below concern adjacent features (kernprof -c and kernprof - (i.e. from stdin)) to the one in the initial scope of the PR (kernprof -m); please accept/reject at your discretion:

• 97e6094:
  • Adds -c to kernprof as a parallel for python -c ..., writing the received command(s) to a tempfile for profiling.
  • Adds special-casing for the script name -, writing the stdin to a tempfile for profiling.
  • Updates the CHANGELOG.
• 4cb118d: Adds two new tests tests/test_autoprofile.py::test_autoprofile_from_{stdin,inlined_script}() for the above commit.

If you find these OK after reviewing, I'll rebase and force-push to the current branch; cheers.

[TTsangSC]

@Erotemic

Just rebased and force-pushed. Sorry for it taking this long, I was confused by how my new test test_explicit_profile_with_kernprof_m() failed after rebasing for the non--b cases. But upon closer inspection those subtests probably shouldn't have passed as-is anyway, since runctx() implies wholesale profiling, while the test explicitly checked that only the functions decorated with @profile are profiled. I've since updated/fixed the test and now it should be alright.

Since you mentioned needing the feature for something, I guess it's for the best that I keep the aforementioned kernprof -c and kernprof - changes out of this PR, in the interest of quick review. Once this has been merged, I'll write a separate PR for those. Cheers.

[codecov]

Codecov Report

Attention: Patch coverage is 97.91667% with 1 line in your changes missing coverage. Please review.

Project coverage is 65.37%. Comparing base (56c2614) to head (000a366).
Report is 19 commits behind head on main.

Files with missing lines	Patch %	Lines
line_profiler/autoprofile/run_module.py	97.67%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #323      +/-   ##
==========================================
+ Coverage   62.45%   65.37%   +2.91%     
==========================================
  Files          12       13       +1     
  Lines         855      901      +46     
  Branches      186      196      +10     
==========================================
+ Hits          534      589      +55     
+ Misses        268      262       -6     
+ Partials       53       50       -3     

Files with missing lines	Coverage Δ
...ne_profiler/autoprofile/ast_profile_transformer.py	80.48% <ø> (+4.87%)	⬆️
line_profiler/autoprofile/autoprofile.py	100.00% <100.00%> (ø)
line_profiler/autoprofile/run_module.py	97.67% <97.67%> (ø)

... and 3 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d430120...000a366. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Erotemic]

I've been thinking about this a bit. I'm not sure that we want -m to run a module as a main script and auto-profile the module.

The way I think about -m is just that we are running a module as a main script. Often we do want to auto-profile that module too, but sometimes we might not. We may want to use the profile decorator or profile some other module that the main script calls. The purpose of -m is to avoid having to deal with paths to script files, instead we can use Python's PYTHONPATH to resolve where the module is.

If we do want to auto-profile and run ourmod, perhaps it's better to do kernprof -m ourmod -p ourmod, where the -m is simply telling it what to run as the main script, -p does the work of telling it what to auto-profile.

Granted, this starts to get burdensome, but I think it's the correct way to do it as it does not hinder expressiveness. I think we can also ease this burden with using pyproject.toml configuration. I can envision a section:

[tool.line_profiler]
prof-mod = ["mymod"]

to control what is automatically profiled in a controlled way.

Let me know what you think.

Also, I have this branch checked out, and I seem unable to even run kernprof with -m. I setup a demo directory structure:

├── invoke.sh
├── mymod
│   ├── helpers.py
│   ├── __init__.py
│   └── __main__.py
└── run_mymod.py

And in invoke.sh I have kernprof -l -m mymod, which says that it cannot find mymod. It's also the case that auto-profiling the helper functions aren't working with:

# Test auto-profile kernprof with an path invocation
kernprof -p mymod -p mymod.helpers -p mymod.__init__ -l run_mymod.py
python -m line_profiler -rmt "run_mymod.py.lprof"

or

kernprof -p mymod,mymod.helpers -l run_mymod.py

Attached is the demo I setup: autoprofile-demo.zip

[TTsangSC]

Fair point, I'll disable the implicit setting of --prof-mod=<module> when resolving the conflict.

The TOML thing is an interesting angle – thanks for the suggestion – and sounds like a good idea for doing what was intended in a more explicit way. Since in your reply to #29 you also mentioned line_profiler.profile being configurable via its .setup_config, .show_config, and .write_config, I wonder if it will also make sense to include those in the hypothetical [tool.line_profiler] section and have GlobalProfiler read the values from there. In any case this sounds like a somewhat substantial feature... we should maybe write a new issue and discuss how to proceed (scope, semantics, etc.).

Thanks for the example. Curious, I thought I covered __main__.py use-cases in both the codebase and the test suite. Will take a closer look later today...

[Erotemic]

Yes, I agree that any pyproject.toml awareness should be taken care of in a separate issue / PR. But the more I think about it the more I like it. I was also thinking it would be a good way to configure the global profiler, and it makes sense with the way the python ecosystem is moving.

Let's get this working such that -m just invokes the module main script and relies on -p for any auto-profile awareness. I think there may be some more issues with auto-profile, but those can be resolved elsewhere. Once I'm convinced that kernprof -m works to execute a module in the same way that python -m would, then I'll do a final review and we can merge.

[TTsangSC]

Just took a look at your demo.

• Initially I just set ${PYTHONPATH} explicitly, and then it Worked on My Machine™.
• Did a bit of digging afterwards, and the crux of the issue seems to be that python -m prepends pwd to sys.path. So we'll have to emulate that to better approximate the behavior of python -m. (The same goes for python -c should we later support that too.)
  • The safe-path (-P) and isolated (-I) modes as well as the ${PYTHONSAFEPATH} env var interact with sys.path and preclude such prepending, but IDK if it's a rabbit hole we want to go down.
  • Since the tests are all run with python -m kernprof instead of kernprof, the bug has so bar been undetected.
• Unfortunately the mypkg.helpers namespace's not being profiled is just a part of how auto-profile currently works. Since mypkg.__main__ only imported run_many_functions from mypkg.__init__, mypkg.helpers was never directly imported in the executed file and thus is not profiled. This is probably the same issue you encountered in Autoprofile is not working as expected. #318, and for the same reasons that @ta946 mentioned.
• As mentioned in my reply to said issue, we can technically profile arbitrary functions even if they or their host modules aren't directly imported – if we're willing to mess with sys.meta_path and the import system that is. I have a repo (pytest-autoprofile) doing that; if you're interested we can see how much of that should be ported upstream here.

The right thing to do now would be to:

• Monkey-patch sys.path in the kernprof -m mode as suggested above, and
• Add a couple test cases to run kernprof -m instead of python -m kernprof -m.

Will do in a couple minutes.

Apropos, when going thru the kernprof.py::main() code I noticed that there are already instances of sys.path patches inside. Since these represent changes to the global state, I'll take the liberty to wrap main() in a context manager so that it restores sys.path on the way out. This will hopefully also make in-process testing easier: ideally we won't have to ub.cmd([sys.executable, '-m', 'kernprof', *args]) in tests anymore, and can just kernprof.main(args).

[TTsangSC]

I think I've fixed the -m bug you mentioned by 15504a5 – of course with the aforementioned caveat about what isn't currently possible with -p. One thing I forgot in the reply though is that your example with multiple -p wouldn't work as intended even if you did import mypkg.helpers in __main__.py, because -p isn't currently an action='append' or 'extend' flag. But maybe it would make sense to make it one...

[Erotemic]

Not sure if there is any way to "terminate the option list" after a -m is seen with argparse. That would better match Python's -m behavior.

[TTsangSC]

We may have to implement some pre-parsing before handing things off to argparse. As it is now, -m is simply a boolean flag and its position doesn't matter at all (and that I agree isn't ideal)... lemme think over this for a moment

[TTsangSC]

Just pushed another fix for -m's help text and also to make it possible to pass multiple -p flags (without the last one overwriting everything), as would suit your earlier use-case.

Will get back to you in about an hour on getting -m to terminate parsing.

[Erotemic]

Ok, this looks good. I'm going to merge.

Are you interested in working on more of the features we discussed? I'd like to continue streamlining usage of the module, while maintaining backwards compatibility. I think we have a 2 paths foward:

1. pyproject.toml configuration
2. improving autoprofiling

for the pyproject.toml configuration, I would like it to have a 1-to-1 match with the CLI with the exclusion of special args (This is the principle I designed scriptconfig around). There we have kernprof and the explicit profile decorator and they have different configs. So either they need to be unified, or we have to special case. As a simple start maybe something like this:

[tool.line_profiler.kernprof]
module = "modname"  # <str|None> 
rich = true
view = true
skip-zero = false
builtin = false
outfile = 'auto'  # rectify with explicit write config
unit = 1e6
setup = ""
line-by-line = false  # we may want to consider just defaulting to true in a future major release
output-interval = "disabled"
prof-mod = ["list", "of", "submods"]   # we may want to change this name

[tool.line_profiler.write]
lprof = true
text = true
timestamped_text = true
stdout = true 

[tool.line_profiler.show]
sort = true
rich = true
details = true
summarize = true

The above could likely be unified and reworked into something much cleaner, but it's a start.

for better autoprofiling, maybe we do have to pre-import the modules marked by prof-mod in order to get the intuitive behavior. (If you specify a module you should get results from that module). For instance in my example, I would expect:

kernprof -lrv -p mymod.helpers -m mymod

to autoprofile the helpers module.

In terms of duplicate -p flags, I'd be fine with supporting that, but I'd like it to work more like "extend" in that each value to -p could be a comma-separated list of module names. In fact it might be nice to support json or YAML like inputs so -p [modname1,modname2] or -p {"modname1", "modname2"} was an option.

Ultimately it would be nice to have the ergonomics of running either:

LINE_PROFILE=1 python -m my_module.entry_point "${@}"
LINE_PROFILE=1 python script.py "${@}"
kernprof -m my_module.entry_point "${@}"
kernprof script.py "${@}"

# or even
kernprof

In the LINE_PROFILER=1 case, it would require that line_profiler was imported somewhere in the Python runtime and then it could configure itself. It might be tricky because if line_profiler is imported late, then we either couldn't profile those modules or we would have to rewrite module state, which could get ugly. Maybe we could warn the user if they requested an auto-profile of a module that hasn't been imported yet.

Running kernprof should be as simple as pre-importing the modules to profile before starting the script. But importing those modules in a different order and context than would happen if kernprof was not used could cause issues (especially with binary libraries that compete for dependencies).

I'm wondering if there is a way to hook into the Python import mechanism to intercept and rewrite the module AST with decorated functions as they are imported. That way a module would never be imported out of the intended order.

[TTsangSC]

The TOML stuff looks exciting – count me in.

The duplicate -p flags currently works exactly as you described: the previous syntax of -p target1,target2 is still supported, see the updated test_autoprofile_exec_package() test, where a subtest does -p test_mod -p test_mod.submod1,test_mod.submod2. I just wrote it as an action='append' instead of 'extend' to avoid argparse splitting the input string into chars. (But of course without the JSON-like parsing.)

On-import AST rewrites are certainly doable, it's what I'm doing in the other project I mentioned. See https://gitlab.com/TTsangSC/pytest-autoprofile/-/blob/master/src/pytest_autoprofile/importers.py?ref_type=heads. Again, we can work out how much of that to put back upstream here – feel free to shop around and see what is to your liking.

(EDIT: since you mentioned import ordering, we may have to look into installing .pth files so that the hook is installed before everything else (beside its dependencies) are imported. That may be a bit more difficult...)

Oh, and do you want to merge now or wait till I'm done special-casing -m and do the truncation-of-parsed-args thing you mentioned? I'm about done with that, just needs about half an hour to round off the corner, docs, and add a test for that.

[TTsangSC]

Sorry for the delay, but we now have option-list termination for kernprof -m. Ready for merge unless you think it needs some cleanup.

[Erotemic]

Looks great. Merging.

[TTsangSC]

Cheers!