JuliaSIMD / CPUSummary.jl

MIT License
7 stars 4 forks source link

Method redefinitions #26

Closed Sbozzolo closed 4 months ago

Sbozzolo commented 5 months ago

Precompiling on 1.10, I see:

WARNING: Method definition num_l1cache() in module CPUSummary at /home/sbozzolo/.julia/packages/CPUSummary/LAKF1/src/x86.jl:9 overwritten at /home/sbozzolo/.julia/packages/CPUSummary/LAKF1/src/CPUSummary.jl:60.
WARNING: Method definition num_cores() in module CPUSummary at /home/sbozzolo/.julia/packages/CPUSummary/LAKF1/src/x86.jl:10 overwritten at /home/sbozzolo/.julia/packages/CPUSummary/LAKF1/src/CPUSummary.jl:63.
WARNING: Method definition cache_size(Union{Base.Val{1}, Static.StaticInt{1}}) in module CPUSummary at /home/sbozzolo/.julia/packages/CPUSummary/LAKF1/src/x86.jl:26 overwritten on the same line (check for duplicate calls to `include`).
WARNING: Method definition cache_size(Union{Base.Val{2}, Static.StaticInt{2}}) in module CPUSummary at /home/sbozzolo/.julia/packages/CPUSummary/LAKF1/src/x86.jl:26 overwritten on the same line (check for duplicate calls to `include`).
WARNING: Method definition cache_size(Union{Base.Val{3}, Static.StaticInt{3}}) in module CPUSummary at /home/sbozzolo/.julia/packages/CPUSummary/LAKF1/src/x86.jl:26 overwritten on the same line (check for duplicate calls to `include`).

With

[[deps.CPUSummary]]
deps = ["CpuId", "IfElse", "PrecompileTools", "Static"]
git-tree-sha1 = "601f7e7b3d36f18790e2caf83a882d88e9b71ff1"
uuid = "2a0fbf3d-bb9c-48f3-b0a9-814d99fd7ab9"
version = "0.2.4"
chriselrod commented 5 months ago

Are you doing anything that might make your CPU look different when the package precompiles vs when it is loaded?

It precompiles a configuration for your CPU. If that configuration is wrong when loaded, it corrects it. This results in those redefinitions.

Sbozzolo commented 5 months ago

Thanks for the explanation (and the very quick response)!

The package where I found this is very standard Julia with nothing fancy.

I'll keep an eye on this and see if I can trace the origin better.

chriselrod commented 5 months ago

Are you running on some kind of cluster with a shared filesystem? It wouldn't be the Julia packages, but the way you start Julia, or where you are running them.

Sbozzolo commented 5 months ago

Are you running on some kind of cluster with a shared filesystem? It wouldn't be the Julia packages, but the way you start Julia, or where you are running them.

No, this was on my laptop. This is my /proc/cpuinfo

processor       : 0
vendor_id       : GenuineIntel
cpu family      : 6
model           : 186
model name      : 13th Gen Intel(R) Core(TM) i5-1340P
stepping        : 2
microcode       : 0x4119
cpu MHz         : 1072.685
cache size      : 12288 KB
physical id     : 0
siblings        : 16
core id         : 0
cpu cores       : 12
apicid          : 0
initial apicid  : 0
fpu             : yes
fpu_exception   : yes
cpuid level     : 32
wp              : yes
flags           : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc art arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid aperfmperf tsc_known_freq pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb ssbd ibrs ibpb stibp ibrs_enhanced tpr_shadow flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt clwb intel_pt sha_ni xsaveopt xsavec xgetbv1 xsaves split_lock_detect avx_vnni dtherm ida arat pln pts hwp hwp_notify hwp_act_window hwp_epp hwp_pkg_req hfi vnmi umip pku ospke waitpkg gfni vaes vpclmulqdq rdpid movdiri movdir64b fsrm md_clear serialize arch_lbr ibt flush_l1d arch_capabilities
vmx flags       : vnmi preemption_timer posted_intr invvpid ept_x_only ept_ad ept_1gb flexpriority apicv tsc_offset vtpr mtf vapic ept vpid unrestricted_guest vapic_reg vid ple shadow_vmcs ept_mode_based_exec tsc_scaling usr_wait_pause
bugs            : spectre_v1 spectre_v2 spec_store_bypass swapgs eibrs_pbrsb
bogomips        : 4377.60
clflush size    : 64
cache_alignment : 64
address sizes   : 39 bits physical, 48 bits virtual
power management:
Sbozzolo commented 5 months ago

