FIX: always padding the bytecode of profiled functions#425
FIX: always padding the bytecode of profiled functions#425TTsangSC wants to merge 4 commits intopyutils:mainfrom
Conversation
tests/test_line_profiler.py::test_nonprofiled_clashing_bytecodes()
New test (currently failing) for the case described in issue pyutils#424:
- One uncalled function passed to the profiler
- Another (bytecode-wise) identical function called, but never
passed to the profiler
- Even profiling is active, the profiler should not have recorded
any line event, but it nontheless did
line_profiler/_line_profiler.pyx::LineProfiler.add_function()
- Now making sure that ALL functions passed to the method will be
NOP-bytecode-padded, instead of from the 2nd one onwards
- No longer re-padding already NOP-bytecode-padded code objects
(could be by another profiler instance or even the same one);
this probably eliminates the need for keeping track on profiler
instances and leaves room for further optimizations
tests/test_line_profiler.py
::test_aggregate_profiling_data_between_code_versions()
- Updated test body to no longer test for bytecode alteration
(because there is no longer any)
- Added note in docstring stating the original context of the test
and how it is no longer relevant (or should we just delete this
test?)
line_profiler/_line_profiler.pyx::LineProfiler
add_function()
No longer managing a set of profilers whose internal states are
to be updated (since it is now always just the instance itself)
_all_instances_by_funcs
No longer keeping this class attribute -- we no longer re-pad
bytecodes, so other instances referring to the same function
don't need to be retrievable
|
Huh the build jobs choked on all non-Linux platforms like the merge pipeline for #423 with HTTP 429 during the (Curiously I tried |
|
Retrying the workflow didn't seem to do the trick. I am noticing that we are still supporting 3.8, and we might want to consider dropping that at this point. But it's very strange that the error on windows is too many requests. It also looks like the issue might be on the main branch, and that did pass before I merged, so it very likely is some issue with the url being dead or something. I wonder if we drop 3.8 or update cibildwheel if it will work. |
2 participants
Closes #424.
Summary
LineProfiler.add_function()has been streamlined to cut down on the need for bookkeeping, resulting in simpler code and a very slight performance gain.Code changes
line_profiler/_line_profiler.pyx::LineProfiler._all_instances_by_funcsRemoved this private class attribute; owing to the changes to
.add_function()as described below, we no longer need to keep tabs on which profiler instance is profiling which function objects.add_function().co_codeof the code object of a (normal non-Cython) profiled function is now ALWAYS padded with at least one NOP; this serves to mark profiled functions apart from non-profiled ones which happen to compile down to the same bytecode..co_codehas already been padded, it is no longer re-padded in the event that multiple profiler instances are profiling the same function..code_hash_mapand._c_code_map.Test changes
tests/test_line_profiler.pytest_nonprofiled_clashing_bytecodes()New test against the edge case described in the issue, making sure that line events from the non-profiled bytecode twin aren't misattributed to the profiled function
test_aggregate_profiling_data_between_code_version()Updated to no longer check for code-object change after decoration by the second profiler, because again we stopped re-padding bytecodes (FIX: Update hash tables of affected profiler instances when rewriting code objects #351); note however that this test is now essentially obsolete and maybe we should just remove it...