diff --git a/src/gc-heap-snapshot.cpp b/src/gc-heap-snapshot.cpp index d6f76add293d1..979090191b945 100644 --- a/src/gc-heap-snapshot.cpp +++ b/src/gc-heap-snapshot.cpp @@ -7,12 +7,16 @@ #include #include +#include #include #include #include +#include using std::vector; using std::string; +using std::ostringstream; +using std::pair; using std::unordered_map; using std::unordered_set; @@ -62,7 +66,7 @@ struct Edge { const int k_node_number_of_fields = 7; struct Node { - size_t type; // index into snapshot->node_types + size_t type; // index into snapshot->node_types string name; size_t id; // This should be a globally-unique counter, but we use the memory address size_t self_size; @@ -217,8 +221,9 @@ size_t record_node_to_gc_snapshot(jl_value_t *a) JL_NOTSAFEPOINT { self_size = jl_is_array_type(type) ? jl_array_nbytes((jl_array_t*)a) : (size_t)jl_datatype_size(type); - + // print full type + // TODO: We _definitely_ have types longer than 1024 bytes.... ios_t str_; ios_mem(&str_, 1024); JL_STREAM* str = (JL_STREAM*)&str_; @@ -254,6 +259,82 @@ size_t record_node_to_gc_snapshot(jl_value_t *a) JL_NOTSAFEPOINT { return node_idx; } +typedef pair inlineallocd_field_type_t; + +// TODO(PR): remove this +static bool debug_log = false; + +bool _fieldpath_for_slot_helper( + vector& out, jl_datatype_t *objtype, + void *obj, void *slot) +{ + int nf = (int)jl_datatype_nfields(objtype); + jl_svec_t *field_names = jl_field_names(objtype); + if (debug_log) { + jl_((jl_value_t*)objtype); + jl_printf(JL_STDERR, "obj: %p, slot: %p, nf: %d\n", obj, (void*)slot, nf); + } + for (int i = 0; i < nf; i++) { + jl_datatype_t *field_type = (jl_datatype_t*)jl_field_type(objtype, i); + void *fieldaddr = (char*)obj + jl_field_offset(objtype, i); + ostringstream ss; // NOTE: must have same scope as field_name, below. + string field_name; + // TODO: NamedTuples should maybe have field names? Maybe another way to get them? + if (jl_is_tuple_type(objtype) || jl_is_namedtuple_type(objtype)) { + ss << "[" << i << "]"; + field_name = ss.str().c_str(); // See scope comment, above. + } else { + jl_sym_t *name = (jl_sym_t*)jl_svecref(field_names, i); + field_name = jl_symbol_name(name); + } + if (debug_log) { + jl_printf(JL_STDERR, "%d - field_name: %s fieldaddr: %p\n", i, field_name.c_str(), fieldaddr); + } + if (fieldaddr >= slot) { + out.push_back(inlineallocd_field_type_t(objtype, field_name)); + return true; + } + // If the field is an inline-allocated struct + if (jl_stored_inline((jl_value_t*)field_type)) { + bool found = _fieldpath_for_slot_helper(out, field_type, fieldaddr, slot); + if (found) { + out.push_back(inlineallocd_field_type_t(field_type, field_name)); + return true; + } + } + } + return false; +} + +vector _fieldpath_for_slot(jl_value_t *obj, void *slot) { + jl_datatype_t *vt = (jl_datatype_t*)jl_typeof(obj); + // TODO(PR): Remove this debugging code + if (vt->name->module == jl_main_module) { + // debug_log = true; + } + + vector result; + bool found = _fieldpath_for_slot_helper(result, vt, obj, slot); + + debug_log = false; + + // TODO: maybe don't need the return value here actually...? + if (!found) { + // TODO: Debug these failures. Some of them seem really wrong, like with the slot + // being _kilobytes_ past the start of the object for an object with 1 pointer and 1 + // field... + jl_printf(JL_STDERR, "WARNING: No fieldpath found for obj: %p slot: %p ", (void*)obj, (void*)slot); + jl_datatype_t* type = (jl_datatype_t*)jl_typeof(obj); + if (jl_is_datatype(type)) { + jl_printf(JL_STDERR, "typeof: "); + jl_static_show(JL_STDERR, (jl_value_t*)type); + } + jl_printf(JL_STDERR, "\n"); + } + // NOTE THE RETURNED VECTOR IS REVERSED + return result; +} + void _gc_heap_snapshot_record_root(jl_value_t *root, char *name) JL_NOTSAFEPOINT { record_node_to_gc_snapshot(root); @@ -286,7 +367,7 @@ size_t _record_stack_frame_node(HeapSnapshot *snapshot, jl_gcframe_t *frame) { // outgoing edges vector(), }; - + auto node_idx = snapshot->nodes.size(); snapshot->node_ptr_to_index_map.insert(val, {frame, node_idx}); snapshot->nodes.push_back(frame_node); @@ -336,35 +417,25 @@ void _gc_heap_snapshot_record_module_edge(jl_module_t *from, jl_value_t *to, cha g_snapshot->names.find_or_create_string_id(name)); } -void _gc_heap_snapshot_record_object_edge(jl_value_t *from, jl_value_t *to, size_t field_index) JL_NOTSAFEPOINT { +void _gc_heap_snapshot_record_object_edge(jl_value_t *from, jl_value_t *to, void* slot) JL_NOTSAFEPOINT { jl_datatype_t *type = (jl_datatype_t*)jl_typeof(from); - if (field_index < 0 || field_index > jl_datatype_nfields(type)) { - // TODO: We're getting -1 in some cases - jl_printf(JL_STDERR, "WARNING - incorrect field index (%d) for type\n", field_index); - jl_(type); - return; - } - - // TODO: It seems like NamedTuples should have field names? Maybe there's another way to get them? - if (jl_is_tuple_type(type) || jl_is_namedtuple_type(type)) { - // TODO: Maybe not okay to match element and object - _record_gc_edge("object", "element", from, to, field_index); - return; - } - if (field_index < 0 || jl_datatype_nfields(type) <= field_index) { - // TODO: We're getting -1 in some cases - //jl_printf(JL_STDERR, "WARNING - incorrect field index (%zu) for type\n", field_index); - //jl_(type); - _record_gc_edge("object", "element", from, to, field_index); - return; + auto field_paths = _fieldpath_for_slot(from, slot); + // Build the new field name by joining the strings, and/or use the struct + field names + // to create a bunch of edges + nodes + // (iterate the vector in reverse - the last element is the first path) + // TODO: Prefer to create intermediate edges and nodes instead of a combined string path. + string path; + for (auto it = field_paths.rbegin(); it != field_paths.rend(); ++it) { + // ... + path += it->second; + if ( it + 1 != field_paths.rend() ) { + path += "."; + } } - jl_svec_t *field_names = jl_field_names(type); - jl_sym_t *name = (jl_sym_t*)jl_svecref(field_names, field_index); - const char *field_name = jl_symbol_name(name); _record_gc_edge("object", "property", from, to, - g_snapshot->names.find_or_create_string_id(field_name)); + g_snapshot->names.find_or_create_string_id(path)); } void _gc_heap_snapshot_record_internal_edge(jl_value_t *from, jl_value_t *to) JL_NOTSAFEPOINT { diff --git a/src/gc-heap-snapshot.h b/src/gc-heap-snapshot.h index a273881db257f..13c8b26500bee 100644 --- a/src/gc-heap-snapshot.h +++ b/src/gc-heap-snapshot.h @@ -20,7 +20,7 @@ void _gc_heap_snapshot_record_task_to_frame_edge(jl_task_t *from, jl_gcframe_t * void _gc_heap_snapshot_record_frame_to_frame_edge(jl_gcframe_t *from, jl_gcframe_t *to) JL_NOTSAFEPOINT; void _gc_heap_snapshot_record_array_edge(jl_value_t *from, jl_value_t *to, size_t index) JL_NOTSAFEPOINT; void _gc_heap_snapshot_record_module_edge(jl_module_t *from, jl_value_t *to, char *name) JL_NOTSAFEPOINT; -void _gc_heap_snapshot_record_object_edge(jl_value_t *from, jl_value_t *to, size_t field_index) JL_NOTSAFEPOINT; +void _gc_heap_snapshot_record_object_edge(jl_value_t *from, jl_value_t *to, void* slot) JL_NOTSAFEPOINT; // Used for objects managed by GC, but which aren't exposed in the julia object, so have no // field or index. i.e. they're not reacahable from julia code, but we _will_ hit them in // the GC mark phase (so we can check their type tag to get the size). @@ -62,9 +62,9 @@ static inline void gc_heap_snapshot_record_module_edge(jl_module_t *from, jl_val _gc_heap_snapshot_record_module_edge(from, to, name); } } -static inline void gc_heap_snapshot_record_object_edge(jl_value_t *from, jl_value_t *to, size_t field_index) JL_NOTSAFEPOINT { +static inline void gc_heap_snapshot_record_object_edge(jl_value_t *from, jl_value_t *to, void* slot) JL_NOTSAFEPOINT { if (__unlikely(gc_heap_snapshot_enabled)) { - _gc_heap_snapshot_record_object_edge(from, to, field_index); + _gc_heap_snapshot_record_object_edge(from, to, slot); } } static inline void gc_heap_snapshot_record_internal_edge(jl_value_t *from, jl_value_t *to) JL_NOTSAFEPOINT { diff --git a/src/gc.c b/src/gc.c index 39ed63444916f..6d9d050c8140d 100644 --- a/src/gc.c +++ b/src/gc.c @@ -1959,8 +1959,7 @@ STATIC_INLINE int gc_mark_scan_obj8(jl_ptls_t ptls, jl_gc_mark_sp_t *sp, gc_mark if (*pnew_obj) { verify_parent2("object", parent, slot, "field(%d)", gc_slot_to_fieldidx(parent, slot)); - gc_heap_snapshot_record_object_edge(parent, *slot, - gc_slot_to_fieldidx(parent, slot)); + gc_heap_snapshot_record_object_edge(parent, *slot, slot); } if (!gc_try_setmark(*pnew_obj, &obj8->nptr, ptag, pbits)) continue; @@ -1996,8 +1995,7 @@ STATIC_INLINE int gc_mark_scan_obj16(jl_ptls_t ptls, jl_gc_mark_sp_t *sp, gc_mar verify_parent2("object", parent, slot, "field(%d)", gc_slot_to_fieldidx(parent, slot)); // TODO: Should this be *parent? Given the way it's used above? - gc_heap_snapshot_record_object_edge(parent, *slot, - gc_slot_to_fieldidx(parent, slot)); + gc_heap_snapshot_record_object_edge(parent, *slot, slot); } if (!gc_try_setmark(*pnew_obj, &obj16->nptr, ptag, pbits)) continue; @@ -2032,8 +2030,7 @@ STATIC_INLINE int gc_mark_scan_obj32(jl_ptls_t ptls, jl_gc_mark_sp_t *sp, gc_mar if (*pnew_obj) { verify_parent2("object", parent, slot, "field(%d)", gc_slot_to_fieldidx(parent, slot)); - gc_heap_snapshot_record_object_edge(parent, *slot, - gc_slot_to_fieldidx(parent, slot)); + gc_heap_snapshot_record_object_edge(parent, *slot, slot); } if (!gc_try_setmark(*pnew_obj, &obj32->nptr, ptag, pbits)) continue;