[PROF-7622] Add support for profiling Ruby 3.3.0-preview1 #2860

ivoanjo · 2023-05-17T16:45:51Z

What does this PR do?:

Ruby 3.3.0-preview1 is the first release of the 3.3 branch. It includes a pretty big change: it drops MJIT (one of the two JIT implementations being shipped, the other being YJIT).

This affects the profiler because we were (ab)using an internal header that Ruby included only for supporting MJIT, and this header is no more.

The solution here is to use the same solution we were using for Ruby < 2.6. Quoting the NativeExtensionDesign.md file:

Because these private header files are not included in regular Ruby installations, we have two different workarounds:

for Ruby versions >= 2.6 we make use use the Ruby private MJIT header

for Ruby versions < 2.6 (legacy Rubies) we make use of the debase-ruby_core_source gem

Thus, the strategy going forward will be to also use the debase-ruby_core_source gem to support Ruby 3.3+.

Furthermore, there were a few changes to the APIs exported by Ruby around ractors, that I also had to fix.

Motivation:

Make sure we keep up-to-date with latest Rubies, and that we don't block our, and our client's experiments with these versions.

Additional Notes:

Because we aren't yet running Ruby 3.3 on our CI, you'll have to take my word for it that this change is correct. (Or check for yourself locally).

Separately from this PR, I'll submit a PR to add the 3.3.0-preview1 headers to debase-ruby_core_source, as I've been testing using a local fork.

Furthermore, I plan to bootstrap Ruby 3.3 testing in our CI so that we can validate that support for 3.3 keeps working.

But I don't see any reason to delay this PR before those two others.

How to test the change?:

This change can only be tested locally at the moment: install Ruby 3.3.0-preview1 in your favorite way, and then point your Gemfile to use https://github.com/DataDog/debase-ruby_core_source/tree/datadog/add-ruby-3.3.0-preview1-headers, and you'll be able to run the profiling specs as usual.

Fixes #2845

ivoanjo · 2023-05-17T16:51:47Z

Upstream PR for debase-ruby_core_source gem: ruby-debug/debase-ruby_core_source#8

**What does this PR do?**: Ruby 3.3.0-preview1 is the first release of the 3.3 branch. It includes a pretty big change: it drops MJIT (one of the two JIT implementations being shipped, the other being YJIT). This affects the profiler because we were (ab)using an internal header that Ruby included only for supporting MJIT, and this header is no more. The solution here is to use the same solution we were using for Ruby < 2.6. Quoting the `NativeExtensionDesign.md` file: > Because these private header files are not included in regular Ruby installations, we have two different workarounds: > > 1. for Ruby versions >= 2.6 we make use use the Ruby private MJIT header > 2. for Ruby versions < 2.6 (legacy Rubies) we make use of the `debase-ruby_core_source` gem Thus, the strategy going forward will be to also use the `debase-ruby_core_source` gem to support Ruby 3.3+. Furthermore, there were a few changes to the APIs exported by Ruby around ractors, that I also had to fix. **Motivation**: Make sure we keep up-to-date with latest Rubies, and that we don't block our, and our client's experiments with these versions. **Additional Notes**: Because we aren't yet running Ruby 3.3 on our CI, you'll have to take my word for it that this change is correct. (Or check for yourself locally). Separately from this PR, I'll submit a PR to add the 3.3.0-preview1 headers to `debase-ruby_core_source`, as I've been testing using a local fork. Furthermore, I plan to bootstrap Ruby 3.3 testing in our CI so that we can validate that support for 3.3 keeps working. But I don't see any reason to delay this PR before those two others. **How to test the change?**: This change can only be tested locally at the moment: install Ruby 3.3.0-preview1 in your favorite way, and then point your `Gemfile` to use <https://github.com/DataDog/debase-ruby_core_source/tree/datadog/add-ruby-3.3.0-preview1-headers>, and you'll be able to run the profiling specs as usual. Fixes #2845

