Merge branch 'main' into categorize_as_gc

.github/workflows/gem_push.yml

@@ -0,0 +1,30 @@
+name: Push gem to RubyGems.org
+
+on:
+  workflow_dispatch:
+
+jobs:
+  push:
+    runs-on: ubuntu-latest
+
+    environment: 
+      name: rubygems.org
+      url: https://rubygems.org/gems/vernier
+
+    permissions:
+      contents: write
+      id-token: write
+
+    steps:
+      # Set up
+      - uses: actions/checkout@v4
+        with:
+          submodules: true
+      - name: Set up Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          bundler-cache: true
+          ruby-version: ruby
+
+      # Release
+      - uses: rubygems/release-gem@v1

.github/workflows/sentry.yml

@@ -0,0 +1,43 @@
+name: Sentry integration
+
+on:
+  push:
+    branches:
+      - main
+
+  pull_request:
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    name: Sentry integration test
+    steps:
+    - name: Checkout sentry
+      uses: actions/checkout@v4
+      with:
+        repository: getsentry/sentry-ruby
+        path: sentry-ruby
+    - name: Checkout vernier
+      uses: actions/checkout@v4
+      with:
+        path: vernier
+    - name: Set up Ruby
+      uses: ruby/setup-ruby@v1
+      with:
+        ruby-version: ruby
+    - name: Build vernier
+      run: |
+        cd vernier
+        bundle
+        bundle exec rake compile
+    - name: Setup sentry-ruby
+      run: |
+        VERNIER_PATH="$(pwd)/vernier"
+        cd sentry-ruby/sentry-ruby
+        sed -i "/gem *['\"]vernier/d" Gemfile
+        echo "gem \"vernier\", path: \"$VERNIER_PATH\"" >> Gemfile
+        bundle install
+    - name: Run tests
+      run: |
+        cd sentry-ruby/sentry-ruby
+        bundle exec rspec spec/sentry/vernier/profiler_spec.rb

.github/workflows/version.yml

@@ -0,0 +1,41 @@
+
+name: Bump version
+
+on:
+  workflow_dispatch:
+    inputs:
+      version:
+        description: 'What version level of bump?'
+        required: true
+        default: 'patch'
+        type: choice
+        options:
+        - major
+        - minor
+        - patch
+
+
+permissions:
+  contents: write
+
+jobs:
+  bump:
+    runs-on: ubuntu-latest
+    env:
+      GH_TOKEN: ${{ github.token }}
+    steps:
+    - uses: actions/checkout@v4
+    - name: Set up Ruby
+      uses: ruby/setup-ruby@v1
+      with:
+        ruby-version: '3.3'
+    - name: Set git config
+      run: |
+        git config --global user.email "${GITHUB_ACTOR_ID}+${GITHUB_ACTOR}@users.noreply.github.com"
+        git config --global user.name "$(gh api /users/${GITHUB_ACTOR} | jq .name -r)"
+        git remote set-url origin "https://x-access-token:${{ github.token }}@github.com/$GITHUB_REPOSITORY"
+      shell: bash
+    - name: Bump version
+      run: gem exec bump ${{ github.event.inputs.version }}
+    - name: push
+      run: git push

.gitignore

@@ -17,3 +17,4 @@ Gemfile.lock
 /examples/my_sleep.c
 /examples/my_sleep.dylib
 .DS_Store
+.gdbinit

exe/vernier

@@ -3,45 +3,134 @@
 require "optparse"
 require "vernier/version"
 
-banner = <<-END
+module Vernier
+  module CLI
+    def self.run(options)
+      banner = <<-END
 Usage: vernier run [FLAGS] -- COMMAND
 
 FLAGS:
-END
+      END
 
-options = {}
-parser = OptionParser.new(banner) do |o|
-  o.version = Vernier::VERSION
+      OptionParser.new(banner) do |o|
+        o.version = Vernier::VERSION
 
