Memory leak with Engine.loadAudioFilesIntoPool

[d.healey]

@Elezeid said in Memory leak with Engine.loadAudioFilesIntoPool:

I used ctrl+F to make sure.

That only searches the current view, did you check all scripts in your project and all callbacks? There is a Find All Occurrences tool (ctrl + shift + f) that might help. However I doubt this is the issue since you say commenting out that line solves it.

[Elezeid]

@d-healey Thanks David, unfortunately this is indeed the only occurrence.

Am I really the first to run into this issue? If so, that definitely suggests user error of some kind, but I really don't know where I might have gone wrong!

[d.healey]

@Elezeid said in Memory leak with Engine.loadAudioFilesIntoPool:

Am I really the first to run into this issue?

First one to report it at least, I haven't tested for it though.

[Elezeid]

@d-healey

I made a minimal project to test the issue and it is indeed present.

Here is the entire onInit:

Content.makeFrontInterface(600, 600);

const var ConvolutionReverb1 = Synth.getAudioSampleProcessor("Convolution Reverb1");

const irs = Engine.loadAudioFilesIntoPool();

//cmbIR
const cmbIR = Content.getComponent("cmbIR");


inline function oncmbIRControl(component, value)
{
	if (value > 0)
		ConvolutionReverb1.setFile(irs[value - 1]);
};

Content.getComponent("cmbIR").setControlCallback(oncmbIRControl);

cmbIR.set("items", "");

for (x in irs)
	cmbIR.addItem(x.replace("{PROJECT_FOLDER}").replace(".wav"));

I've tested this in multiple DAWs as well as on Mac and PC and the memory leak remains. I tried disabling 'saveInPreset' but that didn't help either. The issue is especially egregious when changing presets as well. If you just cycle through presets you can see the RAM load growing with each click.

David suggested putting a check in the combo box's callback to see if the selected IR is already loaded, and if it is don't load it again. I think this is a great idea, but I don't quite know how to go about that.

[d.healey]

@Elezeid If you replace the contents of your control callback with this code it will check if that IR is already loaded before attempting to load the IR.

	if (!value)
		return;
		
	local currentFile = ConvolutionReverb1.getFilename();
	local newFile = irs[value - 1];
	
	if (currentFile == newFile)
		return;
	
	ConvolutionReverb1.setFile(newFile);

[Elezeid]

@d-healey Thanks David,

I tried this and unfortunately the behavior is the same. Out of curiosity, I checked the total file size of my collection of Impulse responses, and wouldn't you know it? That's almost exactly how much the RAM increases by every time a preset is selected!

It looks to me like

Engine.loadAudioFilesIntoPool();

Is running every time a preset is selected, (regardless of whether saveInPreset is active in the project).

What would be really great is if there was such a function as

Engine.purgeAudioFilesFromPool();

Or something to that effect. If there was, I could simply put it at the top of the hierarchy so that onInit any lingering audio files are kicked out in advance before loadAudioFilesIntoPool runs.

[d.healey]

Can you send me your project?

[Elezeid]

@d-healey Sure thing. Here it is without the IRs. Feel free to throw any old audio files in there.

Convolution -Test.zip

[d.healey]

Well I didn't find a solution but I have confirmed the issue. You can demonstrate it with a standalone build, no need to have a DAW in the equation.

@Christoph-Hart You should probably take a look at this. Here's the project as above but I added some IRs.

convolution-test.zip

[AbidextrousMoose]

I fixed it on our end.

There are three things at work here, one on the C++ side and two on the Editor side.

The two separations are dependent on whether the action is question is done in the JS Layer or not.

1. If the Convolution IR is not a UI element that has savedInPreset set to true if any other UI element is and also targets the Convolution IR as a function of its parameter target, it will leak memory.
2. If the combo box selection (as demonstrated in both examples above) is done via the code as it's written, you will leak memory. However this also happens in the case where you manually select the IR via the file browser. I've identified as the culprit of this the way we deallocate the Convolution objects. They are queued for deletion in ConvolutionBase.cpp on line 673 and 674 for both channels. However, they never leave the deletion queue, so the copys that get sent there never run their destructors. This may be an issue with the deletion queue itself, however, I have changed those lines to use the reset() function of the convolution itself and that seems to solve the issue in the near term. Though I don't know the consequences of not queueing the deletion like @Christoph-Hart would.

        if(convolverL != nullptr)
        {
            convolverL.reset();
            convolverR.reset();
        }
        
        convolverL = s1;
        convolverR = s2;

3. In the event that number 1 is active, any preset change will leak memory. Whether through the PresetBrowser or not it will always leak. So if say in FL Studio the Instrument or Plugin is reinitialized or the preset changed and reloaded, it will always leak. We fixed all of this by including the ConvolutionIR itself in the preset.

That's all for now. I don't think it wise to make a PR with the above changes as I don't 100% trust my solution using reset() but it works in our testing and we'll be releasing an update to our primary library (all the plugins of which use this) today.

I'll let you know our results. Would be thrilled to hear any additional opinions.

Edit: loadAudioFilesIntoPool() is a red herring. I wouldn't bother looking into that. The issue is with the Convolution itself.