Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heap Snapshot node names aren't parameterized for Types #47502

Closed
NHDaly opened this issue Nov 8, 2022 · 0 comments · Fixed by #47503
Closed

Heap Snapshot node names aren't parameterized for Types #47502

NHDaly opened this issue Nov 8, 2022 · 0 comments · Fixed by #47503
Assignees
Labels
feature Indicates new feature / enhancement requests profiler

Comments

@NHDaly
Copy link
Member

NHDaly commented Nov 8, 2022

The heap snapshot currently outputs names that are missing Parameters when printing the names of Types.

For example:
Screen Shot 2022-11-08 at 6 38 27 PM

The issue is that we are currently using this code to print the names of types:

else if (jl_is_datatype(a)) {
type_name = string("Type{") + string(jl_symbol_name_(((_jl_datatype_t*)a)->name->name)) + string("}");
name = StringRef(type_name);
self_size = sizeof(jl_task_t);
}

which does this:

julia> t2 = Tuple{Int,Int}
Tuple{Int64, Int64}

julia> static_shown(t2)
"Tuple{Int64, Int64}"

julia> t2.name.name
:Tuple

I think we should be instead following the approach used in jl_static_show to print the fully parameterized names of types. Something like this:

julia/src/rtutils.c

Lines 772 to 845 in 4ff526a

else if (vt == jl_datatype_type) {
// typeof(v) == DataType, so v is a Type object.
// Types are printed as a fully qualified name, with parameters, e.g.
// `Base.Set{Int}`, and function types are printed as e.g. `typeof(Main.f)`
jl_datatype_t *dv = (jl_datatype_t*)v;
jl_sym_t *globname;
int globfunc = is_globname_binding(v, dv) && is_globfunction(v, dv, &globname);
jl_sym_t *sym = globfunc ? globname : dv->name->name;
char *sn = jl_symbol_name(sym);
size_t quote = 0;
if (dv->name == jl_tuple_typename) {
if (dv == jl_tuple_type)
return jl_printf(out, "Tuple");
int taillen = 1, tlen = jl_nparams(dv), i;
for (i = tlen-2; i >= 0; i--) {
if (jl_tparam(dv, i) == jl_tparam(dv, tlen-1))
taillen++;
else
break;
}
if (taillen == tlen && taillen > 3) {
n += jl_printf(out, "NTuple{%d, ", tlen);
n += jl_static_show_x(out, jl_tparam0(dv), depth);
n += jl_printf(out, "}");
}
else {
n += jl_printf(out, "Tuple{");
for (i = 0; i < (taillen > 3 ? tlen-taillen : tlen); i++) {
if (i > 0)
n += jl_printf(out, ", ");
n += jl_static_show_x(out, jl_tparam(dv, i), depth);
}
if (taillen > 3) {
n += jl_printf(out, ", Vararg{");
n += jl_static_show_x(out, jl_tparam(dv, tlen-1), depth);
n += jl_printf(out, ", %d}", taillen);
}
n += jl_printf(out, "}");
}
return n;
}
if (globfunc) {
n += jl_printf(out, "typeof(");
}
if (jl_core_module && (dv->name->module != jl_core_module || !jl_module_exports_p(jl_core_module, sym))) {
n += jl_static_show_x(out, (jl_value_t*)dv->name->module, depth);
n += jl_printf(out, ".");
size_t i = 0;
if (globfunc && !jl_id_start_char(u8_nextchar(sn, &i))) {
n += jl_printf(out, ":(");
quote = 1;
}
}
n += jl_static_show_x_sym_escaped(out, sym);
if (globfunc) {
n += jl_printf(out, ")");
if (quote) {
n += jl_printf(out, ")");
}
}
if (dv->parameters && (jl_value_t*)dv != dv->name->wrapper) {
size_t j, tlen = jl_nparams(dv);
if (tlen > 0) {
n += jl_printf(out, "{");
for (j = 0; j < tlen; j++) {
jl_value_t *p = jl_tparam(dv,j);
n += jl_static_show_x(out, p, depth);
if (j != tlen-1)
n += jl_printf(out, ", ");
}
n += jl_printf(out, "}");
}
}
}

Maybe we can factor out a helper function that shares the same code for jl_static_show and this code?
Or maybe we even just want to directly call jl_static_show() on a stringstream? Is there a reason we didn't do this in the first place?

CC: @apaz-cli, @vtjnash, @IanButterworth, @gbaraldi, @vilterp.

Thanks all! :)

julia/src/rtutils.c

Lines 798 to 809 in 4ff526a

n += jl_printf(out, "Tuple{");
for (i = 0; i < (taillen > 3 ? tlen-taillen : tlen); i++) {
if (i > 0)
n += jl_printf(out, ", ");
n += jl_static_show_x(out, jl_tparam(dv, i), depth);
}
if (taillen > 3) {
n += jl_printf(out, ", Vararg{");
n += jl_static_show_x(out, jl_tparam(dv, tlen-1), depth);
n += jl_printf(out, ", %d}", taillen);
}
n += jl_printf(out, "}");

@NHDaly NHDaly added feature Indicates new feature / enhancement requests profiler labels Nov 8, 2022
KristofferC pushed a commit that referenced this issue Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Indicates new feature / enhancement requests profiler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants