Skip to content

Commit

Permalink
rand() performance restored with faster code path and inlining.
Browse files Browse the repository at this point in the history
  • Loading branch information
@ViralBShah
ViralBShah committed Nov 7, 2014
1 parent ab0d6ca commit 84b6aec
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions base/random.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ globalRNG() = GLOBAL_RNG

# rand: a non-specified RNG defaults to GLOBAL_RNG

rand() = rand(GLOBAL_RNG)
rand(T::Type) = rand(GLOBAL_RNG, T)
@inline rand() = rand_close_open(GLOBAL_RNG)
@inline rand(T::Type) = rand(GLOBAL_RNG, T)
rand(::()) = rand(GLOBAL_RNG, ()) # needed to resolve ambiguity
rand(dims::Dims) = rand(GLOBAL_RNG, dims)
rand(dims::Int...) = rand(dims)
Expand All @@ -137,7 +137,7 @@ rand!(A::AbstractArray) = rand!(GLOBAL_RNG, A)

## random floating point values

rand(r::AbstractRNG) = rand(r, Float64)
@inline rand(r::AbstractRNG) = rand_close_open(r)

# MersenneTwister
rand(r::MersenneTwister, ::Type{Float64}) = rand_close_open(r)
Expand Down

4 comments on commit 84b6aec

@rfourquet
Copy link
Member

Choose a reason for hiding this comment

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

@ViralBShah Great! I remember having tested inlined rand() at some point (before #8854 I think, and without other @inlines of this commit) but noticing degraded performances somewhere else (I think for the array version rand(n::Int)). What I find now is that this commit slows down randn[!] methods by up to 50%, do you confirm?
I just pushed a branch where randmtzig_randn uses a non-inlined version of rand which seem to restore previous perfs of randn, but this starts to make me feel uneasy with all those @inline tweaks. Should I open a PR? (In this branch, I also revert/modify part of this commit because I think rand_close_open is dSFMT specific so I prefer to keep rand(r::AbstractRNG) = rand(r, Float64) which I find more self-documenting, do you agree?).

@ViralBShah
Copy link
Member Author

Choose a reason for hiding this comment

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

On reverting part of the commit - does it affect performance? Given the code path and sensitivity to inlining, I think we can leave it as it is for now.

On randn I will comment on the PR.

@rfourquet
Copy link
Member

Choose a reason for hiding this comment

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

The reverting part of the commit doesn't affect performance either way, so I will remove that from the branch.

@timholy
Copy link
Member

@timholy timholy commented on 84b6aec Nov 8, 2014

Choose a reason for hiding this comment

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

Agreed with @vtjnash that it's best to be quite conservative about adding @inline.

Please sign in to comment.