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.

Sign up for GitHub

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

Jump to bottom

Add native traces to RubySpy::get_stack_trace #372

Open
Maaarcocr opened this issue Dec 13, 2022 · 4 comments · May be fixed by #373
Open

Add native traces to RubySpy::get_stack_trace #372

Maaarcocr opened this issue Dec 13, 2022 · 4 comments · May be fixed by #373

Comments

@Maaarcocr
Copy link

I've been needing to profile a ruby application that uses a variety of ruby extensions (written in C & rust) and also knowing how time is spent in the native extension a lot of the logic resides there.

I've taken inspiration from https://www.benfrederickson.com/profiling-native-python-extensions-with-py-spy/ and their source code to figure out how I could do it with rbspy. I have a prototype that works, and I have 2 questions:

When I get a StackTrace from ruby I also get a thread id, but it does not seem to match any thread id from remoteprocess::Process::threads in RubySpy. Is that expected? Is there a way to get a current thread ID that matches one from remoteprocess?

Would there be any interest in merging this into main here? Right now my changes live in my fork. I've made it such this capability is only turned on if a feature flag is set to true.
@acj
Copy link
Member

acj commented Dec 15, 2022

Hey, thanks for working on this.

I have a prototype that works

🎉

When I get a StackTrace from ruby I also get a thread id, but it does not seem to match any thread id from remoteprocess::Process::threads in RubySpy. Is that expected? Is there a way to get a current thread ID that matches one from remoteprocess?

Hmm, I'm not sure. The thread ID in the StackTrace is read from rb_thread_struct, which might be an internal ID that the Ruby VM controls rather than the OS's thread ID that remoteprocess returns. Can you say more about the problem you're trying to solve? Also, which ruby version are you using to test this?

There's some structural similarity between this and the work in #327 -- might be worth a look.

Would there be any interest in merging this into main here?

Yes, it would be great to support this capability. Feel free to open a draft PR when you're ready, and then we can work out the details. Worst case, we can keep it behind the feature flag until it's working reliably.

@Maaarcocr
Copy link
Author

Can you say more about the problem you're trying to solve? Also, which ruby version are you using to test this?

right so, currently (this is my understanding of rbspy, it may be wrong) rbspy would get the traces only for the current thread every time it samples the ruby process. Ideally I would like to do the same with native traces, i.e. only get the native traces for the thread which is marked as active by the RubyVM and just ignore all the other system threads.

If I do not do this, then it becomes difficult to know how to interweave the native trace and the ruby trace, as we could be in a situation where we have multiple native traces (one for each OS thread from remoteprocess) and just 1 ruby trace.

to make my prototype work I currently just assume that there is only 1 thread, but that would not work in the general case.

@Maaarcocr Maaarcocr linked a pull request Dec 15, 2022 that will close this issue
Open
@Maaarcocr
Copy link
Author

Feel free to open a draft PR when you're ready, and then we can work out the details

here you go: #373!

@acj
Copy link
Member

acj commented Jan 7, 2023

Thanks! I needed to take a break during the holiday, but I should have time to review the PR this next week. Looking forward to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement native profiling Maaarcocr/rbspy
2 participants
@acj @Maaarcocr