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.

Sign up for GitHub

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

Ensure invalidated target gets logged #49048

Merged
merged 1 commit into from
Mar 20, 2023
Merged

Ensure invalidated target gets logged #49048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/gf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2021,6 +2021,11 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method
}
jl_array_ptr_1d_push(oldmi, (jl_value_t*)mi);
invalidate_external(mi, max_world);
if (_jl_debug_method_invalidation) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't fully understand this code, but isn't this pushed into the list later if invalidated is set? Why do we need to push something here too? Is this supposed to have a loctag of jl_box_int32(depth=1); instead (and increment depth argument of invalidate_method_instance)?

Copy link
Member Author

@timholy timholy Mar 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method gets pushed, but not the specific MethodInstance. On previous versions, the format was:

backedge => depth (e.g., `mcc48954i::MethodInstance, 1` in the new test)
targetmi => "jl_method_table_insert" (`target = mc48954i::MethodInstance` in the new test)
targetm => "jl_method_table_insert" (`target = which(mc48954, (AbstractFloat, Int))::Method` in the new test)

Recent master has dropped the middle of this pair; this PR just restores it.

jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)mi);
loctag = jl_cstr_to_string("jl_method_table_insert");
jl_array_ptr_1d_push(_jl_debug_method_invalidation, loctag);
}
}
}
}
Expand Down
24 changes: 24 additions & 0 deletions test/worlds.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,3 +408,27 @@ wc_aiw2 = get_world_counter()
@test Base.invoke_in_world(wc_aiw2, f_inworld, 2) == "world two; x=2"
@test Base.invoke_in_world(wc_aiw1, g_inworld, 2, y=3) == "world one; x=2, y=3"
@test Base.invoke_in_world(wc_aiw2, g_inworld, 2, y=3) == "world two; x=2, y=3"

# logging
mc48954(x, y) = false
mc48954(x::Int, y::Int) = x == y
mc48954(x::Symbol, y::Symbol) = x == y
function mcc48954(container, y)
x = container[1]
return mc48954(x, y)
end

mcc48954(Any[1], 1)
mc48954i = method_instance(mc48954, (Any, Int))
mcc48954i = method_instance(mcc48954, (Vector{Any}, Int))
list48954 = ccall(:jl_debug_method_invalidation, Any, (Cint,), 1)
mc48954(x::AbstractFloat, y::Int) = x == y
ccall(:jl_debug_method_invalidation, Any, (Cint,), 0)
@test list48954 == [
mcc48954i,
1,
mc48954i,
"jl_method_table_insert",
which(mc48954, (AbstractFloat, Int)),
"jl_method_table_insert"
]