Performance fixes for rand #156
Conversation
The current code is `zero(DoubleFloat(T))`, which seems like a typo of `DoubleFloat(zero(T))`. Even though this branch is reached only rarely/never, this change should help Julia generate better code.
Codecov ReportBase: 48.75% // Head: 48.78% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #156 +/- ##
==========================================
+ Coverage 48.75% 48.78% +0.02%
==========================================
Files 63 63
Lines 3425 3425
==========================================
+ Hits 1670 1671 +1
+ Misses 1755 1754 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
`s===0` is equivalent to `isa(s,Int)&iszero(s)` which doesn't seem like the intended behavior. Also changed `||` to `|` because these comparisons don't seem to be costly enough to warrant the branch.
| @@ -1,8 +1,9 @@ | |||
| function rand(rng::AbstractRNG, ::Random.SamplerTrivial{Random.CloseOpen01{DoubleFloat{T}}}) where {T<:IEEEFloat} | |||
| hi, lo = rand(rng, T, 2) | |||
| hi = rand(rng, T) | |||
| lo = rand(rng, T) | |||
There was a problem hiding this comment.
This is unfortunate because vectorized random numbers are faster to generate. Do we need a better idiom in Julia for generating a small fixed amount of random numbers?
There was a problem hiding this comment.
Maybe this won't be needed once the compiler get the full power of escape analysis.
There was a problem hiding this comment.
I guess that StaticArrays hits the same problem with their rand functions. I tried taking a look at the code, but all the generated functions made me nauseous: https://github.com/JuliaArrays/StaticArrays.jl/blob/master/src/arraymath.jl#L38
On the other hand, maybe the only real fix would be for Julia to provide a contiguous stack-allocated array type, because I don't think that tuple operations can be vectorized because they don't necessarily have contiguous layout in memory.
There was a problem hiding this comment.
I did some measurements now, and it turns out that separate rand() calls seem to be faster than rand!(array), at least for tiny lengths like 2 and 4. For length two:
using Random, BenchmarkTools
f(::Val{n}) where {n} = ntuple(i -> rand(), Val{n}())
# warm up
rand!(zeros(2))
f(Val(2))
@benchmark rand!(a) setup = (a = zeros(2);) # Time (median): 5.691 ns
@benchmark rand!(a) setup = (a = zeros(2); Random.seed!(1234)) # Time (median): 5.851 ns
@benchmark f(Val(2)) # Time (median): 4.949 ns
@benchmark f(Val(2)) setup = (Random.seed!(1234);) # Time (median): 4.950 ns
There was a problem hiding this comment.
The above measurements are with nightly Julia.
|
Thank you. This is great. |
Some small fixes.