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

[PROF-4198] Gather CPU time in profiler without monkey patching Thread for all supported Ruby versions (2.1 to 3.x) #1740

Merged
merged 14 commits into from
Nov 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test-cross-platform.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
fail-fast: false
matrix:
os: [windows-latest]
ruby: [2.1, 2.2, 2.3, 2.4, 2.5, 2.6, 2.7, '3.0', head]
ruby: [2.1, 2.2, 2.3, 2.4, 2.5, 2.6, 2.7, '3.0']
runs-on: ${{ matrix.os }}
env:
SKIP_SIMPLECOV: 1
Expand Down
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -375,3 +375,6 @@ Style/TrailingCommaInHashLiteral:
Style/GlobalVars:
Exclude:
- 'ext/ddtrace_profiling_native_extension/extconf.rb'

Naming/VariableNumber:
Enabled: false
7 changes: 5 additions & 2 deletions ddtrace.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,11 @@ Gem::Specification.new do |spec|
spec.add_dependency 'msgpack', '< 1.4'
end

# Used by the profiler
spec.add_dependency 'ffi', '~> 1.0'
# Used by the profiler native extension to support older Rubies (see NativeExtensionDesign.md for notes)
#
# Because we only use this for older Rubies, and we consider it "feature-complete" for those older Rubies,
# we're pinning it at the latest available version and will manually bump the dependency as needed.
spec.add_dependency 'debase-ruby_core_source', '= 0.10.12'

spec.extensions = ['ext/ddtrace_profiling_native_extension/extconf.rb']
end
36 changes: 27 additions & 9 deletions ext/ddtrace_profiling_native_extension/NativeExtensionDesign.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ in future releases -- e.g. disabling the extension will disable profiling entire
The profiling native extension is (and must always be) designed to **not cause failures** during gem installation, even
if some features, Ruby versions, or operating systems are not supported.

E.g. the extension must cleanly build on Ruby 2.1 on Windows, even if at run time it will effectively do nothing for
such a setup.
E.g. the extension must cleanly build on Ruby 2.1 (or the oldest Ruby version we support at the time) on Windows,
even if at run time it will effectively do nothing for such a setup.

We have a CI setup to help validate this, but this is really important to keep in mind when adding to or changing the
existing codebase.
Expand All @@ -32,17 +32,35 @@ existing codebase.
To implement some of the features below, we sometimes require access to private Ruby header files (that describe VM
internal types, structures and functions).

Because these private header files are not included in regular Ruby installations, our current workaround is to piggy
back on a special header that Ruby includes that is only intended for use by the Ruby MJIT compiler. This header is
placed inside the `include/` directory in a Ruby installation, and is named for that specific Ruby version. e.g.
`rb_mjit_min_header-2.7.4.h`.
Because these private header files are not included in regular Ruby installations, we have two different workarounds:

This header was introduced by the first Ruby version that added MJIT, which is 2.6+.

For older Ruby versions (see safety section above), this header is not available, and this must be handled gracefully.
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

Functions which make use of these headers are defined in the <private_vm_api_acccess.c> file.

**Important Note**: Our medium/long-term plan is to stop relying on all private Ruby headers, and instead request and
contribute upstream changes so that they become official public VM APIs.

### Approach 1: Using the Ruby private MJIT header

Ruby versions >= 2.6 introduced a JIT compiler called MJIT. This compiler does not directly generate machine code;
instead it generates C code and uses the system C compiler to turn it into machine code.

The generated C code `#include`s a private header -- which we reference as "the MJIT header" everywhere.
The MJIT header gets shipped with all MJIT-enabled Rubies and includes the layout of many internal VM structures;
and of course the intention is that it is only used by the Ruby MJIT compiler.

This header is placed inside the `include/` directory in a Ruby installation, and is named for that specific Ruby
version. e.g. `rb_mjit_min_header-2.7.4.h`.

### Approach 2: Using the `debase-ruby_core_source` gem

