Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use postponed jobs if YJIT is enabled. #180

Merged
merged 1 commit into from Jul 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 27 additions & 19 deletions ext/stackprof/stackprof.c
Expand Up @@ -25,20 +25,14 @@
#define FAKE_FRAME_MARK INT2FIX(1)
#define FAKE_FRAME_SWEEP INT2FIX(2)

/*
* As of Ruby 3.0, it should be safe to read stack frames at any time
* See https://github.com/ruby/ruby/commit/0e276dc458f94d9d79a0f7c7669bde84abe80f21
*/
#if RUBY_API_VERSION_MAJOR < 3
#define USE_POSTPONED_JOB
#endif

static const char *fake_frame_cstrs[] = {
"(garbage collection)",
"(marking)",
"(sweeping)",
};

static int stackprof_use_postponed_job = 1;

#define TOTAL_FAKE_FRAMES (sizeof(fake_frame_cstrs) / sizeof(char *))

#ifdef _POSIX_MONOTONIC_CLOCK
Expand Down Expand Up @@ -701,15 +695,13 @@ stackprof_job_record_gc(void *data)
stackprof_record_gc_samples();
}

#ifdef USE_POSTPONED_JOB
static void
stackprof_job_sample_and_record(void *data)
{
if (!_stackprof.running) return;

stackprof_sample_and_record();
}
#endif

static void
stackprof_job_record_buffer(void *data)
Expand Down Expand Up @@ -740,15 +732,15 @@ stackprof_signal_handler(int sig, siginfo_t *sinfo, void *ucontext)
_stackprof.unrecorded_gc_samples++;
rb_postponed_job_register_one(0, stackprof_job_record_gc, (void*)0);
} else {
#ifdef USE_POSTPONED_JOB
rb_postponed_job_register_one(0, stackprof_job_sample_and_record, (void*)0);
#else
// Buffer a sample immediately, if an existing sample exists this will
// return immediately
stackprof_buffer_sample();
// Enqueue a job to record the sample
rb_postponed_job_register_one(0, stackprof_job_record_buffer, (void*)0);
#endif
if (stackprof_use_postponed_job) {
rb_postponed_job_register_one(0, stackprof_job_sample_and_record, (void*)0);
} else {
// Buffer a sample immediately, if an existing sample exists this will
// return immediately
stackprof_buffer_sample();
// Enqueue a job to record the sample
rb_postponed_job_register_one(0, stackprof_job_record_buffer, (void*)0);
}
}
pthread_mutex_unlock(&lock);
}
Expand Down Expand Up @@ -826,9 +818,24 @@ stackprof_atfork_child(void)
stackprof_stop(rb_mStackProf);
}

static VALUE
stackprof_use_postponed_job_l(VALUE self)
{
stackprof_use_postponed_job = 1;
return Qnil;
}

void
Init_stackprof(void)
{
/*
* As of Ruby 3.0, it should be safe to read stack frames at any time, unless YJIT is enabled
* See https://github.com/ruby/ruby/commit/0e276dc458f94d9d79a0f7c7669bde84abe80f21
*/
#if RUBY_API_VERSION_MAJOR < 3
stackprof_use_postponed_job = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bug.
If Ruby < 3.0 (or YJIT), we want to use postponed job.
In other words rb_profile_frames() can only be used anywhere for Ruby 3.0+ no YJIT.
This is most likely the cause of https://bugs.ruby-lang.org/issues/18967 and recent CI failures #182 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #186

#endif

size_t i;
#define S(name) sym_##name = ID2SYM(rb_intern(#name));
S(object);
Expand Down Expand Up @@ -890,6 +897,7 @@ Init_stackprof(void)
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, "sample", stackprof_sample, 0);
rb_define_singleton_method(rb_mStackProf, "use_postponed_job!", stackprof_use_postponed_job_l, 0);

pthread_atfork(stackprof_atfork_prepare, stackprof_atfork_parent, stackprof_atfork_child);
}
4 changes: 4 additions & 0 deletions lib/stackprof.rb
@@ -1,5 +1,9 @@
require "stackprof/stackprof"

if defined?(RubyVM::YJIT) && RubyVM::YJIT.enabled?
StackProf.use_postponed_job!
end

module StackProf
VERSION = '0.2.19'
end
Expand Down
4 changes: 4 additions & 0 deletions test/test_stackprof.rb
Expand Up @@ -5,6 +5,10 @@
require 'pathname'

class StackProfTest < MiniTest::Test
def setup
Object.new # warm some caches to avoid flakiness
end

def test_info
profile = StackProf.run{}
assert_equal 1.2, profile[:version]
Expand Down