some inlining cost model updates

[JeffBezanson]

• treat arrayset like arrayref
• higher cost for passing unknown things to builtins
• add small cost for forward branches
• add small cost for sizeof, nfields, isa, subtype

In the spirit of #31455. I tried to keep this quite conservative and unlikely to affect high-performance code. If everyone thinks this looks reasonable I'll run benchmarks.

Some quick stats:
Before:

Base  ─────────── 36.138898 seconds
Total ───────  93.147351 seconds 
Generating precompile statements... 1713 generated in 133.823332 seconds (overhead  99.003688 seconds)
-rw-r--r-- 1 jeff jeff  12616483 Mar 22 13:52 corecompiler.ji
-rwxr-xr-x 1 jeff jeff 150780360 Mar 22 13:59 sys.so

After:

Base  ─────────── 35.660582 seconds
Total ───────  92.700790 seconds 
Generating precompile statements... 1720 generated in 130.747812 seconds (overhead  95.763560 seconds)
-rw-r--r-- 1 jeff jeff  12401243 Mar 23 10:48 corecompiler.ji
-rwxr-xr-x 1 jeff jeff 148853832 Mar 23 10:55 sys.so

[vtjnash]

add small cost for sizeof, nfields, isa, subtype

These seem like they should also be even higher cost, since if they're still present, that means we couldn't constant fold them.

And we should do nanosoldier, even if just to know which benchmarks will be affected after merging.

[JeffBezanson]

My hope was that would be covered by looking at the argument types; with abstract types they will generally be pretty expensive, concrete types not constant-folded might be expensive but might be in-between, e.g. sizeof on a String that does a load. Maybe isa and subtype should have higher costs though.

[vtjnash]

Those also may be just a pointer comparison. Okay, makes sense this is handled by the abstract type cost.

[timholy]

These all look fine to me. In the longer run, should we consider trying to tune these numbers? Maybe even to the user's specific CPU?

[JeffBezanson]

Oops, I messed up the type check here.

[JeffBezanson]

This also fixes the following inlining bug:

julia> const k = [1];

julia> f() = k[1];

julia> @code_typed f()
CodeInfo(
1 ─ %1 = invoke Base.getindex(Main.k::Array{Int64,1}, 1::Int64)::Int64
└──      return %1
) => Int64

All in all, I was hoping this PR would make us do less inlining, but I think it's going to end up just moving things around and improving the decisions, not necessarily doing more or less overall.

[JeffBezanson]

Ok I think this is ready to go. Will run benchmarks if CI passes.

[JeffBezanson]

@nanosoldier runbenchmarks(ALL, vs=":master")

[nanosoldier]

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

[JeffBezanson]

The regressions in Random are due to inlining being rearranged in an unlucky way that prevents us from removing an allocation we used to remove, the Sampler here I believe:

rand!(rng::AbstractRNG, A::AbstractArray{T}, X) where {T}             = rand!(rng, A, Sampler(rng, X))

Looking through it, the new inlining decisions seem totally reasonable, it's just that they're not able to take this into account. So we may want to revisit this after #34126, which should make argument passing cost more uniform.

[JeffBezanson]

@nanosoldier runbenchmarks(ALL, vs=":master")

[nanosoldier]

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

[vtjnash]

That looks quite positive!

@nanosoldier runbenchmarks("random", vs=":master")

[nanosoldier]

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

[vtjnash]

bump?

[JeffBezanson]

@nanosoldier runbenchmarks("random", vs=":master")

[JeffBezanson]

I rebased this, and after some experiments decided to pare it down to some minimal changes. The other changes seemed to make some things a bit slower.

[nanosoldier]

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

[vtjnash]

@nanosoldier runbenchmarks(ALL, vs=":master")

[nanosoldier]

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @christopher-dG

[KristofferC]

Looks pretty good, some things that might be worth looking at:

["array", "cat", "4467"] | 5.16 (5%) | 1.13 (1%)
["random", "types", ("rand", "MersenneTwister", "Complex{Int16}")] | 2.31 (25%) | 1.00 (1%)
["array", "index", ("sumelt_boundscheck", "100000:-1:1")] | 2.00 (50%) | 1.00 (1%)

[vtjnash]

I'm still happy with this. @JeffBezanson merge?

In the array cat example above, it is a moderately complex function which shouldn't usually need inlining. However, vcat is a varargs function, so we're not adequately accounting for the dynamic cost of choosing not to inline that. We should consider giving a boost to Vararg functions which will have a different dynamic cost (due to different compilation signature), but that is separate from this change. We could also consider changing vcat literal lowering to avoid making a tuple, but I don't know if that is necessary.