Skip to content

Commit 6300ad2

Browse files
committed
fix: avoid redundant param processing
1 parent 18c9c7c commit 6300ad2

File tree

2 files changed

+41
-31
lines changed

2 files changed

+41
-31
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Read the code of the native tracer implementation line by line and improve performance while preserving correctness

gems/codetracer-ruby-recorder/ext/native_tracer/src/lib.rs

Lines changed: 40 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -556,13 +556,13 @@ unsafe fn record_variables(recorder: &mut Recorder, binding: VALUE) -> Vec<FullV
556556
result
557557
}
558558

559-
unsafe fn record_parameters(
559+
560+
unsafe fn collect_parameter_values(
560561
recorder: &mut Recorder,
561562
binding: VALUE,
562563
defined_class: VALUE,
563564
mid: ID,
564-
register: bool,
565-
) -> Vec<FullValueRecord> {
565+
) -> Vec<(String, ValueRecord)> {
566566
let method_sym = rb_id2sym(mid);
567567
if rb_method_boundp(defined_class, mid, 0) == 0 {
568568
return Vec::new();
@@ -590,29 +590,38 @@ unsafe fn record_parameters(
590590
if name_c.is_null() {
591591
continue;
592592
}
593-
let name = CStr::from_ptr(name_c).to_str().unwrap_or("");
593+
let name = CStr::from_ptr(name_c).to_str().unwrap_or("").to_string();
594594
let value = rb_funcall(binding, recorder.local_get_id, 1, name_sym);
595595
let val_rec = to_value(recorder, value, 10, recorder.to_s_id);
596-
if register {
597-
recorder
598-
.tracer
599-
.register_variable_with_full_value(name, val_rec.clone());
600-
let var_id = recorder.tracer.ensure_variable_id(name);
601-
result.push(FullValueRecord {
602-
variable_id: var_id,
603-
value: val_rec,
604-
});
605-
}
596+
result.push((name, val_rec));
606597
}
607598
result
608599
}
609600

610-
unsafe fn record_event(tracer: &mut Tracer, path: &str, line: i64, content: &str) {
601+
unsafe fn register_parameter_values(
602+
recorder: &mut Recorder,
603+
params: Vec<(String, ValueRecord)>,
604+
) -> Vec<FullValueRecord> {
605+
let mut result = Vec::with_capacity(params.len());
606+
for (name, val_rec) in params {
607+
recorder
608+
.tracer
609+
.register_variable_with_full_value(&name, val_rec.clone());
610+
let var_id = recorder.tracer.ensure_variable_id(&name);
611+
result.push(FullValueRecord {
612+
variable_id: var_id,
613+
value: val_rec,
614+
});
615+
}
616+
result
617+
}
618+
619+
unsafe fn record_event(tracer: &mut Tracer, path: &str, line: i64, content: String) {
611620
tracer.register_step(Path::new(path), Line(line));
612621
tracer.events.push(TraceLowLevelEvent::Event(RecordEvent {
613622
kind: EventLogKind::Write,
614623
metadata: String::new(),
615-
content: content.to_string(),
624+
content,
616625
}));
617626
}
618627

@@ -666,16 +675,16 @@ unsafe extern "C" fn record_event_api(
666675
content: VALUE,
667676
) -> VALUE {
668677
let recorder = &mut *get_recorder(self_val);
669-
let path_str = if NIL_P(path) {
670-
"".to_string()
678+
let path_slice = if NIL_P(path) {
679+
""
671680
} else {
672681
let ptr = RSTRING_PTR(path);
673682
let len = RSTRING_LEN(path) as usize;
674-
String::from_utf8_lossy(std::slice::from_raw_parts(ptr as *const u8, len)).to_string()
683+
std::str::from_utf8(std::slice::from_raw_parts(ptr as *const u8, len)).unwrap_or("")
675684
};
676685
let line_num = rb_num2long(line) as i64;
677686
let content_str = value_to_string(content, recorder.to_s_id).unwrap_or_default();
678-
record_event(&mut recorder.tracer, &path_str, line_num, &content_str);
687+
record_event(&mut recorder.tracer, path_slice, line_num, content_str);
679688
Qnil.into()
680689
}
681690

@@ -729,10 +738,13 @@ unsafe extern "C" fn event_hook_raw(data: VALUE, arg: *mut rb_trace_arg_t) {
729738
let mid_sym = rb_tracearg_callee_id(arg);
730739
let mid = rb_sym2id(mid_sym);
731740
let defined_class = rb_funcall(self_val, recorder.class_id, 0);
732-
if !NIL_P(binding) {
733-
// ensure parameter types are registered before self
734-
let _ = record_parameters(recorder, binding, defined_class, mid, false);
735-
}
741+
742+
let param_vals = if NIL_P(binding) {
743+
Vec::new()
744+
} else {
745+
collect_parameter_values(recorder, binding, defined_class, mid)
746+
};
747+
736748
let class_name =
737749
cstr_to_string(rb_obj_classname(self_val)).unwrap_or_else(|| "Object".to_string());
738750
let text = value_to_string_safe(self_val, recorder.to_s_id).unwrap_or_default();
@@ -745,13 +757,10 @@ unsafe extern "C" fn event_hook_raw(data: VALUE, arg: *mut rb_trace_arg_t) {
745757
.tracer
746758
.register_variable_with_full_value("self", self_rec.clone());
747759

748-
let mut args = if NIL_P(binding) {
749-
Vec::new()
750-
} else {
751-
record_parameters(recorder, binding, defined_class, mid, true)
752-
};
753-
754-
args.insert(0, recorder.tracer.arg("self", self_rec));
760+
let mut args = vec![recorder.tracer.arg("self", self_rec)];
761+
if !param_vals.is_empty() {
762+
args.extend(register_parameter_values(recorder, param_vals));
763+
}
755764
recorder.tracer.register_step(Path::new(&path), Line(line));
756765
let name_c = rb_id2name(mid);
757766
let mut name = if !name_c.is_null() {

0 commit comments

Comments
 (0)