The [`debase-ruby_core_source`](https://github.com/ruby-debug/debase-ruby_core_source) contains almost no code;
instead, it just contains per-Ruby-version folders with the private VM headers (`.h`) files for that version.

Thus, even though a regular Ruby installation does not include these files, we can access the copy inside this gem.

## Feature: Getting thread CPU-time clock_ids

* **OS support**: Linux
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,19 @@

// This file is only compiled on systems where pthread_getcpuclockid() is available;
// Otherwise we compile clock_id_noop.c
#if defined(HAVE_PTHREAD_GETCPUCLOCKID) && defined(USE_MJIT_HEADER)
#ifdef HAVE_PTHREAD_GETCPUCLOCKID

#include <pthread.h>
#include <time.h>
#include <errno.h>

#include <ruby.h>

#ifdef RUBY_2_1_WORKAROUND
#include <thread_native.h>
#else
#include <ruby/thread_native.h>
#endif

#include "private_vm_api_access.h"

Expand Down
2 changes: 1 addition & 1 deletion ext/ddtrace_profiling_native_extension/clock_id_noop.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

// This file is the dual of clock_id_from_pthread.c for systems where that info
// is not available.
#if !(defined(HAVE_PTHREAD_GETCPUCLOCKID) && defined(USE_MJIT_HEADER))
#ifndef HAVE_PTHREAD_GETCPUCLOCKID

#include <ruby.h>

Expand Down
84 changes: 61 additions & 23 deletions ext/ddtrace_profiling_native_extension/extconf.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
# typed: false
# typed: ignore

def skip_building_extension?
# We don't support JRuby for profiling, and JRuby doesn't support native extensions, so let's just skip this entire
# thing so that JRuby users of dd-trace-rb aren't impacted.
on_jruby = RUBY_ENGINE == 'jruby'

# We don't officially support TruffleRuby for dd-trace-rb at all BUT let's not break adventurous customers that
# want to give it a try.
on_truffleruby = RUBY_ENGINE == 'truffleruby'

# Experimental toggle to disable building the extension.
# Disabling the extension will lead to the profiler not working in future releases.
# If you needed to use this, please tell us why on <https://github.com/DataDog/dd-trace-rb/issues/new>.
disabled_via_env = ENV['DD_PROFILING_NO_EXTENSION'].to_s.downcase == 'true'

on_jruby || disabled_via_env
on_jruby || on_truffleruby || disabled_via_env
end

# IMPORTANT: When adding flags, remember that our customers compile with a wide range of gcc/clang versions, so
Expand All @@ -20,11 +24,19 @@ def add_compiler_flag(flag)
end

if skip_building_extension?
# rubocop:disable Style/StderrPuts
$stderr.puts(%(
+------------------------------------------------------------------------------+
| Skipping build of profiling native extension and replacing it with a no-op |
| Makefile |
+------------------------------------------------------------------------------+

))

File.write('Makefile', 'all install clean: # dummy makefile that does nothing')
return
end

# rubocop:disable Style/StderrPuts
$stderr.puts(%(
+------------------------------------------------------------------------------+
| **Preparing to build the ddtrace native extension...** |
Expand All @@ -40,6 +52,7 @@ def add_compiler_flag(flag)
| |
| Thanks for using ddtrace! You rock! |
+------------------------------------------------------------------------------+

))
# rubocop:enable Style/StderrPuts

Expand All @@ -57,10 +70,6 @@ def add_compiler_flag(flag)
# cause a segfault later. Let's ensure that never happens.
add_compiler_flag '-Werror-implicit-function-declaration'

# Older Rubies don't have the MJIT header (used by the JIT compiler, and we piggy back on it)
# TODO: Development builds of Ruby 3.1 seem to be failing on Windows; to be revisited once 3.1.0 stable is out
$defs << '-DUSE_MJIT_HEADER' unless RUBY_VERSION < '2.6' || (RUBY_VERSION >= '3.1' && Gem.win_platform?)

if RUBY_PLATFORM.include?('linux')
# Supposedly, the correct way to do this is
# ```
Expand All @@ -72,24 +81,53 @@ def add_compiler_flag(flag)
$defs << '-DHAVE_PTHREAD_GETCPUCLOCKID'
end

create_header

# The MJIT header is always (afaik?) suffixed with the exact Ruby VM version,
# including patch (e.g. 2.7.2). Thus, we add to the header file a definition
# containing the exact file, so that it can be used in a #include in the C code.
header_contents =
File.read($extconf_h)
.sub('#endif',
<<-EXTCONF_H.strip
#define RUBY_MJIT_HEADER "rb_mjit_min_header-#{RUBY_VERSION}.h"

#endif
EXTCONF_H
)
File.open($extconf_h, 'w') { |file| file.puts(header_contents) }
# 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'

# Tag the native extension library with the Ruby version and Ruby platform.
# This makes it easier for development (avoids "oops I forgot to rebuild when I switched my Ruby") and ensures that
# the wrong library is never loaded.
# When requiring, we need to use the exact same string, including the version and the platform.
create_makefile "ddtrace_profiling_native_extension.#{RUBY_VERSION}_#{RUBY_PLATFORM}"
EXTENSION_NAME = "ddtrace_profiling_native_extension.#{RUBY_VERSION}_#{RUBY_PLATFORM}".freeze

if CAN_USE_MJIT_HEADER
$defs << '-DUSE_MJIT_HEADER'

# NOTE: This needs to come after all changes to $defs
create_header

# The MJIT header is always (afaik?) suffixed with the exact Ruby VM version,
# including patch (e.g. 2.7.2). Thus, we add to the header file a definition
# containing the exact file, so that it can be used in a #include in the C code.
header_contents =
File.read($extconf_h)
.sub('#endif',
<<-EXTCONF_H.strip
#define RUBY_MJIT_HEADER "rb_mjit_min_header-#{RUBY_VERSION}.h"

#endif
EXTCONF_H
)
File.open($extconf_h, 'w') { |file| file.puts(header_contents) }

create_makefile EXTENSION_NAME
else
# On older Rubies, we use the debase-ruby_core_source gem to get access to private VM headers.
# This gem ships source code copies of these VM headers for the different Ruby VM versions;
# see https://github.com/ruby-debug/debase-ruby_core_source for details

thread_native_for_ruby_2_1 = proc { true }
if RUBY_VERSION < '2.2'
# This header became public in Ruby 2.2, but we need to pull it from the private headers folder for 2.1
thread_native_for_ruby_2_1 = proc { have_header('thread_native.h') }
$defs << '-DRUBY_2_1_WORKAROUND'
end

create_header

require 'debase/ruby_core_source'
dir_config('ruby') # allow user to pass in non-standard core include directory

Debase::RubyCoreSource
.create_makefile_with_core(proc { have_header('vm_core.h') && thread_native_for_ruby_2_1.call }, EXTENSION_NAME)
end
13 changes: 11 additions & 2 deletions ext/ddtrace_profiling_native_extension/private_vm_api_access.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,19 @@

// This file exports functions used to access private Ruby VM APIs and internals.
// To do this, it imports a few VM internal (private) headers.
// Be very careful when changing things here :)
//
// **Important Note**: Our medium/long-term plan is to stop relying on all private Ruby headers, and instead request and
// contribute upstream changes so that they become official public VM APIs.
//
// In the meanwhile, be very careful when changing things here :)

#ifdef USE_MJIT_HEADER
// Pick up internal structures from the private Ruby MJIT header file
#include RUBY_MJIT_HEADER
#else
// On older Rubies, use a copy of the VM internal headers shipped in the debase-ruby_core_source gem
#include <vm_core.h>
#endif

// MRI has a similar rb_thread_ptr() function which we can't call it directly
// because Ruby does not expose the thread_data_type publicly.
Expand All @@ -23,4 +33,3 @@ static inline struct rb_thread_struct *thread_struct_from_object(VALUE thread) {
rb_nativethread_id_t pthread_id_for(VALUE thread) {
return thread_struct_from_object(thread)->thread_id;
}
#endif
2 changes: 0 additions & 2 deletions lib/ddtrace/profiling.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,7 @@ def self.unsupported_reason
private_class_method def self.load_profiling
return false unless supported?

require 'ddtrace/profiling/ext/cpu'
require 'ddtrace/profiling/ext/forking'

require 'ddtrace/profiling/collectors/stack'
require 'ddtrace/profiling/exporter'
require 'ddtrace/profiling/recorder'
Expand Down
Loading