-
Notifications
You must be signed in to change notification settings - Fork 4
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
Invalidations #3
Comments
You shouldn't when starting Julia with If we want to fix these, the solution is to have those libraries / functions stop using |
See the good news in SciML/DifferentialEquations.jl#786 (emergency is over 🙂 ). We should still poke at this but it |
I did just start Julia with With using SnoopCompileCore
invalidations = @snoopr using OrdinaryDiffEq ModelingToolKit;
using SnoopCompile
trees = invalidation_trees(invalidations) and then look for a Again, I don't think this is an issue of primary importance, but it does seems worth keeping track of for a rainy day. The |
using SnoopCompileCore
invalidations = @snoopr using OrdinaryDiffEq, ModelingToolkit;
using SnoopCompile, CPUSummary
trees = invalidation_trees(invalidations);
ctrees = filtermod(CPUSummary, trees) I get julia> ctrees = filtermod(CPUSummary, trees)
1-element Vector{SnoopCompile.MethodInvalidations}:
inserting convert(S::Type{<:Union{Number, T}}, p::MultivariatePolynomials.AbstractPolynomialLike{T}) where T in MultivariatePolynomials at /home/chriselrod/.julia/packages/MultivariatePolynomials/vqcb5/src/conversion.jl:65 invalidated:
mt_backedges: 1: signature Tuple{typeof(convert), Type{Hwloc.Attribute}, Any} triggered MethodInstance for CPUSummary.safe_topology_load!() (1 children)
julia> Threads.nthreads(), Sys.CPU_THREADS
(8, 8) In another Julia session julia> ctrees = filtermod(CPUSummary, trees)
2-element Vector{SnoopCompile.MethodInvalidations}:
inserting convert(S::Type{<:Union{Number, T}}, p::MultivariatePolynomials.AbstractPolynomialLike{T}) where T in MultivariatePolynomials at /home/chriselrod/.julia/packages/MultivariatePolynomials/vqcb5/src/conversion.jl:65 invalidated:
mt_backedges: 1: signature Tuple{typeof(convert), Type{Hwloc.Attribute}, Any} triggered MethodInstance for CPUSummary.safe_topology_load!() (1 children)
deleting num_threads() in CPUSummary at /home/chriselrod/.julia/packages/CPUSummary/dEmFX/src/topology.jl:42 invalidated:
backedges: 1: superseding num_threads() in CPUSummary at /home/chriselrod/.julia/packages/CPUSummary/dEmFX/src/topology.jl:42 with MethodInstance for CPUSummary.num_threads() (2 children)
julia> Threads.nthreads(), Sys.CPU_THREADS
(1, 8)
(ode) pkg> st CPUSummary
Status `~/Documents/progwork/julia/env/ode/Project.toml`
[2a0fbf3d] CPUSummary v0.1.2 So it appears to be working as intended for me. |
I strongly suspect this is enough to favor |
I have $ env | grep -i thread
JULIA_CPU_THREADS=4 Is that possibly problematic? This is on a Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz (6 physical cores). |
Ah, yes. It doesn't actually use Perhaps, in the case of disagreement, I should favor |
At least on nightly, and when starting with Line 62 in f5315fc
Line 34 in f5315fc
Not easy for me to fix because method redefinition like this is not "typical" (I recognize you do amazing, atypical things) and I don't know the motivations well enough to offer an alternative. CC @Tokazama |
I'd be happy to offer an integration test that checks for new inference when running the precompiled workload for a demo consumer of LoopVectorization. If you want it, just let me know which repo I should submit it to. |
We should stop doing that. |
Which repo do you think would be best? LoopVectorization.jl itself, or something that depends on it like TriangularSolve.jl or RecursiveFactorization.jl? |
Probably LV itself. The only issue to be aware of is that tracking down the origin of breakage might require a bit of hunting: if a PR to, say, this package breaks the integration test, then you won't know you've broken it until you next run the tests of LoopVectorization.jl. Unless you like the idea of running that specific test in several of LV's dependencies? You can see somethng similar to what I mean in CodeTracking, which exists to serve Revise: |
Apologies I'm not sure what the original motivation for redefining it like this was. |
There's https://github.com/SciML/OrdinaryDiffEq.jl/blob/master/.github/workflows/Downstream.yml which is a quick way to setup a bunch of integration tests on subsets of downstream package tests. |
One thing to check: are you aware that you can use the precompilation process to your advantage? Your package can contain const some_value_or_type_that_must_be_known_to_inference = begin
# Some complicated computation, calling lots of functions, which may not be inferrable
end and the only thing that gets written to the Of course, if you need to some things in For things like setting the number of threads, can LV basically do the same thing we do with LLVM multiversioning? if Threads.nthreads() == 1
# single-threaded implementation
elseif Threads.nthreads() = 6 # my laptop has 6 physical cores
# 6-thread implementation
else
@debug "Non-optimized implementation"
# fallback
end For users who might want to customize the default number (I typically use 4 threads to reserve a couple for something besides Julia) we could use Preferences. |
@ChrisRackauckas, do you have a link to whatever sits on the opposite side of that workflow? It looks useful but I wasn't sure how to trigger it. |
https://github.com/SciML/OrdinaryDiffEq.jl/blob/master/test/runtests.jl#L5-L17 It just grabs the group and runs the subset of the tests. |
1 similar comment
https://github.com/SciML/OrdinaryDiffEq.jl/blob/master/test/runtests.jl#L5-L17 It just grabs the group and runs the subset of the tests. |
You give me too much credit! This was added for that, under the theory it's unlikely to change normally. All that said, one fix was to remove it from LoopVectorization: Long term, I'm not overly concerned about this library. |
|
Continuing from JuliaLang/julia#41913:
It's not just Polyester:
The way I have it set:
which may explain why I'm seeing those invalidations and others may not?
The text was updated successfully, but these errors were encountered: