FIX: (+ENH?) fixed and extended dispatching for LineProfiler.__call__()#332
Merged
Conversation
line_profiler/profiler_mixin.py[i]
is_{static,bound,partial}method()
is_partial()
is_[cached_]property()
New checker callables
ByCountProfilerMixin.wrap_callable()
Extended dispatch table
ByCountProfilerMixin.wrap_classmethod()
Fixed implementation;
now returns a `classmethod` object
ByCountProfilerMixin.wrap_{static,bound,partial}method()
ByCountProfilerMixin.wrap_{partial,[cached_]property}()
New methods
line_profiler/line_profiler.py[i]::LineProfiler
__call__()
Added dispatch for calling `.add_function()` on the inner
callables of callable-wrapper objects
add_property()
New helper method for calling `.add_function()` on the getter,
setter, and deleter implementations of a property
tests/test_line_profiler.py
test_classmethod_decorator()
Overhauled to test passing a real `classmethod` (instead of a
bound method) to the profiler
test_{static,bound,partial}method_decorator()
test_{partial,[cached_]property}_decorator()
New analogous tests for decorating/profiling other
callable(-like) objects
line_profiler/profiler_mixin.py[i]
is_async_generator()
New checker function
ByCountProfilerMixin.wrap_callable()
Added dispatch to `.wrap_async_generator()`
ByCountProfilerMixin.wrap_async_generator()
New method for wrapping async generator functions
ByCountProfilerMixin.wrap_generator()
Simplified implementation
tests/test_line_profiler.py
test_function_decorator(), test_gen_decorator()
- Added check for profiler timings before/after running the
profiled functions
- Added check that the profiled function is in
`LineProfiler.functions`
- (Only for `test_gen_decorator()`) added check that the wrapper
is a generator function
test_coroutine_decorator(), test_async_gen_decorator()
New tests for wrapping coroutines and async generators
line_profiler/line_profiler.py[i]::LineProfiler
__call__()
- Now deferring object registration to `.add_callable()`
- Revised docstring
add_callable()
New wrapper method around the underlying `.add_function()`
Cython method which expects functions, so that we can deal with
callable-like wrappers like class methods, properties, etc.
(Separated from `.__call__()`)
add_property()
Added docstring
line_profiler/line_profiler.py::LineProfiler
add_property()
Rolled back into `add_callable()`, no longer a standalone method
add_callable()
- Now returning 1 if a function is added to the profiler, and 0
otherwise
- Now checking against adding the wrapper callable around an
already-added callable
line_profiler/profiler_mixin.py::ByCountProfilerMixin
wrap_{function,coroutine,[async_]generator}()
- Now checking if `func` is a wrapper around an already-added
callable, and returns the same wrapper without rewrapping if
that is the case
- Now marking wrappers so that they can be recognized and not
rewrapped
tests/test_line_profiler.py
test_double_decoration()
New test that double-wrapping doesn't erroneously profile the
wrapper instead of the intended callable
test_property_decorator()
Updated because we are no longer double-adding the property
getter
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #332 +/- ##
==========================================
- Coverage 65.37% 62.30% -3.08%
==========================================
Files 13 13
Lines 901 1008 +107
Branches 196 225 +29
==========================================
+ Hits 589 628 +39
- Misses 262 316 +54
- Partials 50 64 +14
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Collaborator
Author
|
Coverage in |
Member
|
I need to figure out how to disable codecov checks. Do you happen to know? It's inserting a bunch of garbage warnings in the diff that makes it harder to read / review. |
Erotemic
reviewed
Apr 14, 2025
|
|
||
| def wrap_coroutine(self, func): | ||
| """ | ||
| Wrap a Python 3.5 coroutine to profile it. |
Erotemic
reviewed
Apr 14, 2025
| return getattr(func, self._profiler_wrapped_marker, None) == id(self) | ||
|
|
||
| def _mark_wrapped(self, func): | ||
| setattr(func, self._profiler_wrapped_marker, id(self)) |
Member
There was a problem hiding this comment.
Is there ever a case where func would be some object where setattr would fail? Some objects with __slots__ cannot have attribute set, and I'm wondering: can we always assume the objects passed here will have settable attributes?
It looks like only wrapper functions are used here, so I think we are ok.
Member
|
This all makes sense and the implementation is clean. Excellent work! |
Collaborator
Author
|
On the topic of disabling
But CI is out of my depths so IDK. Also not sure if ditching I suspect the fundamental issue here is that Footnotes
|
TTsangSC
added a commit
to TTsangSC/line_profiler
that referenced
this pull request
Apr 21, 2025
line_profiler/autoprofile/line_profiler_utils.py
add_imported_function_or_module()
- Added new argument `wrap` for controlling whether to replace
class and module members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `inspect.isfunction()` check (since we expanded the
catalog of addable objects in pyutils#332)
- Now returning whether any function has been added to the
profiler
line_profiler/line_profiler.py
add_module()
- Now shares an implementation with `.add_class()`
- Added new argument `wrap` for controlling whether to replace
members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `is_function()` check (since we expanded the catalog
of addable objects in pyutils#332)
add_class()
New method (alias to `.add_module()`)
TTsangSC
added a commit
to TTsangSC/line_profiler
that referenced
this pull request
Apr 27, 2025
line_profiler/autoprofile/line_profiler_utils.py
add_imported_function_or_module()
- Added new argument `wrap` for controlling whether to replace
class and module members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `inspect.isfunction()` check (since we expanded the
catalog of addable objects in pyutils#332)
- Now returning whether any function has been added to the
profiler
line_profiler/line_profiler.py
add_module()
- Now shares an implementation with `.add_class()`
- Added new argument `wrap` for controlling whether to replace
members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `is_function()` check (since we expanded the catalog
of addable objects in pyutils#332)
add_class()
New method (alias to `.add_module()`)
TTsangSC
added a commit
to TTsangSC/line_profiler
that referenced
this pull request
May 5, 2025
line_profiler/autoprofile/line_profiler_utils.py
add_imported_function_or_module()
- Added new argument `wrap` for controlling whether to replace
class and module members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `inspect.isfunction()` check (since we expanded the
catalog of addable objects in pyutils#332)
- Now returning whether any function has been added to the
profiler
line_profiler/line_profiler.py
add_module()
- Now shares an implementation with `.add_class()`
- Added new argument `wrap` for controlling whether to replace
members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `is_function()` check (since we expanded the catalog
of addable objects in pyutils#332)
add_class()
New method (alias to `.add_module()`)
TTsangSC
added a commit
to TTsangSC/line_profiler
that referenced
this pull request
May 17, 2025
line_profiler/autoprofile/line_profiler_utils.py
add_imported_function_or_module()
- Added new argument `wrap` for controlling whether to replace
class and module members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `inspect.isfunction()` check (since we expanded the
catalog of addable objects in pyutils#332)
- Now returning whether any function has been added to the
profiler
line_profiler/line_profiler.py
add_module()
- Now shares an implementation with `.add_class()`
- Added new argument `wrap` for controlling whether to replace
members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `is_function()` check (since we expanded the catalog
of addable objects in pyutils#332)
add_class()
New method (alias to `.add_module()`)
TTsangSC
added a commit
to TTsangSC/line_profiler
that referenced
this pull request
May 18, 2025
line_profiler/autoprofile/line_profiler_utils.py
add_imported_function_or_module()
- Added new argument `wrap` for controlling whether to replace
class and module members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `inspect.isfunction()` check (since we expanded the
catalog of addable objects in pyutils#332)
- Now returning whether any function has been added to the
profiler
line_profiler/line_profiler.py
add_module()
- Now shares an implementation with `.add_class()`
- Added new argument `wrap` for controlling whether to replace
members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `is_function()` check (since we expanded the catalog
of addable objects in pyutils#332)
add_class()
New method (alias to `.add_module()`)
TTsangSC
added a commit
to TTsangSC/line_profiler
that referenced
this pull request
May 20, 2025
line_profiler/autoprofile/line_profiler_utils.py
add_imported_function_or_module()
- Added new argument `wrap` for controlling whether to replace
class and module members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `inspect.isfunction()` check (since we expanded the
catalog of addable objects in pyutils#332)
- Now returning whether any function has been added to the
profiler
line_profiler/line_profiler.py
add_module()
- Now shares an implementation with `.add_class()`
- Added new argument `wrap` for controlling whether to replace
members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `is_function()` check (since we expanded the catalog
of addable objects in pyutils#332)
add_class()
New method (alias to `.add_module()`)
TTsangSC
added a commit
to TTsangSC/line_profiler
that referenced
this pull request
May 24, 2025
line_profiler/autoprofile/line_profiler_utils.py
add_imported_function_or_module()
- Added new argument `wrap` for controlling whether to replace
class and module members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `inspect.isfunction()` check (since we expanded the
catalog of addable objects in pyutils#332)
- Now returning whether any function has been added to the
profiler
line_profiler/line_profiler.py
add_module()
- Now shares an implementation with `.add_class()`
- Added new argument `wrap` for controlling whether to replace
members with wrappers
- Refactored object adding to be more aggressive, ditching the
explicit `is_function()` check (since we expanded the catalog
of addable objects in pyutils#332)
add_class()
New method (alias to `.add_module()`)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(Closes #328)
Synopsis
This PR:
LineProfilerso that it can reliably handle decorating the following:property,functools.cached_propertyfunctools.partialfunctools.partialmethodMotivation
LineProfiler.__call__()bugged on class and static methods #328, decorating class and static methods withLineProfiler(and by extensionGlobalProfiler) is currently bugged because the returned object is a regulartypes.FunctionType, which has different behaviors on.__get__()compared with the decorated objects..wrap_function(), a.wrap_generator(), a.wrap_coroutine(), but not a corresponding.wrap_async_generator().line_profiler.autoprofilecurrently neglects properties on imports. Indeed, it neglects anything that isn't a vanillatypes.FunctionType(like class and static methods) because of rather conservativeinspect.isfunction()checks inline_profiler/autoprofile/line_profiler_utils.py::add_imported_function_or_module()andline_profiler/line_profiler.py::LineProfiler.add_module().Implementation
line_profiler/profiler_mixin.py::ByCountProfilerMixin.wrap_callable()now dispatches to the following new methods where appropriate:.wrap_{bound,static,partial}method().wrap_partial().wrap_[cached_]property().wrap_async_generator().add_function()directly,line_profiler/line_profiler.py::LineProfiler.__call__()now calls the new.add_callable(), which likewise handles dispatch for these object types and passes the underlying function objects to.add_function().tests/test_line_profiler.py::test_{{bound,static,partial}method,[cached_]property,partial,async_gen}_decorator()for the above.tests/test_line_profiler.py::test_coroutine_decorator()for completeness.tests/test_line_profiler.py::test_{function,gen,classmethod}_decorator()to be more comprehensive and consistent with the new tests.Caveats
The added dispatching may have one-time performance implications. However, as long as we aren't actively adding new items to be profiled by the profiler from within the profiled code, it should not impact the profiling results.
Further work
By merging this, we can be a bit more liberal with the types of objects we can decorate and/or register with a
LineProfiler. This opens avenues for the revision of the implementations of the aforementionedadd_imported_function_or_module()andadd_module(), allowing e.g. properties and class/static methods to be profiled withkernprof --prof-mod=...,kernprof --prof-imports, etc.Though not as thorough as import-time AST rewrites (see comment), it will be useful (1) in the meantime before that is integrated, and (2) covering edge cases that AST rewrites may miss.