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

[NO-TICKET] Remove profiler support for Ruby 2.3 and 2.4 #3621

Merged
merged 2 commits into from
May 7, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ documentation.**
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 not break installation on Ruby 2.1 (or the oldest Ruby version we support at the time) on 64-bit ARM macOS,
E.g. the extension must not break installation on Ruby 2.5 (or the oldest Ruby version we support at the time) on 64-bit ARM macOS,
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
Expand Down
32 changes: 9 additions & 23 deletions ext/datadog_profiling_native_extension/extconf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def add_compiler_flag(flag)
# have_library 'pthread'
# have_func 'pthread_getcpuclockid'
# ```
# but a) it broke the build on Windows, b) on older Ruby versions (2.2 and below) and c) It's slower to build
# but it's slower to build
# so instead we just assume that we have the function we need on Linux, and nowhere else
$defs << '-DHAVE_PTHREAD_GETCPUCLOCKID'
end
Expand Down Expand Up @@ -163,6 +163,11 @@ def add_compiler_flag(flag)
# On older Rubies, there was no jit_return member on the rb_control_frame_t struct
$defs << '-DNO_JIT_RETURN' if RUBY_VERSION < '3.1'

# On older Rubies, rb_gc_force_recycle allowed to free objects in a way that
# would be invisible to free tracepoints, finalizers and without cleaning
# obj_to_id_tbl mappings.
$defs << '-DHAVE_WORKING_RB_GC_FORCE_RECYCLE' if RUBY_VERSION < '3.1'

# On older Rubies, we need to use a backported version of this function. See private_vm_api_access.h for details.
$defs << '-DUSE_BACKPORTED_RB_PROFILE_FRAME_METHOD_NAME' if RUBY_VERSION < '3'

Expand All @@ -175,34 +180,15 @@ def add_compiler_flag(flag)
# On older Rubies, objects would not move
$defs << '-DNO_T_MOVED' if RUBY_VERSION < '2.7'

# On older Rubies, there was no RUBY_SEEN_OBJ_ID flag
$defs << '-DNO_SEEN_OBJ_ID_FLAG' if RUBY_VERSION < '2.7'

# On older Rubies, rb_global_vm_lock_struct did not include the owner field
$defs << '-DNO_GVL_OWNER' if RUBY_VERSION < '2.6'

# On older Rubies, there was no thread->invoke_arg
$defs << '-DNO_THREAD_INVOKE_ARG' if RUBY_VERSION < '2.6'

# On older Rubies, we need to use rb_thread_t instead of rb_execution_context_t
$defs << '-DUSE_THREAD_INSTEAD_OF_EXECUTION_CONTEXT' if RUBY_VERSION < '2.5'

# On older Rubies, extensions can't use GET_VM()
$defs << '-DNO_GET_VM' if RUBY_VERSION < '2.5'

# On older Rubies...
if RUBY_VERSION < '2.4'
# ...we need to use RUBY_VM_NORMAL_ISEQ_P instead of VM_FRAME_RUBYFRAME_P
$defs << '-DUSE_ISEQ_P_INSTEAD_OF_RUBYFRAME_P'
# ...we use a legacy copy of rb_vm_frame_method_entry
$defs << '-DUSE_LEGACY_RB_VM_FRAME_METHOD_ENTRY'
end

# On older Rubies, rb_gc_force_recycle allowed to free objects in a way that
# would be invisible to free tracepoints, finalizers and without cleaning
# obj_to_id_tbl mappings.
$defs << '-DHAVE_WORKING_RB_GC_FORCE_RECYCLE' if RUBY_VERSION < '3.1'

# On older Rubies, there was no RUBY_SEEN_OBJ_ID flag
$defs << '-DNO_SEEN_OBJ_ID_FLAG' if RUBY_VERSION < '2.7'

