Skip to content
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

Method redefinitions #26

Closed
Sbozzolo opened this issue Apr 3, 2024 · 14 comments
Closed

Method redefinitions #26

Sbozzolo opened this issue Apr 3, 2024 · 14 comments

Comments

@Sbozzolo
Copy link

Sbozzolo commented Apr 3, 2024

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
Copy link
Member

chriselrod commented Apr 3, 2024

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
Copy link
Author

Sbozzolo commented Apr 3, 2024

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
Copy link
Member

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
Copy link
Author

Sbozzolo commented Apr 3, 2024

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
Copy link
Author

Sbozzolo commented Apr 3, 2024

This was run from a TestEnv environment, if that matters

@okartal
Copy link

okartal commented May 3, 2024

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
Copy link
Member

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

@chriselrod
Copy link
Member

chriselrod commented May 4, 2024

Specifically, what are the values in the comparisons

if nc != num_l1cache()
@eval num_l1cache() = static($nc)
end
if nc != num_cores()
@eval num_cores() = static($nc)
end
if syst != sys_threads()
@eval sys_threads() = static($syst)

CPUSummary.jl/src/x86.jl

Lines 53 to 55 in 99e2b46

cs !== PrecompiledCacheSize && _eval_cache_size(cs)
ci = CpuId.cacheinclusive()
ci !== PrecompiledCacheInclusive && _eval_cache_inclusive(ci)

It may help give some clue as to why they're different.

@ufechner7
Copy link

ufechner7 commented May 13, 2024

The following bug might be related, even though the error message is different: JuliaLang/julia#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
Copy link
Member

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
Copy link
Member

chriselrod commented May 13, 2024

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
Copy link

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
Copy link
Member

chriselrod commented May 13, 2024

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 33b606c

@chriselrod
Copy link
Member

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.

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

No branches or pull requests

5 participants