Skip to content

Commit

Permalink
[PROF-8667] Test to verify matching hash implementations in heap reco…
Browse files Browse the repository at this point in the history
…rder
  • Loading branch information
AlexJF committed Dec 14, 2023
1 parent 774c96a commit acab787
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 0 deletions.
21 changes: 21 additions & 0 deletions ext/ddtrace_profiling_native_extension/heap_recorder.c
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,27 @@ void heap_recorder_for_each_live_object(
ENFORCE_SUCCESS_HELPER(pthread_mutex_unlock(&heap_recorder->records_mutex), with_gvl);
}

void heap_recorder_testonly_assert_hash_matches(ddog_prof_Slice_Location locations) {
heap_stack *stack = heap_stack_new(locations);
heap_record_key stack_based_key = (heap_record_key) {
.type = HEAP_STACK,
.heap_stack = stack,
};
heap_record_key location_based_key = (heap_record_key) {
.type = LOCATION_SLICE,
.location_slice = &locations,
};

st_index_t stack_hash = heap_record_key_hash_st((st_data_t) &stack_based_key);
st_index_t location_hash = heap_record_key_hash_st((st_data_t) &location_based_key);

heap_stack_free(stack);

if (stack_hash != location_hash) {
rb_raise(rb_eRuntimeError, "Heap record key hashes built from the same locations differ. stack_based_hash=%"PRI_VALUE_PREFIX"u location_based_hash=%"PRI_VALUE_PREFIX"u", stack_hash, location_hash);
}
}

// ==========================
// Heap Recorder Internal API
// ==========================
Expand Down
4 changes: 4 additions & 0 deletions ext/ddtrace_profiling_native_extension/heap_recorder.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,7 @@ void heap_recorder_for_each_live_object(
bool (*for_each_callback)(heap_recorder_iteration_data data, void* extra_arg),
void *for_each_callback_extra_arg,
bool with_gvl);

// v--- TEST-ONLY APIs ---v

void heap_recorder_testonly_assert_hash_matches(ddog_prof_Slice_Location locations);
32 changes: 32 additions & 0 deletions ext/ddtrace_profiling_native_extension/stack_recorder.c
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ static void serializer_set_start_timestamp_for_next_profile(struct stack_recorde
static VALUE _native_record_endpoint(DDTRACE_UNUSED VALUE _self, VALUE recorder_instance, VALUE local_root_span_id, VALUE endpoint);
static void reset_profile(ddog_prof_Profile *profile, ddog_Timespec *start_time /* Can be null */);
static VALUE _native_track_object(DDTRACE_UNUSED VALUE _self, VALUE recorder_instance, VALUE new_obj, VALUE weight);
static VALUE _native_check_heap_hashes(DDTRACE_UNUSED VALUE _self, VALUE locations);


void stack_recorder_init(VALUE profiling_module) {
Expand All @@ -245,6 +246,7 @@ void stack_recorder_init(VALUE profiling_module) {
rb_define_singleton_method(testing_module, "_native_slot_two_mutex_locked?", _native_is_slot_two_mutex_locked, 1);
rb_define_singleton_method(testing_module, "_native_record_endpoint", _native_record_endpoint, 3);
rb_define_singleton_method(testing_module, "_native_track_object", _native_track_object, 3);
rb_define_singleton_method(testing_module, "_native_check_heap_hashes", _native_check_heap_hashes, 1);

ok_symbol = ID2SYM(rb_intern_const("ok"));
error_symbol = ID2SYM(rb_intern_const("error"));
Expand Down Expand Up @@ -739,6 +741,36 @@ static VALUE _native_track_object(DDTRACE_UNUSED VALUE _self, VALUE recorder_ins
return Qtrue;
}

static VALUE _native_check_heap_hashes(DDTRACE_UNUSED VALUE _self, VALUE locations) {
ENFORCE_TYPE(locations, T_ARRAY);
size_t locations_len = rb_array_len(locations);
ddog_prof_Location *locations_arr = ruby_xcalloc(locations_len, sizeof(ddog_prof_Location));
for (size_t i = 0; i < locations_len; i++) {
VALUE location = rb_ary_entry(locations, i);
ENFORCE_TYPE(location, T_ARRAY);
VALUE name = rb_ary_entry(location, 0);
VALUE filename = rb_ary_entry(location, 1);
VALUE line = rb_ary_entry(location, 2);
ENFORCE_TYPE(name, T_STRING);
ENFORCE_TYPE(filename, T_STRING);
ENFORCE_TYPE(line, T_FIXNUM);
locations_arr[i] = (ddog_prof_Location) {
.line = line,
.function = (ddog_prof_Function) {
.name = char_slice_from_ruby_string(name),
.filename = char_slice_from_ruby_string(filename),
}
};
}
ddog_prof_Slice_Location ddog_locations = {
.len = locations_len,
.ptr = locations_arr,
};
heap_recorder_testonly_assert_hash_matches(ddog_locations);

return Qnil;
}

static void reset_profile(ddog_prof_Profile *profile, ddog_Timespec *start_time /* Can be null */) {
ddog_prof_Profile_Result reset_result = ddog_prof_Profile_reset(profile, start_time);
if (reset_result.tag == DDOG_PROF_PROFILE_RESULT_ERR) {
Expand Down
65 changes: 65 additions & 0 deletions spec/datadog/profiling/stack_recorder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -551,4 +551,69 @@ def sample_types_from(decoded_profile)
expect(stack_recorder.serialize.first).to be >= now
end
end

describe '#heap_recorder' do
context 'stack/location hashing' do
it 'works for empty stacks' do
described_class::Testing._native_check_heap_hashes([])
end

it 'works for single-frame stacks' do
described_class::Testing._native_check_heap_hashes(
[
['a name', 'a filename', 123]
]
)
end

it 'works for multi-frame stacks' do
described_class::Testing._native_check_heap_hashes(
[
['a name', 'a filename', 123],
['another name', 'anoter filename', 456],
]
)
end

it 'works with empty names' do
described_class::Testing._native_check_heap_hashes(
[
['', 'a filename', 123],
]
)
end

it 'works with empty filenames' do
described_class::Testing._native_check_heap_hashes(
[
['a name', '', 123],
]
)
end

it 'works with zero lines' do
described_class::Testing._native_check_heap_hashes(
[
['a name', 'a filename', 0]
]
)
end

it 'works with negative lines' do
described_class::Testing._native_check_heap_hashes(
[
['a name', 'a filename', -123]
]
)
end

it 'works with biiiiiiig lines' do
described_class::Testing._native_check_heap_hashes(
[
['a name', 'a filename', 4_000_000]
]
)
end
end
end
end

0 comments on commit acab787

Please sign in to comment.