8000 [PROF-11809] Fix profiler compatibility with ruby-head (3.5) · DataDog/dd-trace-rb@5a10acb · GitHub
[go: up one dir, main page]
Skip to content

Commit 5a10acb

Browse files
ivoanjoivoanjo
committed
[PROF-11809] Fix profiler compatibility with ruby-head (3.5)
**What does this PR do?** This PR fixes three issues in the profiler when used with latest ruby-head: 1. It's no longer possible to ask the object_id from a T_IMEMO object. This showed up as a Ruby VM crash with an error message "T_IMEMO can't have an object_id". (See ruby/ruby#13347 for the upstream change) 2. Creating new instances of a class is now inlined into the caller, and there is no longer a frame to represent the new. This broke some of our tests that expected the stack from allocating an object to have the `new` at the top. (See ruby/ruby#13080 for the upstream change) 3. Object ids now count towards the size of objects. This broke some of our tests that asserted on size of profiled objects. (See ruby/ruby#13159 for the upstream change) **Motivation:** Fix support for Ruby 3.5. **Additional Notes:** N/A **How to test the change?** I've updated our specs to cover these changes. Unfortunately, we don't yet test with Ruby 3.5 in CI, so you'll have to test manually if you want to see the fixes working with 3.5. (Note that these changes showed up after 3.5.0-preview1, so testing on -preview1 is not enough)
1 parent 209bb9a commit 5a10acb

File tree

3 files changed

+32
-6
lines changed

3 files changed

+32
-6
lines changed

‎ext/datadog_profiling_native_extension/extconf.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ def skip_building_extension!(reason)
131131

132132
have_func "malloc_stats"
133133

134+
# On Ruby 3.5, we can't ask the object_id from IMEMOs (https://github.com/ruby/ruby/pull/13347)
135+
$defs << "-DNO_IMEMO_OBJECT_ID" unless RUBY_VERSION < "3.5"
136+
134137
# On Ruby 2.5 and 3.3, this symbol was not visible. It is on 2.6 to 3.2, as well as 3.4+
135138
$defs << "-DNO_RB_OBJ_INFO" if RUBY_VERSION.start_with?("2.5", "3.3")
136139

‎ext/datadog_profiling_native_extension/heap_recorder.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,14 @@ void start_heap_allocation_recording(heap_recorder *heap_recorder, VALUE new_obj
314314
rb_raise(rb_eRuntimeError, "Detected consecutive heap allocation recording starts without end.");
315315
}
316316

317-
if (++heap_recorder->num_recordings_skipped < heap_recorder->sample_rate) {
317+
if (++heap_recorder->num_recordings_skipped < heap_recorder->sample_rate ||
318+
#ifdef NO_IMEMO_OBJECT_ID
319+
// On Ruby 3.5, we can't ask the object_id from IMEMOs (https://github.com/ruby/ruby/pull/13347)
320+
RB_BUILTIN_TYPE(new_obj) == RUBY_T_IMEMO
321+
#else
322+
false
323+
#endif
324+
) {
318325
heap_recorder->active_recording = &SKIPPED_RECORD;
319326
return;
320327
}

‎spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -894,27 +894,43 @@
894894

895895
cpu_and_wall_time_worker.stop
896896

897+
current_method_name = caller_locations(0, 1).first.base_label
898+
897899
test_struct_heap_sample = lambda { |sample|
898900
first_frame = sample.locations.first
899901
first_frame.lineno == allocation_line &&
900902
first_frame.path == __FILE__ &&
901-
first_frame.base_label == "new" &&
903+
(
904+
first_frame.base_label == "new" ||
905+
# From Ruby 3.5 onwards, new is inlined into the bytecode of the caller and there's no "new"
906+
# frame, see https://github.com/ruby/ruby/pull/13080
907+
(RUBY_VERSION >= "3.5.0" && first_frame.base_label == current_method_name)
908+
) &&
902909
sample.labels[:"allocation class"] == "CpuAndWallTimeWorkerSpec::TestStruct" &&
903910
(sample.values[:"heap-live-samples"] || 0) > 0
904911
}
905912

906913
# We can't just use find here because samples might have different gc age labels
907914
# if a gc happens to run in the middle of this test. Thus, we'll have to sum up
908915
# together the values of all matching samples.
909-
relevant_samples = samples_from_pprof(recorder.serialize!)
910-
.select(&test_struct_heap_sample)
916+
relevant_samples = samples_from_pprof(recorder.serialize!).select(&test_struct_heap_sample)
911917

912918
total_samples = relevant_samples.map { |sample| sample.values[:"heap-live-samples"] || 0 }.reduce(:+)
913919
total_size = relevant_samples.map { |sample| sample.values[:"heap-live-size"] || 0 }.reduce(:+)
914920

915921
expect(total_samples).to eq test_num_allocated_object
916-
# 40 is the size of a basic object and we have test_num_allocated_object of them
917-
expect(total_size).to eq test_num_allocated_object * 40
922+
923+
expected_size_of_object = 40 # 40 is the size of a basic object and we have test_num_allocated_object of them
924+
925+
# Starting with Ruby 3.5, the object_id counts towards the object's size
926+
if RUBY_VERSION >= "3.5.0"
927+
expected_size_of_object = 104
928+
929+
expect(ObjectSpace.memsize_of(CpuAndWallTimeWorkerSpec::TestStruct.new.tap(&:object_id)))
930+
.to be expected_size_of_object
931+
end
932+
933+
expect(total_size).to eq test_num_allocated_object * expected_size_of_object
918934
end
919935

920936
describe "heap cleanup after GC" do

0 commit comments

Comments
 (0)
0