# If we got here, libdatadog is available and loaded
ENV['PKG_CONFIG_PATH'] = "#{ENV['PKG_CONFIG_PATH']}:#{Libdatadog.pkgconfig_folder}"
Logging.message("[datadog] PKG_CONFIG_PATH set to #{ENV['PKG_CONFIG_PATH'].inspect}\n")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ def self.unsupported_reason
on_macos? ||
on_unknown_os? ||
on_unsupported_cpu_arch? ||
on_unsupported_ruby_version? ||
expected_to_use_mjit_but_mjit_is_disabled? ||
libdatadog_not_available? ||
libdatadog_not_usable?
Expand Down Expand Up @@ -260,15 +259,6 @@ def self.pkg_config_missing?(command: $PKGCONFIG) # rubocop:disable Style/Global
architecture_not_supported unless RUBY_PLATFORM.start_with?('x86_64', 'aarch64', 'arm64')
end

private_class_method def self.on_unsupported_ruby_version?
ruby_version_not_supported = explain_issue(
'the profiler only supports Ruby 2.3 or newer.',
suggested: UPGRADE_RUBY,
)

ruby_version_not_supported if RUBY_VERSION.start_with?('2.1.', '2.2.')
end

# On some Rubies, we require the mjit header to be present. If Ruby was installed without MJIT support, we also skip
# building the extension.
private_class_method def self.expected_to_use_mjit_but_mjit_is_disabled?
Expand Down
111 changes: 25 additions & 86 deletions ext/datadog_profiling_native_extension/private_vm_api_access.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,7 @@ bool is_current_thread_holding_the_gvl(void) {
}
#else
current_gvl_owner gvl_owner(void) {
rb_vm_t *vm =
#ifndef NO_GET_VM
GET_VM();
#else
thread_struct_from_object(rb_thread_current())->vm;
#endif
rb_vm_t *vm = GET_VM();

// BIG Issue: Ruby < 2.6 did not have the owner field. The really nice thing about the owner field is that it's
// "atomic" -- when a thread sets it, it "declares" two things in a single step
Expand Down Expand Up @@ -163,7 +158,7 @@ bool is_current_thread_holding_the_gvl(void) {
//
// Thus an incorrect `is_current_thread_holding_the_gvl` result may lead to issues inside `rb_postponed_job_register_one`.
//
// For this reason we currently do not enable the new Ruby profiler on Ruby 2.5 and below by default, and we print a
// For this reason we currently do not enable the new Ruby profiler on Ruby 2.5 by default, and we print a
// warning when customers force-enable it.
bool gvl_acquired = vm->gvl.acquired != 0;
rb_thread_t *current_owner = vm->running_thread;
Expand Down Expand Up @@ -213,12 +208,7 @@ uint64_t native_thread_id_for(VALUE thread) {
// Returns the stack depth by using the same approach as rb_profile_frames and backtrace_each: get the positions
// of the end and current frame pointers and subtracting them.
ptrdiff_t stack_depth_for(VALUE thread) {
#ifndef USE_THREAD_INSTEAD_OF_EXECUTION_CONTEXT // Modern Rubies
const rb_execution_context_t *ec = thread_struct_from_object(thread)->ec;
#else // Ruby < 2.5
const rb_thread_t *ec = thread_struct_from_object(thread);
#endif

const rb_execution_context_t *ec = thread_struct_from_object(thread)->ec;
const rb_control_frame_t *cfp = ec->cfp, *end_cfp = RUBY_VM_END_CONTROL_FRAME(ec);

if (end_cfp == NULL) return 0;
Expand Down Expand Up @@ -253,12 +243,7 @@ void ddtrace_thread_list(VALUE result_array) {
rb_ractor_t *current_ractor = ddtrace_get_ractor();
ccan_list_for_each(&current_ractor->threads.set, thread, lt_node) {
#else
rb_vm_t *vm =
#ifndef NO_GET_VM
GET_VM();
#else
thread_struct_from_object(rb_thread_current())->vm;
#endif
rb_vm_t *vm = GET_VM();
list_for_each(&vm->living_threads, thread, vmlt_node) {
#endif
switch (thread->status) {
Expand Down Expand Up @@ -394,13 +379,6 @@ calc_lineno(const rb_iseq_t *iseq, const VALUE *pc)
// * Add thread argument
// * Add is_ruby_frame argument
// * Removed `if (lines)` tests -- require/assume that like `buff`, `lines` is always specified
// * Support Ruby < 2.5 by using rb_thread_t instead of rb_execution_context_t (which did not exist and was just
// part of rb_thread_t)
// * Support Ruby < 2.4 by using `RUBY_VM_NORMAL_ISEQ_P(cfp->iseq)` instead of `VM_FRAME_RUBYFRAME_P(cfp)`.
// Given that the Ruby 2.3 version of `rb_profile_frames` did not support native methods and thus did not need this
// check, how did I figure out what to replace it with? I did it by looking at other places in the VM code where the
// code looks exactly the same but Ruby 2.4 uses `VM_FRAME_RUBYFRAME_P` whereas Ruby 2.3 used `RUBY_VM_NORMAL_ISEQ_P`.
// Examples of these are `errinfo_place` in `eval.c`, `rb_vm_get_ruby_level_next_cfp` (among others) in `vm.c`, etc.
// * Skip dummy frame that shows up in main thread
// * Add `end_cfp == NULL` and `end_cfp <= cfp` safety checks. These are used in a bunch of places in
// `vm_backtrace.c` (`backtrace_each`, `backtrace_size`, `rb_ec_partial_backtrace_object`) but are conspicuously
Expand Down Expand Up @@ -449,11 +427,7 @@ int ddtrace_rb_profile_frames(VALUE thread, int start, int limit, VALUE *buff, i
// Modified from upstream: Instead of using `GET_EC` to collect info from the current thread,
// support sampling any thread (including the current) passed as an argument
rb_thread_t *th = thread_struct_from_object(thread);
#ifndef USE_THREAD_INSTEAD_OF_EXECUTION_CONTEXT // Modern Rubies
const rb_execution_context_t *ec = th->ec;
#else // Ruby < 2.5
const rb_thread_t *ec = th;
#endif
const rb_execution_context_t *ec = th->ec;
const rb_control_frame_t *cfp = ec->cfp, *end_cfp = RUBY_VM_END_CONTROL_FRAME(ec);
#ifndef NO_JIT_RETURN
const rb_control_frame_t *top = cfp;
Expand Down Expand Up @@ -499,11 +473,7 @@ int ddtrace_rb_profile_frames(VALUE thread, int start, int limit, VALUE *buff, i
// rb_profile_frames does not do this check, but `backtrace_each` (`vm_backtrace.c`) does. This frame is not
// exposed by the Ruby backtrace APIs and for now we want to match its behavior 1:1
}
#ifndef USE_ISEQ_P_INSTEAD_OF_RUBYFRAME_P // Modern Rubies
else if (VM_FRAME_RUBYFRAME_P(cfp)) {
#else // Ruby < 2.4
else if (RUBY_VM_NORMAL_ISEQ_P(cfp->iseq)) {
#endif
if (start > 0) {
start--;
continue;
Expand Down Expand Up @@ -719,51 +689,27 @@ check_method_entry(VALUE obj, int can_be_svar)
}
}

#ifndef USE_LEGACY_RB_VM_FRAME_METHOD_ENTRY
// Taken from upstream vm_insnhelper.c at commit 5f10bd634fb6ae8f74a4ea730176233b0ca96954 (March 2022, Ruby 3.2 trunk)
// Copyright (C) 2007 Koichi Sasada
// to support our custom rb_profile_frames (see above)
//
// While older Rubies may have this function, the symbol is not exported which leads to dynamic loader issues, e.g.
// `dyld: lazy symbol binding failed: Symbol not found: _rb_vm_frame_method_entry`.
//
// Modifications: None
MJIT_STATIC const rb_callable_method_entry_t *
rb_vm_frame_method_entry(const rb_control_frame_t *cfp)
{
const VALUE *ep = cfp->ep;
rb_callable_method_entry_t *me;

while (!VM_ENV_LOCAL_P(ep)) {
if ((me = check_method_entry(ep[VM_ENV_DATA_INDEX_ME_CREF], FALSE)) != NULL) return me;
ep = VM_ENV_PREV_EP(ep);
}

return check_method_entry(ep[VM_ENV_DATA_INDEX_ME_CREF], TRUE);
}
#else
// Taken from upstream vm_insnhelper.c at commit 556e9f726e2b80f6088982c6b43abfe68bfad591 (October 2018, ruby_2_3 branch)
// Copyright (C) 2007 Koichi Sasada
// to support our custom rb_profile_frames (see above)
//
// Quite a few macros in this function changed after Ruby 2.3. Rather than trying to fix the Ruby 3.2 version to work
// with 2.3 constants, I decided to import the Ruby 2.3 version.
//
// Modifications: None
const rb_callable_method_entry_t *
rb_vm_frame_method_entry(const rb_control_frame_t *cfp)
{
VALUE *ep = cfp->ep;
rb_callable_method_entry_t *me;
// Taken from upstream vm_insnhelper.c at commit 5f10bd634fb6ae8f74a4ea730176233b0ca96954 (March 2022, Ruby 3.2 trunk)
// Copyright (C) 2007 Koichi Sasada
// to support our custom rb_profile_frames (see above)
//
// While older Rubies may have this function, the symbol is not exported which leads to dynamic loader issues, e.g.
// `dyld: lazy symbol binding failed: Symbol not found: _rb_vm_frame_method_entry`.
//
// Modifications: None
MJIT_STATIC const rb_callable_method_entry_t *
rb_vm_frame_method_entry(const rb_control_frame_t *cfp)
{
const VALUE *ep = cfp->ep;
rb_callable_method_entry_t *me;

while (!VM_EP_LEP_P(ep)) {
if ((me = check_method_entry(ep[-1], FALSE)) != NULL) return me;
ep = VM_EP_PREV_EP(ep);
}
while (!VM_ENV_LOCAL_P(ep)) {
if ((me = check_method_entry(ep[VM_ENV_DATA_INDEX_ME_CREF], FALSE)) != NULL) return me;
ep = VM_ENV_PREV_EP(ep);
}

return check_method_entry(ep[-1], TRUE);
}
#endif // USE_LEGACY_RB_VM_FRAME_METHOD_ENTRY
return check_method_entry(ep[VM_ENV_DATA_INDEX_ME_CREF], TRUE);
}
#endif // RUBY_MJIT_HEADER

#ifndef NO_RACTORS
Expand Down Expand Up @@ -880,13 +826,6 @@ static inline int ddtrace_imemo_type(VALUE imemo) {
// This is used to workaround a VM bug. See "handle_sampling_signal" in "collectors_cpu_and_wall_time_worker" for details.
#ifdef NO_POSTPONED_TRIGGER
void *objspace_ptr_for_gc_finalize_deferred_workaround(void) {
rb_vm_t *vm =
#ifndef NO_GET_VM // TODO: Inline GET_VM below once we drop support in dd-trace-rb 2.x for < Ruby 2.5
GET_VM();
#else
thread_struct_from_object(rb_thread_current())->vm;
#endif

return vm->objspace;
return GET_VM()->objspace;
}
#endif
5 changes: 0 additions & 5 deletions ext/datadog_profiling_native_extension/ruby_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ static inline VALUE process_pending_interruptions(DDTRACE_UNUSED VALUE _) {
return Qnil;
}

// RB_UNLIKELY is not supported on Ruby 2.3
#ifndef RB_UNLIKELY
#define RB_UNLIKELY(x) x
#endif

// Calls process_pending_interruptions BUT "rescues" any exceptions to be raised, returning them instead as
// a non-zero `pending_exception`.
//
Expand Down
3 changes: 1 addition & 2 deletions integration/apps/rack/app/acme.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,7 @@ def detailed_check(request)
[200, { 'content-type' => 'application/json'}, [JSON.pretty_generate(
webserver_process: $PROGRAM_NAME,
profiler_available: Datadog::Profiling.start_if_enabled,
# NOTE: Threads can't be named on Ruby 2.1 and 2.2
profiler_threads: ((Thread.list.map(&:name).select { |it| it && it.include?('Profiling') }) unless RUBY_VERSION < '2.3')
profiler_threads: Thread.list.map(&:name).select { |it| it && it.include?('Profiling') },
)], "\n"]
end
end
Expand Down
3 changes: 1 addition & 2 deletions integration/apps/rack/app/resque_background_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ def self.perform(key, value)
value: value,
resque_process: $PROGRAM_NAME,
profiler_available: Datadog::Profiling.start_if_enabled,
# NOTE: Threads can't be named on Ruby 2.1 and 2.2
profiler_threads: ((Thread.list.map(&:name).select { |it| it && it.include?('Profiling') }) unless RUBY_VERSION < '2.3')
profiler_threads: Thread.list.map(&:name).select { |it| it && it.include?('Profiling') },
))
end
end
3 changes: 1 addition & 2 deletions integration/apps/rack/app/sidekiq_background_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ def perform(key, value)
value: value,
sidekiq_process: $PROGRAM_NAME,
profiler_available: Datadog::Profiling.start_if_enabled,
# NOTE: Threads can't be named on Ruby 2.1 and 2.2
profiler_threads: ((Thread.list.map(&:name).select { |it| it && it.include?('Profiling') }) unless RUBY_VERSION < '2.3')
profiler_threads: Thread.list.map(&:name).select { |it| it && it.include?('Profiling') },
))
end
end
2 changes: 1 addition & 1 deletion integration/apps/rack/spec/integration/basic_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
it { is_expected.to be_a_kind_of(Net::HTTPOK) }
end

let(:expected_profiler_available) { RUBY_VERSION >= '2.3' }
let(:expected_profiler_available) { true }

let(:expected_profiler_threads) do
expected_profiler_available ? contain_exactly(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
require 'resque'

Resque.redis = ENV['REDIS_URL']
# Resque.redis = 'redis:6379' # Ruby 2.2 compatibility
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
require 'resque'

Resque.redis = ENV['REDIS_URL']
# Resque.redis = 'redis:6379' # Ruby 2.2 compatibility
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

let(:json_result) { JSON.parse(subject.body, symbolize_names: true) }

let(:expected_profiler_available) { RUBY_VERSION >= '2.3' }
let(:expected_profiler_available) { true }

let(:expected_profiler_threads) do
expected_profiler_available ? contain_exactly(
Expand Down
1 change: 0 additions & 1 deletion integration/apps/rails-six/config/initializers/resque.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
require 'resque'

Resque.redis = ENV['REDIS_URL']
# Resque.redis = 'redis:6379' # Ruby 2.2 compatibility
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

let(:json_result) { JSON.parse(subject.body, symbolize_names: true) }

let(:expected_profiler_available) { RUBY_VERSION >= '2.3' }
let(:expected_profiler_available) { true }

let(:expected_profiler_threads) do
expected_profiler_available ? contain_exactly(
Expand Down
2 changes: 1 addition & 1 deletion integration/apps/sinatra2-classic/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ gem 'pry-byebug'
# gem 'ruby-prof'
gem 'rspec'
gem 'rspec-wait'
gem 'webrick' if RUBY_VERSION >= '2.3' # Older Rubies can just use the built-in version of webrick
gem 'webrick'
gem 'rackup'
7 changes: 1 addition & 6 deletions integration/apps/sinatra2-classic/app/acme.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,7 @@
JSON.generate(
webserver_process: $PROGRAM_NAME,
profiler_available: Datadog::Profiling.start_if_enabled,
# NOTE: Threads can't be named on Ruby 2.1 and 2.2
profiler_threads: (unless RUBY_VERSION < '2.3'
(Thread.list.map(&:name).select do |it|
it && it.include?('Profiling')
end)
end)
profiler_threads: Thread.list.map(&:name).select { |it| it && it.include?('Profiling') },
)
]
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
it { is_expected.to be_a_kind_of(Net::HTTPOK) }
end

let(:expected_profiler_available) { RUBY_VERSION >= '2.3' }
let(:expected_profiler_available) { true }

let(:expected_profiler_threads) do
expected_profiler_available ? contain_exactly(
Expand Down
5 changes: 1 addition & 4 deletions integration/apps/sinatra2-modular/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@ gem 'datadog', *Datadog::DemoEnv.gem_spec('datadog')

# Development
gem 'pry-byebug'
# gem 'pry-stack_explorer', platform: :ruby
# gem 'rbtrace'
# gem 'ruby-prof'
gem 'rspec'
gem 'rspec-wait'
gem 'webrick' if RUBY_VERSION >= '2.3' # Older Rubies can just use the built-in version of webrick
gem 'webrick'
gem 'rackup'
Loading
Loading