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

Improved performance #5

Closed
7 tasks done
haberdashPI opened this issue Sep 15, 2019 · 9 comments
Closed
7 tasks done

Improved performance #5

haberdashPI opened this issue Sep 15, 2019 · 9 comments

Comments

@haberdashPI
Copy link
Owner

haberdashPI commented Sep 15, 2019

I'm slowly working through benchmarks to make sure each sampleat! implementation is entirely stack based.

Types to check

  • Arrays
  • Functions
  • Numbers
  • until and after
  • pad, append
  • mapsignal and derivatives
  • filtersignal and derivatives
@haberdashPI haberdashPI added this to the 0.2.0 milestone Sep 15, 2019
@haberdashPI
Copy link
Owner Author

This will probably benefit from the refactoring in #2

@haberdashPI haberdashPI mentioned this issue Sep 15, 2019
6 tasks
@haberdashPI
Copy link
Owner Author

Idea: since, sink_helper! dispatches on checkpoint objects we can get efficient code for each channel count by including channel count in the type signature of checkpoint objects.

haberdashPI added a commit that referenced this issue Sep 17, 2019
@haberdashPI
Copy link
Owner Author

haberdashPI commented Sep 27, 2019

Questions for current benchmark test

Why are there so many allocations?
Is @Base.propagate_inbounds working?
Is inline of single loop functions working?
How does the @code_warntype of sampleat! look?

@haberdashPI
Copy link
Owner Author

haberdashPI commented Sep 28, 2019

My earlier idea turns out to be the case: for sampleat!, if we return a tuple, we need to know how many channels there are based on the type of the signal.

It is probably not necessary to use a tuple, I think a lazy array type that just artificially broadcasts across the channels should do fine.

@haberdashPI
Copy link
Owner Author

haberdashPI commented Sep 30, 2019

Notes:

  1. The use of views may be an issue; see Julia issue #14955

  2. There's something wrong with how AxisArrays are handled, I think. @trace (from Traceur) seems to think there are dynamic dispatches to a number of AxisArrray methods.

@haberdashPI
Copy link
Owner Author

I've convinced myself that there are likely not any type instabilities in the code well enough that I think it is worth seeing how performance improves if I avoid array views.

There still could be an issue with AxisArrays, and it would be worth attempting some code that avoids them (currently they are always used) just to see how that goes, if removing the array views doesn't work.

@haberdashPI
Copy link
Owner Author

There is still a type instability:

julia> z = mix(x,y) |> tosamplerate(1000Hz)
julia> @trace SignalOperators.sampleat!(out,z,SignalOperators.SignalTrait(z),1,1,checks[5]) modules=[SignalOperators]
┌ Warning: dynamic dispatch to SignalOperators.writesink!(SignalOperators.one_sample, 1, %new(FillArrays.Fill{Float64,1,Tuple{Base.OneTo{Int64}}}, 0.0, Core.tuple(%new(Base.OneTo{Int64}, Base.ifelse(Base.slt_int(Base.arraysize(Base.getfield(Base.getfield(Base.getfield(Base.getfield(x, padded_signals), 1, $(Expr(:boundscheck))), signal), data), 2), 0), 0, Base.arraysize(Base.getfield(Base.getfield(Base.getfield(Base.getfield(x, padded_signals), 1, $(Expr(:boundscheck))), signal), data), 2))))))
└ @ ~/Documents/work/projects/SignalOperators/src/mapsignal.jl:-1
┌ Warning: dynamic dispatch to SignalOperators.writesink!(SignalOperators.one_sample, 1, %new(AxisArrays.AxisArray{Float64,1,SubArray{Float64,1,Array{Float64,2},Tuple{Int64,Base.Slice{Base.OneTo{Int64}}},true},Tuple{AxisArrays.Axis{:channel,UnitRange{Int64}}}}, %new(SubArray{Float64,1,Array{Float64,2},Tuple{Int64,Base.Slice{Base.OneTo{Int64}}},true}, Base.getfield(Base.getfield(Base.getfield(Base.getfield(x, padded_signals), 2, $(Expr(:boundscheck))), signal), data), Core.tuple(j, %new(Base.Slice{Base.OneTo{Int64}}, %new(Base.OneTo{Int64}, Base.ifelse(Base.slt_int(Base.arraysize(Base.getfield(Base.getfield(Base.getfield(Base.getfield(x, padded_signals), 2, $(Expr(:boundscheck))), signal), data), 2), 0), 0, Base.arraysize(Base.getfield(Base.getfield(Base.getfield(Base.getfield(x, padded_signals), 2, $(Expr(:boundscheck))), signal), data), 2))))), Base.sub_int(Base.add_int(Base.add_int(1, Base.mul_int(Base.sub_int(j, 1), 1)), Base.mul_int(0, Base.mul_int(1, Base.sub_int(Base.ifelse(Base.slt_int(Base.arraysize(Base.getfield(Base.getfield(Base.getfield(Base.getfield(x, padded_signals), 2, $(Expr(:boundscheck))), signal), data), 1), 0), 0, Base.arraysize(Base.getfield(Base.getfield(Base.getfield(Base.getfield(x, padded_signals), 2, $(Expr(:boundscheck))), signal), data), 1)), 0)))), Base.mul_int(Base.mul_int(1, Base.sub_int(Base.ifelse(Base.slt_int(Base.arraysize(Base.getfield(Base.getfield(Base.getfield(Base.getfield(x, padded_signals), 2, $(Expr(:boundscheck))), signal), data), 1), 0), 0, Base.arraysize(Base.getfield(Base.getfield(Base.getfield(Base.getfield(x, padded_signals), 2, $(Expr(:boundscheck))), signal), data), 1)), 0)), 1)), Base.mul_int(1, Base.sub_int(Base.ifelse(Base.slt_int(Base.arraysize(Base.getfield(Base.getfield(Base.getfield(Base.getfield(x, padded_signals), 2, $(Expr(:boundscheck))), signal), data), 1), 0), 0, Base.arraysize(Base.getfield(Base.getfield(Base.getfield(Base.getfield(x, padded_signals), 2, $(Expr(:boundscheck))), signal), data), 1)), 0))), Core.tuple(Base.getfield(Base.getfield(Base.getfield(Base.getfield(Base.getfield(x, padded_signals), 2, $(Expr(:boundscheck))), signal), axes), 2, true))))
└ @ ~/Documents/work/projects/SignalOperators/src/mapsignal.jl:-1
2-element Array{Float64,1}:
 0.3300021471853958
 0.8360567643791674

@haberdashPI
Copy link
Owner Author

This prior issues turned out to be because of a number of different sources of memory allocation during sampleat!; I've now got a handle on it and am going through each of the types to make sure no allocations are happening for each type of signal.

@haberdashPI
Copy link
Owner Author

Closed by e0ec3f0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant