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

Speed up --gil by skipping work for non-GIL threads #630

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

wmanley
Copy link
Contributor

@wmanley wmanley commented Nov 7, 2023

When the user has specified --gil we needn't collect stack traces for the non-GIL threads. This is a significant speedup on heavily threaded Python code.

My server has 271 threads (most of which are idle).

With the previous code (sampling at 10Hz) I get:

$ ./py-spy record -o /var/opt/stbt/profile.svg --pid $(pgrep -f http_service.service) -r 10 -d 10 --gil
py-spy> Sampling process 10 times a second for 10 seconds. Press Control-C to exit.

py-spy> 1.29s behind in sampling, results may be inaccurate. Try reducing the sampling rate
py-spy> 1.36s behind in sampling, results may be inaccurate. Try reducing the sampling rate
py-spy> 1.32s behind in sampling, results may be inaccurate. Try reducing the sampling rate
py-spy> 1.22s behind in sampling, results may be inaccurate. Try reducing the sampling rate
py-spy> 1.00s behind in sampling, results may be inaccurate. Try reducing the sampling rate
py-spy> Wrote flamegraph data to '/var/opt/stbt/profile.svg'. Samples: 3 Errors: 51

With this commit I can sample at 80Hz, and the error rate is way lower:

$ ./py-spy2 record -o /var/opt/stbt/profile.svg --pid $(pgrep -f http_service.service) -r 80 -d 10 --gil
py-spy> Sampling process 80 times a second for 10 seconds. Press Control-C to exit.

py-spy> 1.00s behind in sampling, results may be inaccurate. Try reducing the sampling rate
py-spy> Wrote flamegraph data to '/var/opt/stbt/profile.svg'. Samples: 43 Errors: 0

I suspect the difference in error rate is due to the error handling in _get_stack_traces - an error getting information about any thread means that all the traces are thrown away.

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 the PR!

src/python_spy.rs Outdated Show resolved Hide resolved
@wmanley wmanley force-pushed the faster--gil branch 2 times, most recently from 0eed74d to e81b3e4 Compare November 7, 2023 23:44
When the user has specified --gil we needn't collect stack traces for the
non-GIL threads.  This is a significant speedup on heavily threaded Python
code.

My server has 271 threads (most of which are idle).

With the previous code (sampling at 10Hz) I get:

    $ ./py-spy record -o /var/opt/stbt/profile.svg --pid $(pgrep -f http_service.service) -r 10 -d 10 --gil
    py-spy> Sampling process 10 times a second for 10 seconds. Press Control-C to exit.

    py-spy> 1.29s behind in sampling, results may be inaccurate. Try reducing the sampling rate
    py-spy> 1.36s behind in sampling, results may be inaccurate. Try reducing the sampling rate
    py-spy> 1.32s behind in sampling, results may be inaccurate. Try reducing the sampling rate
    py-spy> 1.22s behind in sampling, results may be inaccurate. Try reducing the sampling rate
    py-spy> 1.00s behind in sampling, results may be inaccurate. Try reducing the sampling rate
    py-spy> Wrote flamegraph data to '/var/opt/stbt/profile.svg'. Samples: 3 Errors: 51

With this commit I can sample at 80Hz, and the error rate is way lower:

    $ ./py-spy2 record -o /var/opt/stbt/profile.svg --pid $(pgrep -f http_service.service) -r 80 -d 10 --gil
    py-spy> Sampling process 80 times a second for 10 seconds. Press Control-C to exit.

    py-spy> Wrote flamegraph data to '/var/opt/stbt/profile.svg'. Samples: 65 Errors: 0

I suspect the difference in error rate is due to the error handling in
`_get_stack_traces` - an error getting information about any thread means
that all the traces are thrown away.
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!

@benfred benfred merged commit 4e69e83 into benfred:master Nov 10, 2023
@wmanley wmanley deleted the faster--gil branch November 13, 2023 10:58
@benfred benfred added the enhancement New feature or request label Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants