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

Allow attaching co_firstlineno to frame name #428

Merged
merged 3 commits into from
Aug 18, 2021

Conversation

Jongy
Copy link
Contributor

@Jongy Jongy commented Jul 31, 2021

Before this PR, we had 2 modes:

  • --function NOT passed: attach the line number from frame->f_lasti to the frame name
  • --function passed: do not attach any line number

The main reason to avoid attaching f_lasti is to have frames aggregated by the function name only;
so different "lines" of the same function are aggregated as a single frame.

However, line numbers are useful as identifiers for functions, especially in large files / for common
function names such as "init", which are likely to appear multiple times in the same file.
This PR allows attaching code->co_firstlineno instead, which can serve to help in identification,
while not preventing frames of the same function from being aggregated together.

After this PR, we have these options:

  • --function, --nolineno NOT passed: same as before - attach f_lasti to the frame name
  • --function passed: attach co_firstlineno to the frame name
  • --nolineno: do not attach any line number (also, do not spend time on retrieving the line number from
    the remote process).

Closes: #424

I have tested it on --format raw and AFAICT the lineno option will remain the default for e.g the py-spy top mode, perhaps we want to add it there as well?

@benfred do you think this deems any module-level tests added? I can add

Jongy and others added 2 commits August 1, 2021 00:19
Before this PR, we had 2 modes:
* --function NOT passed: attach the line number from frame->f_lasti to the frame name
* --function passed: do not attach any line number

The main reason to avoid attaching f_lasti is to have frames aggregated by the function name only;
so different "lines" of the same function are aggregated as a single frame.

However, line numbers are useful as identifiers for functions, especially in large files / for common
function names such as "__init__", which are likely to appear multiple times in the same file.
This PR allows attaching code->co_firstlineno instead, which can serve to help in identification,
while not preventing frames of the same function from being aggregated together.

After this PR, we have these options:
* --function, --nolineno NOT passed: same as before - attach f_lasti to the frame name
* --function passed: attach co_firstlineno to the frame name
* --nolineno: do not attach any line number (also, do not spend time on retrieving the line number from
  the remote process).

Closes: benfred#424
@Jongy
Copy link
Contributor Author

Jongy commented Aug 16, 2021

@benfred , is it missing anything?

Copy link
Owner

@benfred benfred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this!

@benfred benfred merged commit 5ca90c6 into benfred:master Aug 18, 2021
@Jongy Jongy deleted the linenos-improvements branch August 18, 2021 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow displaying co_firstlineno in -F mode
2 participants