Faster promoting arithmetic

[timholy]

Reference: #21065. The main reason to introduce this is so that it can be backported to 0.5; perhaps on 0.6 there can be a deeper solution. I could re-file this explicitly against release-0.5 if that's preferable.

[martinholters]

How about this instead:

--- a/base/promotion.jl
+++ b/base/promotion.jl
@@ -171,6 +171,7 @@ promote_result{T,S}(::Type{T},::Type{S},::Type{Bottom},::Type{Bottom}) = (@_pure
 promote() = ()
 promote(x) = (x,)
 function promote{T,S}(x::T, y::S)
+    @_inline_meta
     (convert(promote_type(T,S),x), convert(promote_type(T,S),y))
 end
 promote_typeof(x) = (@_pure_meta; typeof(x))

[timholy]

I wondered about that. There are occasionally weird things that can happen performance-wise when you change the "level" that gets inlined (e.g., JuliaImages/ImageCore.jl#26). But I guess with nanosoldier there is little reason not to try and find out what happens.

[martinholters]

Let's ask @nanosoldier runbenchmarks(ALL, vs = ":master") and then again for the inlining of promote version and see whether that gives any insights how better to proceed.

[nanosoldier]

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

[stevengj]

It seems like it would be better to fix promote(x,y)... inlining.

[KristofferC]

IIUC, that is being fixed for 0.6 and this is a minimal PR towards 0.5.2?

[stevengj]

This shouldn't this be a PR against the 0.5 branch rather than master?

[timholy]

IIUC, that is being fixed for 0.6 and this is a minimal PR towards 0.5.2?

Sort of. Forced inlining of promote is not currently part of #21069. I'll submit a version that does both just so we can get benchmarks, and then generate separate PRs against master and release-0.5.

[timholy]

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

[martinholters]

Are these changes necessary in addition to the inlining of promote?

[StefanKarpinski]

Yes, I really don't like having to expand these like this – it's very ugly and we've never had to do so before.

[timholy]

Certainly much less important with #21069. The main problem is that as we implement more abstractions, the inlining heuristics sometimes seem to fail to realize how simple something really is. I don't think we can exclude the possibility that we're noticing problems that we didn't notice before.

[nanosoldier]

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

[martinholters]

So maybe inlining promote is not such a good idea after all...

[timholy]

Now that's what I'd call an uninspiring benchmark result. I expected some regressions, but that really took the idea and ran with it.

[martinholters]

So back to just rewriting +, *, -, / as done in the first commit it is, I guess. Is there enough benefit to do that in addition to #21069? Or should we do that just for 0.5? And/or should we backport #21069 to 0.5?

[timholy]

I'd be content with #21069 alone, if it is backportable. It's certainly the most far-reaching of the solutions.

Adding the first commit is another 35% faster on this particular benchmark; let's see what others think of whether that's worth it.

[martinholters]

Backport of #21069 at and #21080.