Skip to content

Commit

Permalink
Rebase: Performance regression of scalar randn() between Julia 1.4 an…
Browse files Browse the repository at this point in the history
…d 1.5 (#39319)

* definition of randn for scalars reverted to 1.4

* Added comments why this change is done

Co-authored-by: Benjamin Lungwitz <52384612+lungben@users.noreply.github.com>
  • Loading branch information
KristofferC and lungben authored Jan 19, 2021
1 parent d6ef750 commit 6813340
Showing 1 changed file with 20 additions and 1 deletion.
21 changes: 20 additions & 1 deletion stdlib/Random/src/normal.jl
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,26 @@ julia> randn(rng, ComplexF32, (2, 3))
0.611224+1.56403im 0.355204-0.365563im 0.0905552+1.31012im
```
"""
@inline randn(rng::AbstractRNG=default_rng()) = _randn(rng, rand(rng, UInt52Raw()))
@inline function randn(rng::AbstractRNG=default_rng())
#=
When defining
`@inline randn(rng::AbstractRNG=default_rng()) = _randn(rng, rand(rng, UInt52Raw()))`
the function call to `_randn` is currently not inlined, resulting in slightly worse
performance for scalar random normal numbers than repeating the code of `_randn`
inside the following function.
=#
@inbounds begin
r = rand(rng, UInt52Raw())

# the following code is identical to the one in `_randn(rng::AbstractRNG, r::UInt64)`
r &= 0x000fffffffffffff
rabs = Int64(r>>1) # One bit for the sign
idx = rabs & 0xFF
x = ifelse(r % Bool, -rabs, rabs)*wi[idx+1]
rabs < ki[idx+1] && return x # 99.3% of the time we return here 1st try
return randn_unlikely(rng, idx, rabs, x)
end
end

@inline function _randn(rng::AbstractRNG, r::UInt64)
@inbounds begin
Expand Down

4 comments on commit 6813340

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - successfully executed benchmarks. A full report can be found here. cc @christopher-dG

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

Please sign in to comment.