Skip to content

Commit 7d0acaa

Browse files
committed
Add design note explaining why heap_records is useful
1 parent c29be53 commit 7d0acaa

File tree

1 file changed

+7
-2
lines changed

1 file changed

+7
-2
lines changed

ext/datadog_profiling_native_extension/heap_recorder.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@ typedef struct {
4242
} heap_frame;
4343

4444
// A compact representation of a stacktrace for a heap allocation.
45-
// Used to dedup
46-
// heap allocation stacktraces across multiple objects sharing the same allocation location.
45+
// Used to dedup heap allocation stacktraces across multiple objects sharing the same allocation location.
4746
typedef struct {
4847
// How many objects are currently tracked in object_records recorder for this heap record.
4948
uint32_t num_tracked_objects;
@@ -83,6 +82,11 @@ struct heap_recorder {
8382
// NOTE: This table is currently only protected by the GVL since we never interact with it
8483
// outside the GVL.
8584
// NOTE: This table has ownership of its heap_records.
85+
//
86+
// This is a cpu/memory trade-off: Maintaining the "heap_records" map means we spend extra CPU when sampling as we need
87+
// to do de-duplication, but we reduce the memory footprint of the heap profiler.
88+
// In the future, it may be worth revisiting if we can move this inside libdatadog: if libdatadog was able to track
89+
// entire stacks for us, then we wouldn't need to do it on the Ruby side.
8690
st_table *heap_records;
8791

8892
// Map[obj_id: long, record: object_record*]
@@ -718,6 +722,7 @@ static int update_heap_record_entry_with_new_allocation(st_data_t *key, st_data_
718722
}
719723

720724
static heap_record* get_or_create_heap_record(heap_recorder *heap_recorder, ddog_prof_Slice_Location locations) {
725+
// See note on "heap_records" definition for why we keep this map.
721726
heap_record *stack = heap_record_new(heap_recorder, locations);
722727

723728
heap_record *new_or_existing_record = NULL; // Will be set inside update_heap_record_entry_with_new_allocation

0 commit comments

Comments
 (0)