-  o.on('--output [FILENAME]', String, "output filename") do |s|
-    options[:output] = s
-  end
-  o.on('--interval [MICROSECONDS]', Integer, "sampling interval (default 500)") do |i|
-    options[:interval] = i
-  end
-  o.on('--allocation_sample_rate [ALLOCATIONS]', Integer, "allocation sampling interval (default 0 disabled)") do |i|
-    options[:allocation_sample_rate] = i
-  end
-  o.on('--signal [NAME]', String, "specify a signal to start and stop the profiler") do |s|
-    options[:signal] = s
-  end
-  o.on('--start-paused', "don't automatically start the profiler") do
-    options[:start_paused] = true
-  end
-  o.on('--hooks [HOOKS]', String, "Enable instrumentation hooks. Currently supported: rails") do |s|
-    options[:hooks] = s
+        o.on('--output [FILENAME]', String, "output filename") do |s|
+          options[:output] = s
+        end
+        o.on('--interval [MICROSECONDS]', Integer, "sampling interval (default 500)") do |i|
+          options[:interval] = i
+        end
+        o.on('--allocation_sample_rate [ALLOCATIONS]', Integer, "allocation sampling interval (default 0 disabled)") do |i|
+          options[:allocation_sample_rate] = i
+        end
+        o.on('--signal [NAME]', String, "specify a signal to start and stop the profiler") do |s|
+          options[:signal] = s
+        end
+        o.on('--start-paused', "don't automatically start the profiler") do
+          options[:start_paused] = true
+        end
+        o.on('--hooks [HOOKS]', String, "Enable instrumentation hooks. Currently supported: rails") do |s|
+          options[:hooks] = s
+        end
+      end
+    end
+
+    def self.view(options)
+      banner = <<-END
+Usage: vernier view [FLAGS] FILENAME
+
+FLAGS:
+      END
+
+      OptionParser.new(banner) do |o|
+        o.on('--top [COUNT]', Integer, "number of frames to show (default 20)") do |i|
+          options[:top] = i
+        end
+      end
+    end
+
+    def self.inverted_tree(top, file)
+      # Print the inverted tree from a Vernier profile
+      require "json"
+
+      is_gzip = File.binread(file, 2) == "\x1F\x8B".b # check for gzip header
+
+      json = if is_gzip
+        require "zlib"
+        Zlib::GzipReader.open(file) { |gz| gz.read }
+      else
+        File.read file
+      end
+
+      info = JSON.load json
+
+      main = info["threads"].find { |thread| thread["isMainThread"] }
+
+      weight_by_frame = Hash.new(0)
+
+      stack_frames = main["stackTable"]["frame"]
+      frame_table = main["frameTable"]["func"]
+      func_table = main["funcTable"]["name"]
+      string_array = main["stringArray"]
+
+      main["samples"]["stack"].zip(main["samples"]["weight"]).each do |stack, weight|
+        top_frame_index = stack_frames[stack]
+        func_index = frame_table[top_frame_index]
+        string_index = func_table[func_index]
+        str = string_array[string_index]
+        weight_by_frame[str] += weight
+      end
+
+      total = weight_by_frame.values.inject :+
+
+      header = ["Samples", "%", ""]
+      widths = header.map(&:bytesize)
+
+      columns = weight_by_frame.sort_by { |k,v| v }.reverse.first(top).map { |k,v|
+        entry = [v.to_s, ((v / total.to_f) * 100).round(1).to_s, k]
+        entry.each_with_index { |str, i| widths[i] = str.bytesize if widths[i] < str.bytesize }
+        entry
+      }
+
+      print_separator widths
+      print_row header, widths
+      print_separator widths
+      columns.each { print_row(_1, widths) }
+      print_separator widths
+    end
+
+    def self.print_row(list, widths)
+      puts("|" + list.map.with_index { |str, i| " " + str.ljust(widths[i] + 1) }.join("|") + "|")
+    end
+
+    def self.print_separator(widths)
+      puts("+" + widths.map { |i| "-" * (i + 2) }.join("+") + "+")
+    end
   end
 end
 
-parser.parse!
-parser.abort(parser.help) if ARGV.shift != "run"
-parser.abort(parser.help) if ARGV.empty?
+options = {}
+run = Vernier::CLI.run(options)
+view = Vernier::CLI.view(options)
 
-env = {}
-options.each do |k, v|
-  env["VERNIER_#{k.to_s.upcase}"] = v.to_s
-end
-vernier_path = File.expand_path('../lib', __dir__)
-env['RUBYOPT'] = "-I #{vernier_path} -r vernier/autorun #{ENV['RUBYOPT']}"
+case ARGV.shift
+when "run"
+  run.parse!
+  run.abort(run.help) if ARGV.empty?
 
-Kernel.exec(env, *ARGV)
+  env = {}
+  options.each do |k, v|
+    env["VERNIER_#{k.to_s.upcase}"] = v.to_s
+  end
+  vernier_path = File.expand_path('../lib', __dir__)
+  env['RUBYOPT'] = "-I #{vernier_path} -r vernier/autorun #{ENV['RUBYOPT']}"
+
+  Kernel.exec(env, *ARGV)
+when "view"
+  view.parse!
+  view.abort(view.help) if ARGV.empty?
+  Vernier::CLI.inverted_tree(options[:top] || 20, ARGV.shift)
+else
+  run.abort(run.help + "\n" + view.help)
+end

ext/vernier/vernier.cc

@@ -672,10 +672,6 @@ StackTable::stack_table_convert(VALUE self, VALUE original_tableval, VALUE origi
     StackTable *original_table = get_stack_table(original_tableval);
     int original_idx = NUM2INT(original_idxval);
 
-    if (original_idx == -1) {
-        return original_idxval;
-    }
-
     int original_size;
     {
         const std::lock_guard<std::mutex> lock(original_table->stack_mutex);
@@ -1028,13 +1024,15 @@ class SampleList {
         }
 
         void record_sample(int stack_index, TimeStamp time, Category category) {
-            if (
-                    !empty() &&
-                    stacks.back() == stack_index &&
-                    categories.back() == category)
-            {
-                // We don't compare timestamps for de-duplication
-                weights.back() += 1;
+          // FIXME: probably better to avoid generating -1 higher up.
+          // Currently this happens when we measure an empty stack. Ideally we would have a better representation
+          if (stack_index < 0)
+            return;
+
+          if (!empty() && stacks.back() == stack_index &&
+              categories.back() == category) {
+            // We don't compare timestamps for de-duplication
+            weights.back() += 1;
             } else {
                 stacks.push_back(stack_index);
                 timestamps.push_back(time);
@@ -1117,7 +1115,11 @@ class Thread {
             sample.sample();
 
             int stack_idx = translator.translate(frame_list, sample);
-            allocation_samples.record_sample(stack_idx, TimeStamp::Now(), 1);
+            if (stack_idx >= 0) {
+                allocation_samples.record_sample(stack_idx, TimeStamp::Now(), 1);
+            } else {
+                // TODO: should we log an empty frame?
+            }
         }
 
         void set_state(State new_state) {

lib/vernier/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Vernier
-  VERSION = "1.1.1"
+  VERSION = "1.2.1"
 end

test/integration_test.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class IntegrationTest < Minitest::Test
+  include FirefoxTestHelpers
+
+  LIB_DIR = File.expand_path("../lib", __dir__)
+
+  def test_allocations
+    result = run_vernier({allocation_sample_rate: 1}, "-e", "")
+    assert_valid_firefox_profile(result)
+  end
+
+  def run_vernier(options, *argv)
+    result_json = nil
+    Tempfile.open('vernier') do |tempfile|
+      options = {
+        quiet: true,
+        output: tempfile.path,
+      }.merge(options)
+      env = options.map do |k, v|
+        ["VERNIER_#{k.to_s.upcase}", v.to_s]
+      end.to_h
+      result = system(env, RbConfig.ruby, "-I", LIB_DIR, "-r", "vernier/autorun", *argv, exception: true)
+      tempfile.rewind
+      result_json = tempfile.read
+    end
+    result = JSON.parse(result_json)
+    result
+  end
+end