iszero type piracy causes a lot of invalidations

[KristofferC]

The definition here:

ApproxFunBase.jl/src/Fun.jl

Line 506 in 6637707

iszero(x::Number) = x == 0

changes the definition of iszero for all numbers (type piracy) and causes a huge number of invalidations

 inserting iszero(x::Number) in ApproxFunBase at /home/kc/.julia/packages/ApproxFunBase/r0W9A/src/Fun.jl:506 invalidated:
   backedges: 1: superseding iszero(x) in Base at number.jl:40 with MethodInstance for iszero(::UInt32) (1 children)
              2: superseding iszero(x) in Base at number.jl:40 with MethodInstance for iszero(::UInt64) (2 children)
              3: superseding iszero(x) in Base at number.jl:40 with MethodInstance for iszero(::Int64) (6236 children) <---------------

You can see the effect of this by loading ApproxFunBase and notice that the whole julia session becomes sluggish while it is recompiling everything.

[dlfivefifty]

Thanks! That’s a bizarre one how that got there

[KristofferC]

Seems to have come from https://github.com/JuliaApproximation/ApproxFun.jl/pull/488/files#diff-315609d5896fe40b083c84a9432a2361R444.

[dlfivefifty]

Thanks! It looks like I didn't import iszero when I added the function (so was meant to be ApproxFun.iszero), and later added import Base: iszero.

I'll fix it now.

[KristofferC]

Never use import Base: foo :P

[dlfivefifty]

I always use import Base: foo 🤣 That way when Base: foo becomes LinearAlgebra: foo its a one-line change.

[dlfivefifty]

I'm also following the official convention advocated by StdLib: https://github.com/JuliaLang/julia/blob/395e47f4da4630a521333ad67eccf614c56aaf9c/stdlib/LinearAlgebra/src/LinearAlgebra.jl#L11

[KristofferC]

I'm also following the official convention advocated by StdLib:

Most of the code in the stdlibs were written at a time when people didn't know better 🤣

[dlfivefifty]

I think introspection tools are a better way to go, say a GitHub action that does whatever it is you did to find these types of errors. Expecting developer style (and more importantly refactoring 100k's of existing Julia code) to prevent this is unrealistic.

[KristofferC]

Accidental method extension is a problem So I stand by my advice to not use a way of coding which makes that easy.

[dlfivefifty]

You might be right, but it's probably easier to get Julia to spit out a warning if a package overloads functions with types that do not belong to that package than it is to re-write every single Julia package in that style...

(Yes I'm aware it's not so simple because of "sanctioned" overloading like OffsetArrays.jl, but I'm sure some macro could be introduced or something to stop the warning.)

[timholy]

We're about to announce a competition on discourse...shhh, don't tell, this one is the current record-holder for the largest number of invalidations caused by a single method definition. A whopping 13% of all MethodInstances on Julia 1.6!

No one who has read this is allowed to submit it, that honor goes to @KristofferC.

[dlfivefifty]

I had no idea back in 2014 that my hacked together package would go on to cause so much damage to the Julia ecosystem... (And I guess given energy usage, the real ecosystem as well!) 🤯

[timholy]

No worries, we've all learned a lot in the last few months about this stuff.

[timholy]

ImageCore had these: JuliaImages/ImageCore.jl#131. A mere 200ish invalidations, so I bow before your greatness, but no better from a piracy perspective.

[dlfivefifty]

The worst part is I've spent literally weeks trying to debug why ApproxFun.jl loading was so slow via code-bisection.... was probably very close to finding this culprit

[timholy]

How much did it change with this PR?

[dlfivefifty]

Oh actually not that much, 15.1s v 15.3s...

[KristofferC]

I think the biggest difference will be in sluggishness in the REPL and package manager etc after loading the package.

[KristofferC]

Also, even with this fixed there is still a large number of invalidations going on (e.g. invenia/Intervals.jl#144).

[timholy]

Yeah, in general when you fix an invalidation problem in PkgA, it's PkgB (which depends on PkgA) that will thank you. So if you're still thinking this package is really slow to load, start using @snoopr on the packages it depends on.

[dlfivefifty]

I’m in the process of completely redesigning it via ContinuumArrays.jl which is much snappier already, so I won’t be looking into it