Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prevent fill! from inlining #26418

Merged
merged 1 commit into from
Mar 14, 2018
Merged

prevent fill! from inlining #26418

merged 1 commit into from
Mar 14, 2018

Conversation

KristofferC
Copy link
Sponsor Member

Fixes https://discourse.julialang.org/t/performance-degradation-of-fill-in-latest-julia-0-7-dev/9648. Not sure why inlining this function is problematic though. It contains a loop though so perhaps it shouldn't really be inlined?

@ararslan ararslan added performance Must go faster arrays [a, r, r, a, y, s] labels Mar 12, 2018
@KristofferC
Copy link
Sponsor Member Author

@nanosoldier runbenchmarks(ALL, vs = ":master")

@ararslan
Copy link
Member

Nanosoldier ran into this, which is why it never got back to you: JuliaWeb/HTTP.jl#220. Not sure whether this is intermittent with the newer HTTP version it's using, so I'll run it again: @nanosoldier runbenchmarks(ALL, vs=":master")

@ararslan
Copy link
Member

Okay Nanosoldier is just kind of sad and broken at the moment. Going to revert the HTTP upgrade, which means he'll ignore people again, but he'll actually work when he doesn't ignore people.

@ararslan
Copy link
Member

@nanosoldier runbenchmarks(ALL, vs=":master")

@ararslan
Copy link
Member

The server isn't starting, so I'm taking it offline for maintenance for a while. Sorry for the inconvenience!

@ararslan
Copy link
Member

@nanosoldier runbenchmarks(ALL, vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

@KristofferC KristofferC merged commit ead9eab into master Mar 14, 2018
@fredrikekre fredrikekre deleted the kc/noinline_fill branch March 14, 2018 08:45
mbauman added a commit that referenced this pull request Apr 5, 2019
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
JeffBezanson pushed a commit that referenced this pull request Jan 31, 2020
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
KristofferC pushed a commit that referenced this pull request Apr 11, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants