Use Span<T> to reduce memory pressure

[ladeak]

This way a user may pre-allocate a buffer for calling the FFTpower and FFTfreq and then reuse those arrays later, during the lifetime of the application.

• Adding Span overloads on FFT transform methods to avoid allocation of arrays and with that reducing pressure on LOH and GC.
• Adding dependency on System.Memory to use spans in netstandard2.0
• Using ArrayBuffer to rent temporary arrays for intermediate calculations
• Adding unit tests

[swharden]

Hi @ladeak, thank you very much for this PR!

I want to review this topic a little more thoroughly so it may take me a few days before I'm able to merge it, but I greatly appreciate this contribution.

Note that implementing Memory<T> and Span<T> (instead of double[]) for all mathematical operations was proposed a while back for ScottPlot (ScottPlot/ScottPlot#766) and we didn't go for it at that time because it would have made the API too complex. This implementation is very elegant though, and what I learn by working on this PR may translate nicely to that project as well.

[ladeak]

One thing to consider is to multitarget netstandard2.0 and net5.0 (net6 in a few weeks). In net5.0 the nuget package is part of the appframework, so we would not need to list it explicitly.

[swharden]

Hey @ladeak, I finally got some time to go through this thoroughly. Thanks for this PR!

I removed span support for FftFreq() - that method generates an array from nothing and returns it. I don't think anyone would create a span and pass it into this function to be mutated, but let me know if I'm overlooking a use case justifying it.

I'm a little concerned about the try statements with no exception handling. What errors are they supposed to be catching?

[swharden]

One thing to consider is to multitarget netstandard2.0 and net5.0 (net6 in a few weeks). In net5.0 the nuget package is part of the appframework, so we would not need to list it explicitly.

I updated this to use multi-targeting with conditional package dependencies 😎

<TargetFrameworks>netstandard2.0;net5.0;</TargetFrameworks>

<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
  <PackageReference Include="System.ValueTuple" Version="4.5.0" />
  <PackageReference Include="System.Memory" Version="4.5.4" />
</ItemGroup>

[swharden]

I'm a little concerned about the try statements with no exception handling. What errors are they supposed to be catching?

I removed them and it builds and tests pass so I'm interested in your thoughts on this. If the PR looks good let me know and I'll merge and push it to NuGet 👍 🚀

[ladeak]

Multitargeting looks reasonable.
If you want to, we could consider converting the Demo project to SDK style csproj or to a NET5 based WinForms app, and then use Directory.Build.props file to define package versions. (but I would probably consider doing this in a separate PR).

I'm a little concerned about the try statements with no exception handling. What errors are they supposed to be catching?

The benefit of having try-finally statements is that in case of an exception, the rented array is still returned to the pool. Without exception block, the exception will be handled just the same way as without the try-finally block. So without a try-finally block if exceptions are thrown, we would likely decrease efficiency, as the pool needs to create new instances when it runs out pooled items. I am just searching for sample examples mentioning this pattern without much success, but see the pattern in .net bcl itself example1, example2.

I planned to use FFTfreq with a Span overload. I have a huge while( forever ) loop, in which I calculate FFTpower and FFTfreq too. I did not want GC to be hit every time; but looking at this.

// For audio we typically want the FFT amplitude (in dB)
fftPower = FftSharp.Transform.FFTpower(fftBuffer);

// Create an array of frequencies for each point of the FFT
fftFreq = FftSharp.Transform.FFTfreq(_sampleRate, fftPower.Length);

But thinking out this loud, my sampleRate does not change during the app lifecycle, and the input fftBuffer is the same length always, it means that fftPower.Length is going to be constant too, right? - If so I would be fine without the Span overload of FFTfreq.

[swharden]

Exception Handling

The benefit of having try-finally statements is that in case of an exception, the rented array is still returned to the pool

I agree we should prevent that memory leak, but I don't love wrapping FFT() in a try that has no catch. The inability to calculate a FFT is an exceptional event, and should throw so the program halts if it happens.

I added a catch statement that re-throws the exception. I think this allows the user to optionally wrap the call in their own try statement, and even if they don't catch the memory should still be freed. Does this solution seem good to you?

try
{
    var buffer = temp.AsSpan(0, input.Length);
    MakeComplex(buffer, input);
    FFT(buffer);
    buffer.Slice(0, destination.Length).CopyTo(destination);
}
catch (Exception ex)
{
    throw new Exception("Could not calculate FFT", ex);
}
finally
{
    ArrayPool<Complex>.Shared.Return(temp);
}

FFTfreq()

But thinking out this loud, my sampleRate does not change during the app lifecycle, and the input fftBuffer is the same length always, it means that fftPower.Length is going to be constant too, right?

Exactly. Frequency for each RFFT point is just an evenly-spaced range between 0 and the Nyquist frequency (half the sample rate). This function doesn't have to be called more than once as long as your sample rate and FFT size doesn't change. A lot of the logic here is for managing the mirror case, but the idea is the same. You should be able to just generate this once.

FftSharp/src/FftSharp/Transform.cs

Lines 106 to 136 in 6e65e4c

	/// <summary>
	/// Calculate sample frequency for each point in a FFT
	/// </summary>
	public static double[] FFTfreq(double sampleRate, int pointCount, bool oneSided = true)
	{
	double[] freqs = new double[pointCount];
	if (oneSided)
	{
	double fftPeriodHz = sampleRate / pointCount / 2;
	// freqs start at 0 and approach maxFreq
	for (int i = 0; i < pointCount; i++)
	freqs[i] = i * fftPeriodHz;
	return freqs;
	}
	else
	{
	double fftPeriodHz = sampleRate / pointCount;
	// first half: freqs start a 0 and approach maxFreq
	int halfIndex = pointCount / 2;
	for (int i = 0; i < halfIndex; i++)
	freqs[i] = i * fftPeriodHz;
	// second half: then start at -maxFreq and approach 0
	for (int i = halfIndex; i < pointCount; i++)
	freqs[i] = -(pointCount - i) * fftPeriodHz;
	return freqs;
	}
	}

Modern Demo

consider converting the Demo project to SDK style csproj or to a NET5 based WinForms app

This is an excellent suggestion! I'll take care of this #38 and upgrade the ScottPlot as well

---

Thanks again for your help on this! Let me know if this all sounds good to you and if so I'll merge it and push it to NuGet 👍 🚀

[ladeak]

Thank you for the confirmation on FFTfreq().

On the exceptions I still have some mixed feelings, but I think all solutions will work. In all cases the user of the library still gets an exception if something goes wrong.
My personal choice would be try-finally, or without try-finally, just because I don't see we can add too much value in the catch blocks (other than a different message).

[swharden]

My personal choice would be try-finally, or without try-finally, just because I don't see we can add too much value in the catch blocks (other than a different message)

That's fair! My preference would be for the app to halt if the FFT fails.

Complex[] buffer = { /* complex rocket data */ }
FFT(buffer); // modifies the buffer in-place

If I'm going to determine how much rocket fuel I need to get back home from the moon using FFT (go with it, lol) my preference is to have a big-bang exception that halts the program if the calculation fails. Leaving the buffer filled with incomplete or erroneous data without any warning in the case of a failure seems dangerous 😅

Thanks again for all your help on this! I revamped the demo app in response to your feedback (it's now .NET 5)