Skip to content

Conversation

timholy
Copy link
Member

@timholy timholy commented Mar 20, 2025

No description provided.

@timholy
Copy link
Member Author

timholy commented Mar 21, 2025

I now have a much clearer understanding of what is going wrong, but I still lack an understanding of why.

In case later changes fix it, the report in question is https://github.com/timholy/SnoopCompile.jl/actions/runs/13989097828/job/39168795257?pr=413

Here's the logic: https://github.com/timholy/SnoopCompile.jl/blob/teh/debug_1.11/test/testmodules/Stale/StaleC/src/StaleC.jl should "heal" invalidations in the build_stale call chain. So the appearance of use_stale in the recompilation result https://github.com/timholy/SnoopCompile.jl/blob/a8f0490237f0c26ac50d35baca38f1298f532b80/test/snoop_inference.jl#L873-L877

InferenceTimingNode: 0.038472/0.054491 on Core.Compiler.Timings.ROOT() with 2 direct children
├─ InferenceTimingNode: 0.000088/0.000097 on StaleB.useA() with 1 direct children
│  └─ InferenceTimingNode: 0.000009/0.000009 on getproperty(StaleA::Module, stale::Symbol) with 0 direct children
└─ InferenceTimingNode: 0.015922/0.015922 on StaleA.use_stale(::Vector{Any}) with 0 direct children
unexpected result: 2 backedges

is surprising. "0 direct children" seems to suggest that nothing "needed" it, and a test a little higher up in the file https://github.com/timholy/SnoopCompile.jl/blob/a8f0490237f0c26ac50d35baca38f1298f532b80/test/snoop_inference.jl#L858 explicitly checks that there is a valid compiled instance. So I can't understand why this method is being recompiled. It is being triggered by the https://github.com/timholy/SnoopCompile.jl/blob/teh/debug_1.11/test/snoop_inference.jl#L875 line (if you comment it out, use_stale isn't recompiled).

Locally, this happens only with coverage=true, with coverage=false it behaves as I expect.

@timholy
Copy link
Member Author

timholy commented Mar 21, 2025

Summary of a discussion on slack: this is a consequence of avoiding cached native code:

in conjunction with the more recent (Julia 1.11.3 and Julia 1.11.4) removal of the inferred code:

JuliaLang/julia#56749

The plan is to revert the latter on Julia 1.11, hopefully landing in Julia 1.11.5. Meanwhile, I'll edit the test to compensate.

The signature: in

    mius = only(methodinstances(StaleA.use_stale))
    cius = mius.cache
    @test cius.max_world == typemax(UInt)
    @show cius.specptr cius.invoke cius.inferred

one gets

cius.specptr = Ptr{Nothing} @0x0000000000000000
cius.invoke = Ptr{Nothing} @0x0000000000000000
cius.inferred = nothing

when coverage is on. (When coverage is off, those pointers are non-NULL.)

@timholy timholy changed the title [DO NOT MERGE] Debug test failures on 1.11 Fix test failures on 1.11 Mar 21, 2025
Mostly, this compensates for a bug present in Julia 1.11.3 and 1.11.4.
xref #413 (comment)
@timholy
Copy link
Member Author

timholy commented Mar 21, 2025

Ah, lovely.

@timholy timholy merged commit 5d48a06 into master Mar 21, 2025
12 of 15 checks passed
@timholy timholy deleted the teh/debug_1.11 branch March 21, 2025 16:24
timholy added a commit to JuliaLang/julia that referenced this pull request Mar 22, 2025
…)"

This reverts commit bdf8219.

Rationale: when coverage is on, both the native code and the inferred code
might be eliminated, a complete loss of all precompilation results.
There are intentions to adopt a new strategy for Julia 1.12, but in the
meantime we should revert this change since it is "just" a sysimg size
reduction.

xref JuliaDebug/SnoopCompile.jl#413 (comment)
KristofferC pushed a commit to JuliaLang/julia that referenced this pull request Mar 25, 2025
…eless inferred code" (#57864)

This reverts commit bdf8219, from
#56749

**Note that this PR is made against `backports-release-1.11`.**

Rationale: when coverage is on, both the native code and the inferred
code might be eliminated, a complete loss of all precompilation results.
There are intentions to adopt a new strategy for Julia 1.12, but in the
meantime we should revert this change since it is "just" a sysimg size
reduction.

Affected Julia versions: 1.11.3, 1.11.4

xref
JuliaDebug/SnoopCompile.jl#413 (comment)
KristofferC pushed a commit to JuliaLang/julia that referenced this pull request Mar 25, 2025
…eless inferred code" (#57864)

This reverts commit bdf8219, from
#56749

**Note that this PR is made against `backports-release-1.11`.**

Rationale: when coverage is on, both the native code and the inferred
code might be eliminated, a complete loss of all precompilation results.
There are intentions to adopt a new strategy for Julia 1.12, but in the
meantime we should revert this change since it is "just" a sysimg size
reduction.

Affected Julia versions: 1.11.3, 1.11.4

xref
JuliaDebug/SnoopCompile.jl#413 (comment)
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.

1 participant