**What does this PR do?**: This PR goes through every file that needs to be modified to support a new Ruby version (usually helpfully marked with "ADD NEW RUBIES HERE") and adds Ruby 3.3 there. This allows us to start testing with Ruby 3.3 in CI, ahead of the expected release in December. **Motivation**: Having Ruby 3.3 in CI allows us to spot issues early and to be ready for it once it releases. (I've also heard of at least one downstream gem that integrates with ddtrace and tests with the latest Ruby, and we want to be in good shape for this kind of testing as well). In particular, the profiler needs a few tweaks to work with Ruby 3.3 (see #2860) and this allows us to make sure those tweaks are working. **Additional Notes**: This PR sits atop #2860 and <ruby-debug/debase-ruby_core_source#8> so that the profiler test suite can be run as well. **How to test the change?**: I've modified the usual testing docker images, so the usual testing approach used for other Ruby versions documented in `DevelopmentGuild.md` also applies to this new Ruby version.

GustavoCaso · 2023-05-18T15:07:24Z

ext/ddtrace_profiling_native_extension/native_extension_helpers.rb

-      # Older Rubies don't have the MJIT header, used by the JIT compiler, so we need to use a different approach
-      CAN_USE_MJIT_HEADER = RUBY_VERSION >= '2.6'
+      # The MJIT header was introduced on 2.6 and removed on 3.3; for other Rubies we rely on debase-ruby_core_source
+      CAN_USE_MJIT_HEADER = RUBY_VERSION.start_with?('2.6', '2.7', '3.0.', '3.1.', '3.2.')


Why do you add a last . for version 3 and above?

What do you think of RUBY_VERSION > '2.6' && RUBY_VERSION <= '3.2'?

I'm adding a .because technically in around 7 years maybe Ruby 3.10 will come around and 3.10 would match 3.1 as well. But I'm not particularly consistent in doing this everywhere.

What do you think of RUBY_VERSION > '2.6' && RUBY_VERSION <= '3.2'?

No strong opinions, if you think it's worth it, I can change it.

If we use RUBY_VERSION > '2.6' && RUBY_VERSION <= '3.2', we do not have to worry about the case you mention above regarding 3.10 😄

Actually we do:

[1] pry(main)> ruby_version = "3.10.0" => "3.10.0" [2] pry(main)> ruby_version > '2.6' && ruby_version <= '3.2' => true

As a last suggestion, which is not at all necessary, we could use Gem::Version instance.

ruby_version = Gem::Version.new("3.10.0") => Gem::Version.new("3.10.0") irb(main):007:0> ruby_version > '2.6' && ruby_version <= '3.2' => false

We talked about this on slack, but let me copy this here for posteriority:

Even though the simplified version has potential for bugs, my thinking is:

We never got to .10 in the past on a Ruby release, that I know of (at least minor version -- patch version yes, like 2.4.10)

We set the maximum version that can be used in the gem, so it's not like dd-trace-rb 1.11 is ever going to be used with Ruby 3.10

If ever Ruby 3.8/3.9 approaches, we can do a search in the entire codebase by RUBY_VERSION and fix this anyway, it's not like it's that hard to find (and would be caught by tests in a lot of cases)

ext/ddtrace_profiling_native_extension/extconf.rb

GustavoCaso · 2023-05-18T16:01:47Z

ext/ddtrace_profiling_native_extension/private_vm_api_access.c

+    // versions, so we need to do a bit more work.
+    struct rb_ractor_struct *ruby_single_main_ractor = NULL;
+
+    // Taken from upstream ractor.c at commit a1b01e7701f9fc370f8dff777aad6d39a2c5a3e3 (May 2023, Ruby 3.3.0-preview1)


I noticed we only place the commit sha, would it be useful to have the full URL?

Hmm... I haven't considered, because usually I leave this to be able to have a reference when comparing to earlier/later changes. So usually I don't find a lot of value in looking at this commit, but looking at others that happened before or after it.

On the other hand, it wouldn't hurt, so I don't mind adding it if you find it relevant.

I am happy either way. I noticed because I have to go and look at the code for the ruby source and then search for the commit.

Of course, that was my flow. Others might have the code locally and simply checkout to that specific commit and check.

So I leave it to you 😄

Ok so for now I'll be lazy, but I promise to add it if we find a third person that looks at it and also finds it missing :)

ext/ddtrace_profiling_native_extension/private_vm_api_access.c

Minor code tweak Co-authored-by: Gustavo Caso <gustavocaso@gmail.com>

GustavoCaso

Great work 🎉

ivoanjo requested a review from a team May 17, 2023 16:45

github-actions bot added the profiling Involves Datadog profiling label May 17, 2023

ivoanjo force-pushed the ivoanjo/prof-7622-profiler-ruby3.3 branch from 7f9bd4e to 21bfb64 Compare May 18, 2023 12:13

ivoanjo mentioned this pull request May 18, 2023

Bootstrap Ruby 3.3 testing (with 3.3.0-preview1) #2863

Merged

GustavoCaso reviewed May 18, 2023

View reviewed changes

ext/ddtrace_profiling_native_extension/extconf.rb Show resolved Hide resolved

GustavoCaso reviewed May 18, 2023

View reviewed changes

ext/ddtrace_profiling_native_extension/private_vm_api_access.c Outdated Show resolved Hide resolved

Update ext/ddtrace_profiling_native_extension/private_vm_api_access.c

e3f9ec5

Minor code tweak Co-authored-by: Gustavo Caso <gustavocaso@gmail.com>

GustavoCaso approved these changes May 18, 2023

View reviewed changes

ivoanjo merged commit 51aeeda into master May 18, 2023
197 checks passed

ivoanjo deleted the ivoanjo/prof-7622-profiler-ruby3.3 branch May 18, 2023 16:51

github-actions bot added this to the 1.12.0 milestone May 18, 2023

ivoanjo mentioned this pull request Jun 27, 2023

Profiler does not work on ruby-head (3.3.0dev) #2685

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PROF-7622] Add support for profiling Ruby 3.3.0-preview1 #2860

[PROF-7622] Add support for profiling Ruby 3.3.0-preview1 #2860

ivoanjo commented May 17, 2023

ivoanjo commented May 17, 2023

GustavoCaso May 18, 2023

ivoanjo May 18, 2023

GustavoCaso May 18, 2023

ivoanjo May 18, 2023 •

edited

GustavoCaso May 18, 2023

ivoanjo May 18, 2023

GustavoCaso May 18, 2023

ivoanjo May 18, 2023

GustavoCaso May 18, 2023

ivoanjo May 18, 2023

GustavoCaso left a comment

[PROF-7622] Add support for profiling Ruby 3.3.0-preview1 #2860

[PROF-7622] Add support for profiling Ruby 3.3.0-preview1 #2860

Conversation

ivoanjo commented May 17, 2023

ivoanjo commented May 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivoanjo May 18, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GustavoCaso left a comment

Choose a reason for hiding this comment

ivoanjo May 18, 2023 •

edited