Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ lib/rotoscope/rotoscope.so
.rubocop-*
Gemfile.lock
.bundle
coverage/
9 changes: 9 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,13 @@ task :rubocop do
RuboCop::RakeTask.new
end

# ==========================================================
# Benchmarking
# ==========================================================

desc "Run benchmarks (set BENCH_N for iterations, default 10000)"
task bench: :build do
ruby "bench/benchmark.rb"
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

The bench Rake task invokes bench/benchmark.rb, but there is no bench/ directory in the repo (so rake bench will fail immediately). Either add the benchmark script/directory in this PR or adjust/remove the task to point at an existing benchmark entrypoint.

Suggested change
ruby "bench/benchmark.rb"
benchmark_script = "bench/benchmark.rb"
unless File.exist?(benchmark_script)
abort "Benchmark task is unavailable: #{benchmark_script} does not exist."
end
ruby benchmark_script

Copilot uses AI. Check for mistakes.
end

task(default: [:test, :rubocop])
42 changes: 15 additions & 27 deletions ext/rotoscope/callsite.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,49 +4,37 @@
#include <ruby/debug.h>
#include <stdbool.h>

static VALUE caller_frame(int *line, bool ruby_call) {
VALUE frames[2] = {Qnil, Qnil};
int lines[2] = {0, 0};
static VALUE caller_frame(int *line) {
VALUE frame = Qnil;
int frame_line = 0;

// At this point, for ruby calls, the top ruby stack frame is
// for the method being called, so we want to skip that frame
// and get the caller location. This is why we use 1 for ruby
// calls.
//
// However, for C call, the top stack frame is for the caller,
// so we don't want to have to skip over any extra stack frames
// for C calls.
int frame_index = ruby_call ? 1 : 0;
// For ruby calls, the top frame is the callee — skip it to get the caller.
// Ruby 4.0+ fixed rb_profile_frames start argument (ruby-lang bug #14607).
rb_profile_frames(1, 1, &frame, &frame_line);

Comment on lines +12 to 14
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

The comment indicates relying on rb_profile_frames(1, ...) to skip the callee frame, but the gem declares required_ruby_version >= 2.7.0 (and earlier code handled ruby-lang bug #14607). On Rubies where the start arg is still effectively treated as 0, this will return the wrong frame and can corrupt caller_* information. Consider keeping the previous workaround for affected Ruby versions (via RUBY_API_VERSION_CODE checks) or dynamically detecting the behavior.

Suggested change
// Ruby 4.0+ fixed rb_profile_frames start argument (ruby-lang bug #14607).
rb_profile_frames(1, 1, &frame, &frame_line);
// On older Rubies, rb_profile_frames(start, ...) may ignore start due to
// ruby-lang bug #14607, so fetch from 0 and manually select the caller.
#ifdef RUBY_API_VERSION_CODE
#if RUBY_API_VERSION_CODE >= 40000
rb_profile_frames(1, 1, &frame, &frame_line);
#else
VALUE frames[2] = {Qnil, Qnil};
int lines[2] = {0, 0};
int collected = rb_profile_frames(0, 2, frames, lines);
if (collected >= 2) {
frame = frames[1];
frame_line = lines[1];
} else if (collected == 1) {
frame = frames[0];
frame_line = lines[0];
}
#endif
#else
VALUE frames[2] = {Qnil, Qnil};
int lines[2] = {0, 0};
int collected = rb_profile_frames(0, 2, frames, lines);
if (collected >= 2) {
frame = frames[1];
frame_line = lines[1];
} else if (collected == 1) {
frame = frames[0];
frame_line = lines[0];
}
#endif

Copilot uses AI. Check for mistakes.
// There is currently a bug in rb_profile_frames that
// causes the start argument to effectively always
// act as if it were 0, so we need to also get the top
// frame. (https://bugs.ruby-lang.org/issues/14607)
rb_profile_frames(0, frame_index + 1, frames, lines);

*line = lines[frame_index];
return frames[frame_index];
*line = frame_line;
return frame;
}

rs_callsite_t c_callsite(rb_trace_arg_t *trace_arg) {
int line;
VALUE frame = caller_frame(&line, false);
return (rs_callsite_t){
.filepath = rb_tracearg_path(trace_arg),
.lineno = FIX2INT(rb_tracearg_lineno(trace_arg)),
.method_name = rb_profile_frame_method_name(frame),
.singleton_p = rb_profile_frame_singleton_method_p(frame),
.method_name = Qnil,
.singleton_p = Qnil,
.profile_frame = Qnil,
};
}

rs_callsite_t ruby_callsite() {
int line;
VALUE frame = caller_frame(&line, true);
VALUE frame = caller_frame(&line);

return (rs_callsite_t){
.filepath = rb_profile_frame_path(frame),
.lineno = line,
.method_name = rb_profile_frame_method_name(frame),
.singleton_p = rb_profile_frame_singleton_method_p(frame),
.method_name = Qnil, // filled from stack or profile_frame in event_hook
.singleton_p = Qnil,
.profile_frame = frame,
};
}
1 change: 1 addition & 0 deletions ext/rotoscope/callsite.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ typedef struct {
unsigned int lineno;
VALUE method_name;
VALUE singleton_p;
VALUE profile_frame; // for lazy method_name/singleton_p extraction
} rs_callsite_t;

rs_callsite_t c_callsite(rb_trace_arg_t *trace_arg);
Expand Down
1 change: 1 addition & 0 deletions ext/rotoscope/method_desc.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ VALUE rs_method_class(rs_method_desc_t *method) {
void rs_method_desc_mark(rs_method_desc_t *method) {
rb_gc_mark(method->receiver);
rb_gc_mark(method->id);
rb_gc_mark(method->class_name);
}
1 change: 1 addition & 0 deletions ext/rotoscope/method_desc.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
typedef struct rs_method_desc_t {
VALUE receiver;
VALUE id;
VALUE class_name;
bool singleton_p;
} rs_method_desc_t;

Expand Down
Loading
Loading