Skip to content

Commit

Permalink
Revert #26418, remove noinline annotation from fill!
Browse files Browse the repository at this point in the history
This used to be necessary to avoid a strange edge case in the compiler, but it is no longer necessary -- and can now in fact cause other performance snags.

Using the test case from [the original discourse post that prompted #26418](https://discourse.julialang.org/t/performance-degradation-of-fill-in-latest-julia-0-7-dev/9648):

```julia
julia> @Btime fill(1.0,5,5);
  49.335 ns (1 allocation: 288 bytes)

julia> @Btime fill(0.0,5,5);
  52.773 ns (1 allocation: 288 bytes)

julia> @Btime fill(0.0,5,5);
  46.724 ns (1 allocation: 288 bytes)

julia> @Btime fill(1.0,5,5);
  42.202 ns (1 allocation: 288 bytes)
```

Even more compelling is the case for a larger array where LLVM can exploit some sort of wider/simdier implementation for zeros when this gets inlined thanks to constant propagation:

```julia
julia> A = Array{Float64}(undef, 1000, 1000);

julia> @Btime fill!($A,0.0);
  345.103 μs (0 allocations: 0 bytes)

julia> @Btime fill!($A,1.0);
  458.976 μs (0 allocations: 0 bytes)
```

Ref https://discourse.julialang.org/t/performance-of-filling-an-array/22788
  • Loading branch information
mbauman committed Apr 5, 2019
1 parent db428ef commit 67e4eda
Showing 1 changed file with 0 additions and 1 deletion.
1 change: 0 additions & 1 deletion base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,6 @@ copyto!(dest::Array{T}, src::Array{T}) where {T} = copyto!(dest, 1, src, 1, leng
# N.B: The generic definition in multidimensional.jl covers, this, this is just here
# for bootstrapping purposes.
function fill!(dest::Array{T}, x) where T
@_noinline_meta
xT = convert(T, x)
for i in eachindex(dest)
@inbounds dest[i] = xT
Expand Down

0 comments on commit 67e4eda

Please sign in to comment.