cazala / synaptic

architecture-free neural network library for node.js and the browser
http://caza.la/synaptic
Other
6.92k stars 666 forks source link

Memory leak: fromJSON adds a global reference for each neuron: Layer.prototype.neurons.{ID} #164

Open deltaidea opened 7 years ago

deltaidea commented 7 years ago

I noticed that creating networks from JSON leads to a memory leak that is very noticeable with 100-300 networks per minute.

Network.fromJSON calls layers.{layer}.add(neuron) for each added neuron. Layer.prototype.add (layer.js:189) does this:

    this.neurons[neuron.ID] = neuron || new Neuron();
    this.list.push(neuron);
    this.size++;

Because the layer instance doesn't have neurons and its __proto__ does (Layer.prototype.neurons()), every neuron is saved as a field of that method and is forever accessible as Layer.prototype.neurons[ID].

I can't find where this is ever used, so I imagine this could be fixed by simply removing that line.

Thank you for the awesome library!

uiteoi commented 7 years ago

There is also an inconsistency in the constructor of Layer that should call this.add( new Neuron ) in the while loop instead of partially duplicating the code of add(). It should be written:

while( size-- ) this.add( new Neuron() );

Also, line 189 does not work if neuron is undefined or null, because neuron.ID would throw an error 'Cannot read property 'ID' of undefined'.

A correct solution would be:

neuron = neuron || new Neuron()
this.neuron[ neuron.ID ] = neuron;

That is if line 189 is at all required.

This would in turn allow to write in the constructor:

while( size-- ) this.add();