Skip to content

Conversation

@serenity4
Copy link
Contributor

@serenity4 serenity4 commented May 5, 2025

Implements #20.

Tests don't all pass at the moment, it seems that only code instances with StructureCache() and ADCache() as owners get invalidated:

julia> display(getproperty.(cached, :max_world))
8-element Vector{UInt64}:
 0xffffffffffffffff
 0x0000000000009824
 0x0000000000009824
 0xffffffffffffffff
 0xffffffffffffffff
 0xffffffffffffffff
 # these two were from another (previous) `refresh()`
 0x0000000000009814
 0x0000000000009814

julia> for ci in cached; println(ci.owner); end
DAECompiler.RHSSpec(DAECompiler.TornCacheKey(BitSet([1]), BitSet([]), BitSet([]), BitSet([]), Pair{BitSet, BitSet}[]), 1)
DAECompiler.StructureCache()
DAECompiler.ADCache()
DAECompiler.RHSSpec(DAECompiler.TornCacheKey(BitSet([1]), BitSet([]), BitSet([]), BitSet([]), Pair{BitSet, BitSet}[]), 1)
DAECompiler.TornIRSpec(DAECompiler.TornCacheKey(BitSet([1]), BitSet([]), BitSet([]), BitSet([]), Pair{BitSet, BitSet}[]))
DAECompiler.SICMSpec(DAECompiler.TornCacheKey(BitSet([1]), BitSet([]), BitSet([]), BitSet([]), Pair{BitSet, BitSet}[]))
DAECompiler.StructureCache()
DAECompiler.ADCache()

which I can believe might be due to the other keys not being singletons, which may surface a bug - as currently implemented edges should be identical across all owners, except maybe ADCache().

@serenity4
Copy link
Contributor Author

serenity4 commented May 5, 2025

This requires JuliaLang/julia#58324 to avoid a crash during invalidation.

@serenity4 serenity4 force-pushed the invalidate-caches-on-refresh branch from ffcb4b2 to 620f9f1 Compare May 5, 2025 23:30
@serenity4 serenity4 force-pushed the invalidate-caches-on-refresh branch from 620f9f1 to 64bcf7c Compare May 5, 2025 23:31
@serenity4
Copy link
Contributor Author

Tests now pass locally.

I'm thinking that instead of passing edges around from top-level like currently implemented (which is a bit cumbersome), we could instead always add an edge to the single CodeInstance that is the source in cache_dae_ci!. Does that sound correct to you?

@Keno
Copy link
Member

Keno commented May 7, 2025

Does that sound correct to you?

Yes, I suggested this in one of the review comments (#24 (comment))

@serenity4
Copy link
Contributor Author

Indeed, I just wanted to confirm you weren't only talking about that specific case 👍

Keno pushed a commit to JuliaLang/julia that referenced this pull request May 9, 2025
I am not sure when we may be in the situation where `edges` is `null`,
but if we do, we crash.

I triggered such a crash in
JuliaComputing/DAECompiler.jl#24, which manually adds
edges to handcrafted code instances. I may be doing something wrong, but
in any case, it doesn't hurt to be more robust (and perhaps handle this
situation more gracefully).

---------

Co-authored-by: Jameson Nash <[email protected]>
@serenity4
Copy link
Contributor Author

Tests are passing, this is good for final review & merge.

@Keno Keno merged commit 3b6e14a into JuliaComputing:main May 11, 2025
2 checks passed
charleskawczynski pushed a commit to charleskawczynski/julia that referenced this pull request May 12, 2025
…ang#58324)

I am not sure when we may be in the situation where `edges` is `null`,
but if we do, we crash.

I triggered such a crash in
JuliaComputing/DAECompiler.jl#24, which manually adds
edges to handcrafted code instances. I may be doing something wrong, but
in any case, it doesn't hurt to be more robust (and perhaps handle this
situation more gracefully).

---------

Co-authored-by: Jameson Nash <[email protected]>
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