Skip to content

Update @snoop_inference for Julia 1.12 #419

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

Draft
wants to merge 7 commits into
base: dev
Choose a base branch
from
Draft

Conversation

timholy
Copy link
Owner

@timholy timholy commented Apr 15, 2025

This leverages the new timing fields of CodeInstance to replace the old Core.Compiler.Timings module. There is a bit of loss particularly in the domain of constant-propagation, which is illustrated in https://gist.github.com/timholy/9a64b27c1932bb414e69b8fe48284b5e. (Read testcase.jl before looking at the results.) As a consequence there are a small handful of failing tests locally, but the vast majority pass.

This requires JuliaLang/julia#58124 or alternatively (with some modification to this PR) JuliaLang/julia#58123.

@timholy timholy requested review from vtjnash and NHDaly April 15, 2025 12:33
const newly_inferred = CodeInstance[]

function start_tracking()
islocked(snoop_inference_lock) && error("already tracking inference (cannot nest `@snoop_inference` blocks)")
Copy link
Collaborator

Choose a reason for hiding this comment

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

(this really should be a Base feature, since implementing it indirectly in packages is not very robust)

Suggested change
islocked(snoop_inference_lock) && error("already tracking inference (cannot nest `@snoop_inference` blocks)")
snoop_inference_lock.reentrancy_cnt == 1 || throw(ConcurrencyViolationError("already tracking inference (cannot nest `@snoop_inference` blocks)"))

finish_snoop_inference()
end
function stop_tracking()
@assert islocked(snoop_inference_lock)
Copy link
Collaborator

@vtjnash vtjnash Apr 15, 2025

Choose a reason for hiding this comment

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

nit: more concisely and correctly

Suggested change
@assert islocked(snoop_inference_lock)
Base.assert_havelock(snoop_inference_lock)

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.

2 participants