Make tracepoints with set_trace_func or TracePoint.new ractor local#7184
Make tracepoints with set_trace_func or TracePoint.new ractor local#7184luke-gru wants to merge 1 commit intoruby:masterfrom
Conversation
|
This broke some internal events tests, like GC events, so I'll take a look soon. |
|
Thanks for picking up this fix!
This would be great as well! I work on the profiler included in the This way, we won't have to do things like intercept Ractor creation to make sure we reactivate our tracepoints for every new one that gets created. Even if such an API is only available from C (I understand it may be harder to get Ruby-level tracepoints working cross-ractor), it would be incredibly valuable :) |
ff26fcd to
06ead01
Compare
Not a problem 😄 I think a |
|
Just wanted to follow up to ask if there's any plans to finish this. This would help enable reliable memory allocation profiling in our infrastructure. |
|
This should be getting more attention soon. I plan on working on the 'debug' gem to enable breakpoints to work inside Ractors, and this PR would enable that work. |
Before this change, GC'ing any Ractor object caused you to lose all enabled tracepoints across all ractors (even main). Now tracepoints are ractor-local and this doesn't happen. Internal events are still global. Fixes [Bug #19112]
c797f88 to
35cceee
Compare
It does! I just checked and the tracepoint we use for GC and the one we use for NEWOBJ no longer get disabled on Ruby 4. We have some documentation in the Datadog gem about this issue, I'm about to open a PR to update it to mention that it's a Ruby 3.x issue only, and fixed on Ruby 4.
This will indeed be quite useful :) I mentioned above we don't have support for observing non-main ractors in the datadog gem, and that's still the case; but with the renewed push to get Ractors to be non-experimental, this is something we'll need to support in the future. |
**What does this PR do?** For a while now, whenever the GC and more importantly the allocation profiling features were enabled, we logged a user-visible message that if Ractors were used, a bug in Ruby could cause the profiler to miss data (due to https://bugs.ruby-lang.org/issues/19112 ). The upstream issue has been fixed in <ruby/ruby#15468> (some discussion as well in <ruby/ruby#7184> ). Thus, this PR does a few small things: * Update the logged messages to mention the bug doesn't happen on Ruby 4 * Moves all messages to be debug-level messages (our logic already made sure they were only printed on Ruby 3, hah turns out we predicted this!) In particular our concern in the past for making the message user-visible by default was that customers might use Ractors and run into this issue. As it turns out that in Ruby 3 ractors were always experimental and had a number of bugs/issues, we don't expect anyone to run Ractors in production with Ruby 3. (Maybe this will change with Ruby 4?) * Removes the `OnlyOnce` that was used to limit verboseness -- this is extra complexity that we don't need to maintain now that the log is debug level **Motivation:** Avoid annoying customers with log messages about Ractor support on Ruby 3. **Additional Notes:** N/A **How to test the change?** For the warnings themselves, I've updated our tests to match the new setup. For the upstream issue, I manually used the testcase from https://bugs.ruby-lang.org/issues/19112 and added prints to the profiling GC and NEWOBJ tracepoints, confirming that on Ruby 3.4 they get disabled but on Ruby 4.0 they work fine. I considered adding an explicit test for this but it seems a bit brittle as it would be a test for something not happening after Ractor GC which is kinda fishy to test for, so I ended up not doing it.
Awesome, thank you for checking! |
Before this change, GC'ing any Ractor object caused you to lose all enabled tracepoints across all ractors (even main). Now tracepoints are ractor-local and this doesn't happen.
Fixes [Bug #19112]
NOTE: I have yet to change the docs for TracePoint to mention that they are ractor-local, but I'm going to do that
in a separate PR. Also, I want to make it so that there's a flag to TracePoint#enable to make them global.