Don't special-case inference of splatting ::Any parameters #22364
Conversation
martinholters
force-pushed
the
mh/splat_any
branch
from
42f27c3 to
4ce6a40
Compare
|
Hard to be sure but it looks like this might increase CI time on travis. I guess there's no reason it would only affect travis but the times there are worrying. |
|
Just re-running the Travis Job for OSX brought it from a timeout down to 1:10. Maybe the Travis workers just had a bad day; will re-run the other jobs, too. For reference: 32bit: 1:59, 64bit: 1:45. |
|
Nope, hardly any improvements in the timings. Strange it only affects Linux, though. |
test/inference.jl
Outdated
| @@ -938,3 +938,10 @@ copy_dims_pair(out, dim::Int, tail...) = copy_dims_out(out => dim, tail...) | |||
| copy_dims_pair(out, dim::Colon, tail...) = copy_dims_out(out => dim, tail...) | |||
| @test Base.return_types(copy_dims_pair, (Tuple{}, Vararg{Union{Int,Colon}})) == Any[Tuple{}, Tuple{}, Tuple{}] | |||
| @test all(m -> 5 < count_specializations(m) < 25, methods(copy_dims_out)) | |||
|
|
|||
| # splatting an ::Any should still allow inference to use types of parameters preceeding it | |||
|
Travis on 32bit timed out (while storing the cache, tests passed), restarted. Looking at recent timings of Travis running against master, the times fluctuate a lot, with the times I see in this PR consistently at the upper end. Bad luck or problem of this change? |
|
I went through past Travis runs on master and the average runtimes on 32bit have jumped up starting at build no. 50331, I even found two timeouts. But that build is triggered by the merge of #21996, which seems rather unlikely as being the cause here. And the most recent builds look better again. So I might just have been over-interpreting noise. The question whether this PR makes anything worse is still open, though. My current thinking is to rebase it in a couple of days and see what's happening then, unless anyone has better ideas. |
|
I think Travis CI vCPU allocation might have changed (or just got randomly got a sequence of poor worker assignments). We print timings on many things before starting the tests, and those had became proportionally much slower also. @nanosoldier |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
|
Recent 32bit Travis runs on master have a high probably of timing out, too, so this PR should be good go. Any objections against merging? EDIT: Should be squashed upon merge. |
I don't think we need this ad-hoc heuristic anymore after #21933.