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

Dont panic for 0 frequence #182

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Dont panic for 0 frequence #182

wants to merge 1 commit into from

Conversation

arilou
Copy link

@arilou arilou commented Dec 4, 2022

Make 0 frequency a valid option.

Make 0 frequency a valid option.
@YangKeao
Copy link
Member

YangKeao commented Dec 9, 2022

Thanks for your contribution 🍻.

If the interval is 0, the timer is stopped. I don't think it's intuitive for users. Could we return the error to the user?

Actually, panic is not that bad 🤔. Could you give more explanation on why you want to avoid the panic?

@arilou
Copy link
Author

arilou commented Dec 9, 2022

Hi @YangKeao, yes I can give you the scenario where this found me, I have integrated pprof-rs to an application I'm writing, so first of all thank it's a great crate and it helped find some nice places that required additional optimizations.

As for the scenario, the application I'm writing has a server listening thread which accepts commands, for example one of the command is to start the profiling.

I have added another command which collects the backtraces of all the threads in the application using your crate, basically what I do is set the frequency to 0, and then tkill I can send a SIGPROF to each thread, once I'm done sending the SIGPROF to each thread I collect the backtraces from the Profiler.

I thought about contributing that piece of code as well but it's very platform specific (only Linux) and it's not really a profiling scenario.

Thanks,
-- Jon.

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.

None yet

2 participants