search

tmm1 / stackprof

a sampling call-stack profiler for ruby 2.2+
MIT License
2.08k stars 128 forks source link

TruffleRuby support #159

Closed eregon closed 2 years ago

eregon commented 3 years ago

Hello!

Currently stackprof fails to install on TruffleRuby, due to the extconf.rb checking for rb_postponed_job_register_one() and rb_profile_frames() and those APIs are not implemented on TruffleRuby.

It's also a good question whether implementing rb_profile_frames() on TruffleRuby would even make sense, because it would likely be pretty inefficient.

I see multiple alternatives, which I detailed in https://github.com/oracle/truffleruby/issues/2044#issuecomment-654848324

I think in the shorter term it would be nice if the gem would install on TruffleRuby, but when used at runtime raise an exception with some message linking to https://github.com/oracle/truffleruby/issues/2044 and mentioning the built-in --cpusampler as an alternative.

Longer term, I think it would be worth a try to implement rb_profile_frames() in TruffleRuby using CPUSampler#takeSample and see if that has a reasonable overhead or if it's too high to be practical and other APIs would be needed.

Do the stackprof maintainers have any opinion about that?

tenderlove commented 3 years ago

I'm not strictly opposed to this. But those checks are in place so that the gem can't be installed on older Rubys that don't have the function.

Maybe this is a silly idea, but could you implement rb_profile_frames() in TruffleRuby and just have it raise an exception?

Anyway, I'm not opposed as long as we can find a way to make sure the gem won't install on platforms where we don't want it.

eregon commented 3 years ago

Maybe this is a silly idea, but could you implement rb_profile_frames() in TruffleRuby and just have it raise an exception?

That's possible, but it's suboptimal because some other gem could check for rb_profile_frames() and if that gem has a fallback (i.e., can handle that function not being there) it would likely work on TruffleRuby if it's not defined, but it would break (i.e., worse than the current state) if we define it as a "stub raising an exception". We've seen that issue with other C API functions, so in general stubs should be avoided as they break detection via have_func.

Anyway, I'm not opposed as long as we can find a way to make sure the gem won't install on platforms where we don't want it.

Right, we'd only change things if RUBY_ENGINE == 'truffleruby' since then we can also have a TruffleRuby-specific error message mentioning --cpusampler.

tenderlove commented 3 years ago

Right, we'd only change things if RUBY_ENGINE == 'truffleruby' since then we can also have a TruffleRuby-specific error message mentioning --cpusampler.

OK, sounds good. Send a PR and I'll merge it! 😄

eregon commented 2 years ago

Better late than never :sweat_smile:: https://github.com/tmm1/stackprof/pull/184