[PROF-8648] Add labels to heap samples #3263
Conversation
b3f2aba to
39dc8cf
Compare
| inline static ddog_CharSlice char_slice_from_c_string(char *string) { | ||
| return (ddog_CharSlice) { | ||
| .ptr = string, | ||
| .len = strlen(string), | ||
| }; | ||
| } | ||
|
|
There was a problem hiding this comment.
Minor: I've been somewhat hesitating about adding this specific helper, because it "hides" the strlen; there's a few places in the codebase that could've used it and actually explicitly do the construction to expose that detail.
But... maybe it's premature optimization? Any thoughts?
(I guess a third solution would be to name this something like char_slice_from_char_plus_strlen or something like that, although this may just me striking back again with long function names haha)
| @@ -184,10 +191,20 @@ static int st_object_records_iterate(st_data_t key, st_data_t value, st_data_t e | |||
| object_record *record = (object_record*) value; | |||
| internal_iteration_data *iteration_data = (internal_iteration_data*) extra; | |||
|
|
|||
| // FIXME: Uncomment the below block | |||
| /* | |||
| if (!rb_gc_is_ptr_to_obj(obj)) { | |||
There was a problem hiding this comment.
Yeah... this one... Would be nice to have it indeed. To be honest, I don't think we've tried asking for it to be exposed yet, so maybe we could get it for some future Ruby version? 🤔
| object_metadata object_metadata_init(ddog_CharSlice *alloc_class, size_t alloc_generation) { | ||
| object_metadata metadata; | ||
| metadata.alloc_class = ruby_strdup(alloc_class->ptr); |
There was a problem hiding this comment.
Minor: Hmm... is it me or are we rountripping a ddog_CharSlice back to a char *, so we can later turn it back to a ddog_CharSlice? Maybe worth introducing a char_slice_dup helper or something like that?
| ddog_CharSlice *alloc_class; | ||
| size_t alloc_gen; |
There was a problem hiding this comment.
Minor: I think this could be object_metadata as well?
|
Replaced with more production-ready versions starting from #3281 |
What does this PR do?
Builds on top of #3261 by tagging heap samples with
alloc classandgc gen agelabels that describe, respectively, what class a sample corresponds to and how many GCs has this sample survived for. The latter is especially useful to track memory leaks since anything that has a very recent age is susceptible to be cleared in a near-future GC whereas real leaks will tend to have very big ages.Motivation:
Add more context to heap profiling data.
Additional Notes:
How to test the change?
For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance.Unsure? Have a question? Request a review!