codecov[bot]
commented
1 year ago Codecov Report
Merging #201 (8956a10) into main (fdd69e1) will increase coverage by
11.57%. Report is 9 commits behind head on main. The diff coverage is60.85%.
Closed ta946 closed 10 months ago
codecov[bot]
commented
1 year ago Merging #201 (8956a10) into main (fdd69e1) will increase coverage by
11.57%. Report is 9 commits behind head on main. The diff coverage is60.85%.
ZeyuSun
commented
11 months ago How is this going? This feature is much needed.
ta946
commented
11 months ago How is this going? This feature is much needed.
The code works, but needs some cleanup before it can be merged into this repo. I've been using it for a while with no obvious issues with my limited testing (not much of a guarantee), so you can manually apply this change if you want
Erotemic
commented
11 months ago I'll try to make some time to look at this within the month.
Erotemic
commented
11 months ago Did some minor cleanup on the code as I prepare to dig into this. Rebased on the latest main, which gives us access to CI upgrades. Moved the POC to the "dev" folder as well as added a "dev/maintain/port_utilities.py" script which does the autogeneration of "util_static.py" (renamed from util_import.py). Liberator is generating some funky indentations in strings, and I'm not sure why, but it's fine for now.
Going to need to add docstrings to most functions / classes to describe what they do at a highlevel. Will also need to add tests for this.
Erotemic
commented
11 months ago @ta946 I've rebased this on main, which includes the latest updates for rich output and the explicit profile decorator. I've added a changelog note, stubs for docs, and some basic tests for this feature as well.
Could you:
Add docstrings to important classes / function that explain what they are / what they are used for. (Note if you follow google-style docstrings with types, similar to what I've done in show_func, then I will be able to auto-generate type stubs for zero-overhead type checking).
Add more tests for relevant cases, particularly the case where you are running a script that depends on multiple modules and you want to autoprofile some, but not all of them?
Add a docstring to autoprofile/__init__.py that gives a introduction / tutorial for how to use this feature (this docstring will be added to the readthedocs, which I publish with this next release). This can be in a similar vein to what I've done with explicit_profile.py
Once these three tasks are completed I think this will be ready for a merge and release.
ta946
commented
11 months ago @Erotemic Will start on it, but I might be abit slow as work has been crazy lately A reminder for myself that we need to update the version number.
One thing I need feedback on. What should we do if auto profile is selected but no modules are supplied? No profiling or full script profiling (instead of having to pass the script path)?
Erotemic
commented
11 months ago What should we do if auto profile is selected but no modules are supplied?
I don't think that can happen because the only way to auto profile it to specify something to -p. One good test case might be to see what happens when the specified module doesn't exist, but I expect it will be the same as if you ran kernprof without decorating anything with @profile.
A reminder for myself that we need to update the version number.
It's already taken care of, the next release will be 4.1.0.
Will start on it, but I might be abit slow as work has been crazy lately
No problem, because I'm fairly sure everything is working and there is at least one test, the most important thing to do is write the tutorial.
ta946
commented
11 months ago I don't think that can happen because the only way to auto profile it to specify something to
-p
I could just test this, but what happens if they supply kernprof.exe ... -p -o output.lprof where they dont supply anything to it? i have it default to empty string. I assume it wouldnt affect -o and would just be empty.
So i see 3 options, remove the default and force them to supply a str or argparse will complain
empty string means no auto profiling
or empty string means profile the current script
Erotemic
commented
11 months ago So i see 3 options, remove the default and force them to supply a str or argparse will complain empty string means no auto profiling
I think the third "don't auto-decorate anything" option makes the most sense given how the code is laid out. I don't like erroring here, I don't think there is a reason to. The "profile the current script behavior" might be useful, but it feels like it shoul be a different argument than -p if we wanted to include that feature. For now let's just cleanup, document, and release the current implementation rather than adding new features.
ta946
commented
10 months ago Finished adding docstrings, hopefully it is meaningful enough for whoever needs to modify the code at some point. Will work on the tutorial next, but i can't write documentation to save my life 😅
Erotemic
commented
10 months ago The docstrings look good - way more than I was expecting.
I pushed up changee to your branch (be sure to pull them before continuing work). In the line_profiler/__init__.py I added a docstring for basic usage that gives a quickstart example of how to use kernprof / line_profiler. I'm looking for something of at that level of detail where it's just an example the user can walk through to see the code working. Taking the "find-primes" example and then just showing how to use with the kernprof's autoprofiler would be enough. It would be nice to have that in the line_profiler/autoprofile/__init__.py docstring (because I like coupling docs with the code).
EDIT: I also fixed the type annotations so they would generate pyi files correctly.
Erotemic
commented
10 months ago @ta946 I'll be able to write the tutorial. Will merge after I do that and tests pass.
Erotemic
commented
10 months ago I've added a few more tests for autoprofiling in the case where an entire module is profiled, and profile imports is on. In the later case this breaks on 3.11 due to #232, which will hopefully be fixed soon. I'm going to skip that test for now.
I've also removed the '-m' shortcut for --prof-imports because I'd like to reserve that for future use.
Erotemic
commented
10 months ago Thanks for the contribution @ta946!
ta946
commented
10 months ago Thanks and sorry for having you do the docs on my behalf!
tmm1
commented
10 months ago I'm trying to understand how to integrate this with pytest-line-profiler and other programatic profiling approaches. Is there any non-cli example, for instance showing how to profile all methods in a module?
Erotemic
commented
10 months ago References to the original discussions don't seem to be present or are burried so adding them here:
The way it currently works is that the functions to profile are statically extracted from the AST of the script / module(s) to profile via the AstTreeProfiler and then transformed to add the @profile decorator using AstProfileTransformer. The line_profiler.autoprofile.run is the entry point.
It's integration with kernprof is fairly coupled at the moment. I would be interested in a refactoring to make things simpler / decoupled.
However, I'm not sure if non-CLI use makes sense with the current implementation. It's written the way it is to allow us to inject profiles into the code before it is imported. Using it programatically means that you couldn't profile any module you've already imported because there is no way to modify it's AST at that point.
What you could do instead is take a normal LineProfiler (or the new explicit globals one) object and monkey patch all of the functions in a module to wrap them in a @profile call.
Something like this should work:
def foo():
return 1 + 2
def bar():
return 2 + 2
def main():
a = foo()
b = bar()
print(a + b)
def profile_globals():
"""
Adds the profile decorator to all global functions
"""
import sys
import inspect
parent_frame = inspect.currentframe().f_back
parent_frame.f_globals
name = parent_frame.f_globals['__name__']
module = sys.modules[name]
from xdoctest.dynamic_analysis import is_defined_by_module
import line_profiler
line_profiler.profile.enable()
for k, v in module.__dict__.items():
if is_defined_by_module(v, module):
if callable(v):
v = line_profiler.profile(v)
parent_frame.f_globals[k] = v
profile_globals()
if __name__ == '__main__':
main()I'm trying to understand how to integrate this with pytest-line-profiler and other programatic profiling approaches. Is there any non-cli example, for instance showing how to profile all methods in a module?
you could run the autoprofiling in a programatic way, as technically that is what kernprof is doing, but it is using an exec() call in line_profiler.autoprofile.run. So im not sure if that would work the way you need it, as it will need to be a script that will run the script+profiling instead of adding some code to the script itself to enable.
heres an example of what i meant, not the prettiest and the builtins/ns part could potentially be replaced with a dict
import line_profiler
script_file = 'path/to/script.py'
prof_mod = [script_file]
prof = line_profiler.LineProfiler()
builtins.__dict__['profile'] = prof
ns = locals()
line_profiler.autoprofile.run(script_file, ns, prof_mod=prof_mod)
# to get the profiling output after running the script
prof.dump_stats(options.outfile)
prof.print_stats(output_unit=options.unit,
stripzeros=options.skip_zero,
rich=options.rich,
stream=original_stdout)@Erotemic maybe this example can be a good starting point of whats needed to refactor to make it less coupled
Mogost
commented
3 weeks ago I'm trying to understand how to integrate this with pytest-line-profiler and other programatic profiling approaches. Is there any non-cli example, for instance showing how to profile all methods in a module?
@tmm1 Did you manage to progress? It seems I'm trying to solve a similar problem.
Profile a script using line_profiler without modifying the source code