This was run from a TestEnv environment, if that matters

okartal commented 4 months ago

This was run from a TestEnv environment, if that matters

I get the same warning when running test from the REPL if I use ReTestItems.jl

chriselrod commented 4 months ago

It would help if you dev the package and show both values on the check that failed, thus causing it to redefine methods.

chriselrod commented 4 months ago

Specifically, what are the values in the comparisons https://github.com/JuliaSIMD/CPUSummary.jl/blob/99e2b4696104bbac13c6aca9541916f27e0300e4/src/CPUSummary.jl#L59-L66 https://github.com/JuliaSIMD/CPUSummary.jl/blob/99e2b4696104bbac13c6aca9541916f27e0300e4/src/x86.jl#L53-L55 It may help give some clue as to why they're different.

ufechner7 commented 4 months ago

The following bug might be related, even though the error message is different: https://github.com/JuliaLang/julia/issues/54448

Precompiling project...
  ? CPUSummary
  ? PolyesterWeave
  82 dependencies successfully precompiled in 34 seconds. 6 already precompiled.
  2 dependencies failed but may be precompilable after restarting julia
  2 dependencies had output during precompilation:
┌ PolyesterWeave
│  WARNING: Method definition cache_size(Union{Base.Val{1}, Static.StaticInt{1}}) in module CPUSummary at C:\Users\ufechner\.julia\packages\CPUSummary\LAKF1\src\x86.jl:26 overwritten on the same line (check for duplicate calls to `include`).
│  ERROR: Method overwriting is not permitted during Module precompilation. Use `__precompile__(false)` to opt-out of precompilation.
└
┌ CPUSummary
│  WARNING: Method definition cache_size(Union{Base.Val{1}, Static.StaticInt{1}}) in module CPUSummary at C:\Users\ufechner\.julia\packages\CPUSummary\LAKF1\src\x86.jl:26 overwritten on the same line (check for duplicate calls to `include`).
│  ERROR: Method overwriting is not permitted during Module precompilation. Use `__precompile__(false)` to opt-out of precompilation.
└

For me this is very annoying, if I a tell a class of 30 students to install my software and they encounter this kind of bugs this is really annoying if you try to promote the use of Julia...

chriselrod commented 4 months ago

For me this is very annoying, if I a tell a class of 30 students to install my software and they encounter this kind of bugs this is really annoying if you try to promote the use of Julia...

Well, I can't reproduce. If you make a PR fixing it, I can review and merge.

chriselrod commented 4 months ago

Alternatively, you can make PRs to remove CPUSummary.jl from the dependencies of the DiffEq ecosystem.

There are 8 dependent packages: https://juliahub.com/ui/Packages/General/CPUSummary I believe only these four are used by the DiffEq ecosystem: VectorizationBase PolyesterWeave Polyester LoopVectorization

Just dev these packages, remove it from the dependencies, and replace any functionality that they were using. I don't believe any of them were using cache size information. The main thing they probably used is just cacheline size, which you can define as

if Sys.ARCH === :AArch64 # Apple silicon
const CACHELINESIZE = 128 # A64FX actually wants 256...
else # 64 is more common
const CACHELINESIZE = 64
end

It is a performance rather than a correctness thing.

jaakkor2 commented 4 months ago

Commenting out _extra_init() in https://github.com/JuliaSIMD/CPUSummary.jl/blob/v0.2.4/src/precompile.jl#L9 seems to make the error message go away on Julia 1.10.3.

chriselrod commented 4 months ago

Commenting out _extra_init() in https://github.com/JuliaSIMD/CPUSummary.jl/blob/v0.2.4/src/precompile.jl#L9 seems to make the error message go away on Julia 1.10.3.

Oops, that is really bad. Was added here https://github.com/JuliaSIMD/CPUSummary.jl/commit/33b606c2cfe2f55fef7dc716a1fe8e2bdf8dcd0f

chriselrod commented 4 months ago

It's still worth asking why feature detection is changing on your systems at all if you aren't doing anything weird, but it should NOT be running _extra_init() during precompilation. A precompile(_extra_init, ()) type statement would probably be fine, but running it during precompilation is not.