Merge pull request #4460 from DataDog/ivoanjo/prof-11394-heap-profiling-graduate-try2

[PROF-11405] Graduate heap profiling from alpha to preview, second try

Author: ivoanjo

Rakefile

@@ -453,13 +453,13 @@ namespace :changelog do
 end
 
 NATIVE_EXTS = [
+  Rake::ExtensionTask.new("libdatadog_api.#{RUBY_VERSION[/\d+.\d+/]}_#{RUBY_PLATFORM}") do |ext|
+    ext.ext_dir = 'ext/libdatadog_api'
+  end,
+
   Rake::ExtensionTask.new("datadog_profiling_native_extension.#{RUBY_VERSION}_#{RUBY_PLATFORM}") do |ext|
     ext.ext_dir = 'ext/datadog_profiling_native_extension'
   end,
-
-  Rake::ExtensionTask.new("libdatadog_api.#{RUBY_VERSION[/\d+.\d+/]}_#{RUBY_PLATFORM}") do |ext|
-    ext.ext_dir = 'ext/libdatadog_api'
-  end
 ].freeze
 
 NATIVE_CLEAN = ::Rake::FileList[]

benchmarks/README.md

@@ -4,22 +4,16 @@
 
 1. Use one of the following prefixes:
 
-  - `library_`
-  - `profiling_`
-  - `tracing_`
-  - `di_`
-  - `error_tracking_`
+  - `library_` (spec in `spec/validate_benchmarks_spec.rb`)
+  - `profiling_` (spec in `./spec/datadog/profiling/validate_benchmarks_spec.rb`)
+  - `tracing_` (spec in `./spec/datadog/tracing/validate_benchmarks_spec.rb`)
+  - `di_` (spec in `./spec/datadog/di/validate_benchmarks_spec.rb`)
+  - `error_tracking` (spec in `./spec/datadog/error_tracing/validate_benchmarks_spec.rb`)
 
 2. Add the new file to `run_all.sh` in this directory.
 
 3. Depending on the prefix, add the new file to the correct
-  `validate_benchmarks_spec.rb` as follows:
-
-  - `library_` prefix: `spec/validate_benchmarks_spec.rb`
-  - `profiling_` prefix: `./spec/datadog/profiling/validate_benchmarks_spec.rb`
-  - `tracing_` prefix: `./spec/datadog/tracing/validate_benchmarks_spec.rb`
-  - `di_` prefix: `./spec/datadog/di/validate_benchmarks_spec.rb`
-  - `error_tracking_` prefix: `./spec/datadog/error_tracing/validate_benchmarks_spec.rb`
+  `validate_benchmarks_spec.rb` as listed above
 
 ## Adding Benchmarks For a New Product
 

benchmarks/profiling_string_storage_intern.rb

@@ -0,0 +1,66 @@
+# Used to quickly run benchmark under RSpec as part of the usual test suite, to validate it didn't bitrot
+VALIDATE_BENCHMARK_MODE = ENV['VALIDATE_BENCHMARK'] == 'true'
+
+return unless __FILE__ == $PROGRAM_NAME || VALIDATE_BENCHMARK_MODE
+
+require_relative 'benchmarks_helper'
+
+# This benchmark measures the performance of string storage-related APIs
+
+# Temporary hack to make CI happy. Our CI tries to compare benchmarks between master and branches, and if benchmarks
+# are not backwards-compatible between them (e.g. a new API was added...) it breaks. As a workaround, I'm disabling this
+# benchmark so we can merge it to master, and then I'll follow up with a micro-PR to re-enable it.
+TEMPORARY_DISABLE_BENCHMARK = true
+
+class ProfilerStringStorageIntern
+  def initialize
+    @recorder = Datadog::Profiling::StackRecorder.for_testing(heap_samples_enabled: true)
+  end
+
+  def run_benchmark
+    Benchmark.ips do |x|
+      benchmark_time = VALIDATE_BENCHMARK_MODE ? { time: 0.01, warmup: 0 } : { time: 10, warmup: 2 }
+      x.config(
+        **benchmark_time,
+      )
+      x.report('intern_all 1000 repeated strings') do
+        Datadog::Profiling::StackRecorder::Testing._native_benchmark_intern(@recorder, "hello, world!", 1000, true) unless TEMPORARY_DISABLE_BENCHMARK
+      end
+
+      x.save! "#{File.basename(__FILE__)}-1-results.json" unless VALIDATE_BENCHMARK_MODE
+      x.compare!
+    end
+
+    Benchmark.ips do |x|
+      benchmark_time = VALIDATE_BENCHMARK_MODE ? { time: 0.01, warmup: 0 } : { time: 10, warmup: 2 }
+      x.config(
+        **benchmark_time,
+      )
+
+      x.report('intern mixed existing and new') do
+        recorder = Datadog::Profiling::StackRecorder.for_testing(heap_samples_enabled: true)
+
+        strings_to_intern = 100_000
+        existing_strings = (strings_to_intern * 0.9).to_i
+        new_strings = strings_to_intern - existing_strings
+
+        new_strings.times do |i|
+          Datadog::Profiling::StackRecorder::Testing._native_benchmark_intern(recorder, ("%010d" % i), 1, false) unless TEMPORARY_DISABLE_BENCHMARK
+        end
+
+        existing_strings.times do |i|
+          Datadog::Profiling::StackRecorder::Testing._native_benchmark_intern(recorder, "hello, world!", 1, false) unless TEMPORARY_DISABLE_BENCHMARK
+        end
+      end
+
+      x.save! "#{File.basename(__FILE__)}-2-results.json" unless VALIDATE_BENCHMARK_MODE
+      x.compare!
+    end
+  end
+end
+
+puts "Current pid is #{Process.pid}"
+
+ProfilerStringStorageIntern.new.instance_exec do
+  run_benchmark
+end

benchmarks/run_all.sh

@@ -18,6 +18,7 @@ for file in \
   `dirname "$0"`/profiling_sample_loop_v2.rb \
   `dirname "$0"`/profiling_sample_serialize.rb \
   `dirname "$0"`/profiling_sample_gvl.rb \
+  `dirname "$0"`/profiling_string_storage_intern.rb \
   `dirname "$0"`/tracing_trace.rb;
 do
   bundle exec ruby "$file"

ext/datadog_profiling_native_extension/collectors_thread_context.c

@@ -1023,7 +1023,7 @@ static per_thread_context *get_or_create_context_for(VALUE thread, thread_contex
   if (st_lookup(state->hash_map_per_thread_context, (st_data_t) thread, &value_context)) {
     thread_context = (per_thread_context*) value_context;
   } else {
-    thread_context = ruby_xcalloc(1, sizeof(per_thread_context));
+    thread_context = calloc(1, sizeof(per_thread_context)); // See "note on calloc vs ruby_xcalloc use" in heap_recorder.c
     initialize_context(thread, thread_context, state);
     st_insert(state->hash_map_per_thread_context, (st_data_t) thread, (st_data_t) thread_context);
   }
@@ -1122,7 +1122,7 @@ static void initialize_context(VALUE thread, per_thread_context *thread_context,
 
 static void free_context(per_thread_context* thread_context) {
   sampling_buffer_free(thread_context->sampling_buffer);
-  ruby_xfree(thread_context);
+  free(thread_context); // See "note on calloc vs ruby_xcalloc use" in heap_recorder.c
 }
 
 static VALUE _native_inspect(DDTRACE_UNUSED VALUE _self, VALUE collector_instance) {