Skip to content

Commit

Permalink
Bootstrap support for running profiling tests with ruby_memcheck
Browse files Browse the repository at this point in the history
The [`ruby_memcheck`](https://github.com/Shopify/ruby_memcheck) gem
provides a Ruby-specific user-friendly wrapper around valgrind.

In some cases, some of our tests were hanging or timing out due
to how valgrind runs code in a sequential manner. I've added a
new tag to be able to skip such tests.

I also explored using valgrind's `fair-sched` option
(see Shopify/ruby_memcheck#51) but ran
into issues with its output being incomplete, so decided to go
the skip route instead.

You can run the new job with `bundle exec rake spec:profiling:memcheck`.
  • Loading branch information
ivoanjo committed Aug 20, 2024
1 parent b7b99b2 commit 6ba1e5f
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 5 deletions.
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ group :check do
gem 'steep', '~> 1.6.0', require: false
end
gem 'standard', require: false
gem 'ruby_memcheck', '>= 3' if RUBY_VERSION >= '3.4.0' && RUBY_PLATFORM != 'java'
end

group :dev do
Expand Down
31 changes: 31 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@ require 'rspec/core/rake_task'
require 'rake/extensiontask'
require 'yard'
require 'os'
if Gem.loaded_specs.key? 'ruby_memcheck'
require 'ruby_memcheck'
require 'ruby_memcheck/rspec/rake_task'

RubyMemcheck.config(
# If there's an error, print the suppression for that error, to allow us to easily skip such an error if it's
# a false-positive / something in the VM we can't fix.
valgrind_generate_suppressions: true,
# This feature provides better quality data -- I couldn't get good output out of ruby_memcheck without it.
use_only_ruby_free_at_exit: true,
)
end

Dir.glob('tasks/*.rake').each { |r| import r }

Expand Down Expand Up @@ -314,6 +326,25 @@ namespace :spec do
t.rspec_opts = [*args.to_a, '-t ractors'].join(' ')
end

desc 'Run spec:profiling:main tests with memory leak checking'
if Gem.loaded_specs.key?('ruby_memcheck')
RubyMemcheck::RSpec::RakeTask.new(:memcheck) do |t, args|
t.pattern = 'spec/datadog/profiling/**/*_spec.rb,spec/datadog/profiling_spec.rb'
# Some of our specs use multi-threading + busy looping, or multiple processes, or are just really really slow.
# We skip running these when running under valgrind.
# (As a reminder, by default valgrind simulates a sequential/single-threaded execution).
#
# @ivoanjo: I previously tried https://github.com/Shopify/ruby_memcheck/issues/51 but in some cases valgrind
# would give incomplete output, causing a "FATAL: Premature end of data in tag valgrindoutput line 3" error in
# ruby_memcheck. I did not figure out why exactly.
t.rspec_opts = [*args.to_a, '-t ~ractors -t ~memcheck_valgrind_skip'].join(' ')
end
else
task :memcheck do
raise 'Memcheck requires the ruby_memcheck gem to be installed'
end
end

# Make sure each profiling test suite has a dependency on compiled native extensions
Rake::Task[:all].prerequisite_tasks.each { |t| t.enhance([:compile_native_extensions]) }
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@
# See native bits for more details.
let(:options) { {**super(), skip_idle_samples_for_testing: true} }

it "triggers sampling and records the results" do
it "triggers sampling and records the results", :memcheck_valgrind_skip do
start

all_samples = loop_until do
Expand All @@ -182,6 +182,7 @@
it(
"keeps statistics on how many samples were triggered by the background thread, " \
"as well as how many samples were requested from the VM",
:memcheck_valgrind_skip,
) do
start

Expand Down Expand Up @@ -337,7 +338,7 @@
background_thread.join
end

it "is able to sample even when the main thread is sleeping" do
it "is able to sample even when the main thread is sleeping", :memcheck_valgrind_skip do
background_thread
ready_queue.pop

Expand Down Expand Up @@ -546,7 +547,7 @@
# and we clamp it if it goes over the limit.
# But the total amount of allocations recorded should match the number we observed, and thus we record the
# remainder above the clamped value as a separate "Skipped Samples" step.
context "with a high allocation rate" do
context "with a high allocation rate", :memcheck_valgrind_skip do
let(:options) { {**super(), dynamic_sampling_rate_overhead_target_percentage: 0.1} }
let(:thread_that_allocates_as_fast_as_possible) { Thread.new { loop { BasicObject.new } } }

Expand Down
2 changes: 1 addition & 1 deletion spec/datadog/profiling/native_extension_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
it { is_expected.to be false }
end

describe "correctness" do
describe "correctness", :memcheck_valgrind_skip do
let(:ready_queue) { Queue.new }
let(:background_thread) do
Thread.new do
Expand Down
2 changes: 1 addition & 1 deletion spec/datadog/profiling/validate_benchmarks_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require "datadog/profiling/spec_helper"

RSpec.describe "Profiling benchmarks" do
RSpec.describe "Profiling benchmarks", :memcheck_valgrind_skip do
before { skip_if_profiling_not_supported(self) }

around do |example|
Expand Down
17 changes: 17 additions & 0 deletions suppressions/ruby-3.4.supp
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# This is a valgrind suppression configuration file.
#
# We use it together with the ruby_memcheck gem to find issues in the dd-trace-rb native extensions; in some cases
# we need to ignore potential issues as they're not something we can fix (e.g. outside our code.)
#
# See https://valgrind.org/docs/manual/manual-core.html#manual-core.suppress for details.

{
ruby-weak-map
Memcheck:Addr8
fun:wmap_cmp
fun:find_table_bin_ind
fun:st_general_foreach
fun:rb_st_foreach
...
}

0 comments on commit 6ba1e5f

Please sign in to comment.