invenia / Parallelism.jl

A library for threaded and distributed parallelism.
MIT License
8 stars 1 forks source link

Performance hazard in `fetch()`ing from a Task #7

Open NHDaly opened 4 years ago

NHDaly commented 4 years ago

I just discovered this repo today, and did some scanning through it, and encountered this: https://github.com/invenia/Parallelism.jl/blob/159a138d562a1583e44129340a0dfa784a34c523/src/tmap.jl#L9-L12

This will end up boxing every value as an Any, so you'll end up always returning a Vector{Any}.

Probably this would be better served by pre-allocating a vector of Ts, and then assigning the results into it? But I guess you don't know what the types will be.... Maybe you could either take it as an input to the function, or use Core.Compiler.return_type() like Julia does?: https://github.com/JuliaLang/julia/blob/55a6dab76329b693f0fab372b1a80289bff01a90/base/array.jl#L660

So ultimately, something like:

 T = ... # Maybe `Core.Compiler.return_type(f, eltype(xss))`?
 vs = Vector{T}(undef, length(xss))
 for (i,x) in enumerate(xss)
     Threads.@spawn vs[i] = f(x...) 
 end 
 return vs

This is the pattern we've been using so far, and it seems to mostly work okay.

oxinabox commented 4 years ago

Good idea. We have mostly be using tmap for things that are fundermentally very expensive, and that are minimally consumed afterwards. e.g. tmaping over a few hundred different simulations.

NHDaly commented 4 years ago

yeah, makes sense! :)

I just remember Valentin telling me about the ::Any box inside Tasks and that we should avoid fetching from them. But yeah, i agree given what you said it's probably not relevant 👍

oxinabox commented 4 years ago

A PR would be accepted, if you are interested in using this for that case.