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

Some broadcast performance tweaks #19879

Merged
merged 1 commit into from
Jan 6, 2017

Conversation

pabloferz
Copy link
Contributor

@pabloferz pabloferz commented Jan 5, 2017

This allows dot-calls to fully specialize on the first argument when it is a type. The solution here does not extend to type arguments in other positions. For that, something like #19829 for type arguments would be needed, but I'm not sure that is possible in general (except for built-in types).

This PR also allows to remove shape checking in broadcast! when applying an @inbounds to it.

Should fix #19849

@KristofferC KristofferC added performance Must go faster broadcast Applying a function over a collection labels Jan 5, 2017
@pabloferz
Copy link
Contributor Author

With this patch, on my machine, I get

julia> A = collect(0.0:0.1:100.0);

julia> @benchmark (x->trunc(Int,x)).($A)
BenchmarkTools.Trial: 
  memory estimate:  8.00 kb
  allocs estimate:  1
  --------------
  minimum time:     2.635 μs (0.00% GC)
  median time:      2.738 μs (0.00% GC)
  mean time:        3.113 μs (6.93% GC)
  maximum time:     124.929 μs (92.14% GC)
  --------------
  samples:          10000
  evals/sample:     9
  time tolerance:   5.00%
  memory tolerance: 1.00%

julia> @benchmark trunc.(Int, $A)
BenchmarkTools.Trial: 
  memory estimate:  8.00 kb
  allocs estimate:  1
  --------------
  minimum time:     2.662 μs (0.00% GC)
  median time:      2.798 μs (0.00% GC)
  mean time:        3.061 μs (6.39% GC)
  maximum time:     69.634 μs (90.93% GC)
  --------------
  samples:          10000
  evals/sample:     9
  time tolerance:   5.00%
  memory tolerance: 1.00%

@tkelman
Copy link
Contributor

tkelman commented Jan 5, 2017

Nice. A little off-topic, but I sent you an email (at the address you use in your gitconfig) on new year's eve, did it go to spam?

@pabloferz
Copy link
Contributor Author

I just had a look at the email you sent me (it got lost among a bunch of other emails). That's a really nice new year's surprise. Thank you!

@pabloferz
Copy link
Contributor Author

pabloferz commented Jan 5, 2017

Let's give my new granted powers a run: @nanosoldier runbenchmarks(ALL, vs = ":master")

@tkelman
Copy link
Contributor

tkelman commented Jan 5, 2017

I think you need to click a confirm button somewhere, github likes to hide these things

@pabloferz
Copy link
Contributor Author

Found it. Let's try again. @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 @jrevels

@martinholters
Copy link
Member

I had another idea the other day: To make the outer layer broadcast[!] inspect its argument types, and if any of them are scalar, inline them in the function argument, so that e.g. with T=Int and xs=rand(5), broadcast(f, T, xs) would call broadcast(a -> f(T, a), xs). I'm not sure whether that is at all possible without loosing inferability. The problem boils down to being able to write something like

function fixarg{i}(f, ::Type{Val{i}}, v)
    (args...) -> begin
        f(args[1:i-1]..., v, args[i:end]...)
    end
end

so that foo() = fixarg(^, Val{2}, 2)(3) could be inferred. I think this is impossible as it would need to instantiate a new closure type based on the Val{i} type, i.e. during inference. Can someone confirm this or is the general idea worth pursuing?

@tkelman
Copy link
Contributor

tkelman commented Jan 6, 2017

like #19724?

@martinholters
Copy link
Member

Yes, of course, thanks for the pointer! Let me see whether I can make something useful out of that...

@pabloferz
Copy link
Contributor Author

pabloferz commented Jan 6, 2017

There's also some discussion of other possible approaches here

@JeffBezanson JeffBezanson merged commit 236a8af into JuliaLang:master Jan 6, 2017
@pabloferz pabloferz deleted the pz/bc-typearg branch January 6, 2017 22:18
@stevengj
Copy link
Member

stevengj commented Jan 7, 2017

We can't do #19829 for type objects, because there's no "type literal" in Julia... something like Int is just an identifier, and it is only resolved to be a type much later in the compilation process (long after lowering).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broadcast Applying a function over a collection performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Large performance regression in trunc with vector argument
7 participants