From 86622c83d8c51b856b2e9b3daa16f08eceb2d544 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 4 May 2021 12:47:51 -0700 Subject: [PATCH 1/6] Convert StackProf.run / StackProf.result to Ruby This commit is kind of a yak shave. I would like to add thread tracking to stackprof. I want to allow people to optionally dump the thread for which the stack was collected on by doing `StackProf.result(threads: true)` Updating the signature for `result` was kind of hard, so I am refactoring it to be in Ruby. This commit technically breaks the case of someone doing: ```ruby StackProf.start(out: "some_file.data") ... StackProf.result ``` But I think it's strange to ask the start function to dump the file rather than asking the result function. The result function actually writes to the file, so I think it makes more sense to pass the file (or file name) to the result function. --- ext/stackprof/stackprof.c | 37 ++++++++----------------------------- lib/stackprof.rb | 22 ++++++++++++++++++++++ test/test_stackprof.rb | 2 +- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/ext/stackprof/stackprof.c b/ext/stackprof/stackprof.c index 813ca76c..87cf6116 100644 --- a/ext/stackprof/stackprof.c +++ b/ext/stackprof/stackprof.c @@ -96,7 +96,6 @@ static struct { VALUE mode; VALUE interval; - VALUE out; VALUE metadata; int ignore_gc; @@ -131,7 +130,7 @@ static struct { static VALUE sym_object, sym_wall, sym_cpu, sym_custom, sym_name, sym_file, sym_line; static VALUE sym_samples, sym_total_samples, sym_missed_samples, sym_edges, sym_lines; -static VALUE sym_version, sym_mode, sym_interval, sym_raw, sym_metadata, sym_frames, sym_ignore_gc, sym_out; +static VALUE sym_version, sym_mode, sym_interval, sym_raw, sym_metadata, sym_frames, sym_ignore_gc; static VALUE sym_aggregate, sym_raw_sample_timestamps, sym_raw_timestamp_deltas, sym_state, sym_marking, sym_sweeping; static VALUE sym_gc_samples, objtracer; static VALUE gc_hook; @@ -145,7 +144,7 @@ stackprof_start(int argc, VALUE *argv, VALUE self) { struct sigaction sa; struct itimerval timer; - VALUE opts = Qnil, mode = Qnil, interval = Qnil, metadata = rb_hash_new(), out = Qfalse; + VALUE opts = Qnil, mode = Qnil, interval = Qnil, metadata = rb_hash_new(); int ignore_gc = 0; int raw = 0, aggregate = 1; VALUE metadata_val; @@ -158,7 +157,6 @@ stackprof_start(int argc, VALUE *argv, VALUE self) if (RTEST(opts)) { mode = rb_hash_aref(opts, sym_mode); interval = rb_hash_aref(opts, sym_interval); - out = rb_hash_aref(opts, sym_out); if (RTEST(rb_hash_aref(opts, sym_ignore_gc))) { ignore_gc = 1; } @@ -220,7 +218,6 @@ stackprof_start(int argc, VALUE *argv, VALUE self) _stackprof.interval = interval; _stackprof.ignore_gc = ignore_gc; _stackprof.metadata = metadata; - _stackprof.out = out; _stackprof.target_thread = pthread_self(); if (raw) { @@ -343,7 +340,7 @@ frame_i(st_data_t key, st_data_t val, st_data_t arg) } static VALUE -stackprof_results(int argc, VALUE *argv, VALUE self) +stackprof_results(VALUE self, VALUE io) { VALUE results, frames; @@ -409,35 +406,22 @@ stackprof_results(int argc, VALUE *argv, VALUE self) _stackprof.raw = 0; } - if (argc == 1) - _stackprof.out = argv[0]; - - if (RTEST(_stackprof.out)) { + if (RTEST(io)) { VALUE file; - if (rb_respond_to(_stackprof.out, rb_intern("to_io"))) { - file = rb_io_check_io(_stackprof.out); + if (rb_respond_to(io, rb_intern("to_io"))) { + file = rb_io_check_io(io); } else { - file = rb_file_open_str(_stackprof.out, "w"); + file = rb_file_open_str(io, "w"); } rb_marshal_dump(results, file); rb_io_flush(file); - _stackprof.out = Qnil; return file; } else { return results; } } -static VALUE -stackprof_run(int argc, VALUE *argv, VALUE self) -{ - rb_need_block(); - stackprof_start(argc, argv, self); - rb_ensure(rb_yield, Qundef, stackprof_stop, self); - return stackprof_results(0, 0, self); -} - static VALUE stackprof_running_p(VALUE self) { @@ -799,9 +783,6 @@ stackprof_gc_mark(void *data) if (RTEST(_stackprof.metadata)) rb_gc_mark(_stackprof.metadata); - if (RTEST(_stackprof.out)) - rb_gc_mark(_stackprof.out); - if (_stackprof.frames) st_foreach(_stackprof.frames, frame_mark_i, 0); } @@ -875,7 +856,6 @@ Init_stackprof(void) S(raw); S(raw_sample_timestamps); S(raw_timestamp_deltas); - S(out); S(metadata); S(ignore_gc); S(frames); @@ -910,10 +890,9 @@ Init_stackprof(void) rb_mStackProf = rb_define_module("StackProf"); rb_define_singleton_method(rb_mStackProf, "running?", stackprof_running_p, 0); - rb_define_singleton_method(rb_mStackProf, "run", stackprof_run, -1); rb_define_singleton_method(rb_mStackProf, "start", stackprof_start, -1); rb_define_singleton_method(rb_mStackProf, "stop", stackprof_stop, 0); - rb_define_singleton_method(rb_mStackProf, "results", stackprof_results, -1); + rb_define_singleton_method(rb_mStackProf, "_results", stackprof_results, 1); rb_define_singleton_method(rb_mStackProf, "sample", stackprof_sample, 0); rb_define_singleton_method(rb_mStackProf, "use_postponed_job!", stackprof_use_postponed_job_l, 0); diff --git a/lib/stackprof.rb b/lib/stackprof.rb index 4de14e22..4253e39f 100644 --- a/lib/stackprof.rb +++ b/lib/stackprof.rb @@ -15,6 +15,28 @@ module StackProf VERSION = '0.2.23' + + class << self + private :_results + + def run(mode: :wall, out: nil, interval: nil, raw: nil, metadata: nil, debug: nil, &block) + raise unless block_given? + + start(mode: mode, interval: interval, raw: raw, metadata: metadata, debug: nil) + + begin + yield + ensure + stop + end + + results out + end + + def results(io = nil) + _results io + end + end end StackProf.autoload :Report, "stackprof/report.rb" diff --git a/test/test_stackprof.rb b/test/test_stackprof.rb index 2aedc0bc..37b72e76 100644 --- a/test/test_stackprof.rb +++ b/test/test_stackprof.rb @@ -77,7 +77,7 @@ def test_object_allocation_interval end def test_cputime - profile = StackProf.run(mode: :cpu, interval: 500) do + profile = StackProf.run(mode: :cpu, interval: 1000) do math end From 9c3679128865976ec64357f7db81b2ac2fd5d2d9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 5 May 2021 12:32:59 -0700 Subject: [PATCH 2/6] run the build From bf4167efd5046a1f6ed384b42413c4eec42241be Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 5 May 2021 12:36:40 -0700 Subject: [PATCH 3/6] testing --- test/test_stackprof.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/test_stackprof.rb b/test/test_stackprof.rb index 37b72e76..e2539951 100644 --- a/test/test_stackprof.rb +++ b/test/test_stackprof.rb @@ -65,7 +65,12 @@ def test_object_allocation if RUBY_VERSION >= '3' assert_equal [4, 0], frame[:lines][profile_base_line] else - assert_equal [2, 0], frame[:lines][profile_base_line] + begin + assert_equal [2, 0], frame[:lines][profile_base_line] + rescue + p frame[:lines] + raise + end end end From 0b05968587fcb1cef53fcd617f624612a895f234 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 5 May 2021 12:39:29 -0700 Subject: [PATCH 4/6] testing --- test/test_stackprof.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_stackprof.rb b/test/test_stackprof.rb index e2539951..e9321e35 100644 --- a/test/test_stackprof.rb +++ b/test/test_stackprof.rb @@ -67,9 +67,9 @@ def test_object_allocation else begin assert_equal [2, 0], frame[:lines][profile_base_line] - rescue + rescue Exception => e p frame[:lines] - raise + raise e end end end From 8cd73fa6e9ed4641a20d9a9b86341068d12b807f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 5 May 2021 16:18:22 -0700 Subject: [PATCH 5/6] I don't want to support this many Ruby versions --- .github/workflows/ci.yml | 2 +- README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 983b2082..fcf12a14 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,7 +8,7 @@ jobs: strategy: fail-fast: false matrix: - ruby: [ ruby-head, '3.1', '3.0', '2.7', '2.6', '2.5', '2.4', '2.3', '2.2', truffleruby ] + ruby: [ ruby-head, '3.2', '3.1', '3.0', '2.7', '2.6', '2.5', '2.4', '2.3', '2.2', truffleruby ] steps: - name: Checkout uses: actions/checkout@v2 diff --git a/README.md b/README.md index 79e9bede..aa52bb3b 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Inspired heavily by [gperftools](https://code.google.com/p/gperftools/), and wri ## Requirements -* Ruby 2.2+ +* Ruby 2.7+ * Linux-based OS ## Getting Started From 70f3910fee4f3d3ab7695f850b4e503d70ab1bd1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 5 May 2021 16:21:03 -0700 Subject: [PATCH 6/6] testing --- test/test_stackprof.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/test_stackprof.rb b/test/test_stackprof.rb index e9321e35..1dffb06b 100644 --- a/test/test_stackprof.rb +++ b/test/test_stackprof.rb @@ -44,7 +44,12 @@ def test_object_allocation assert_equal :object, profile[:mode] assert_equal 1, profile[:interval] if RUBY_VERSION >= '3' - assert_equal 4, profile[:samples] + begin + assert_equal 4, profile[:samples] + rescue Exception => e + p profile + raise + end else assert_equal 2, profile[:samples] end