Skip to content

[RFC] Log a backtrace on entrance to type inference (option 2) #58124

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

timholy
Copy link
Member

@timholy timholy commented Apr 15, 2025

This is needed to eliminate the Core.Compiler.Timings module while still keeping SnoopCompile working.

This is one mechanism to add a backtrace to each entrance to inference. An alternative approach is presented in #58123. Only one of these should be merged.

This is needed to eliminate the `Core.Compiler.Timings` module while
still keeping SnoopCompile working.
@vchuravy
Copy link
Member

I have a weak preference for this over option 1.

@timholy
Copy link
Member Author

timholy commented Apr 15, 2025

I like this one better too.

@@ -1432,6 +1438,7 @@ function typeinf_ext_toplevel(methods::Vector{Any}, worlds::Vector{UInt}, trim_m
inspected = IdSet{CodeInstance}()
tocompile = Vector{CodeInstance}()
codeinfos = []
bt = collect_dispatch_backtrace[] ? backtrace() : nothing
Copy link
Member

Choose a reason for hiding this comment

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

This method is not actually relevant to dispatch, so these additional changes (just the first one) should not be included in this PR

Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

I am neutral between these PR, as they seem to express entirely different operations (first-dispatch vs first-infer) which may occur at different times in different places. The one thing that must occur before merging though is adding a lock around them (c.f. jl_push_newly_inferred for possible inspiration, as it seems to be fairly common that we need a primitive "locked array push" similar to that)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants