Memory leak?

[Sieluna]

Description

The onUpdate function will cause a memory leak, where -->>

https://github.com/sunag/flow/blob/0c9b314050d42523cc7a9970113faf47d671b99b/jsm/core/Canvas.js#L265-L269

https://github.com/sunag/flow/blob/0c9b314050d42523cc7a9970113faf47d671b99b/jsm/core/Canvas.js#L481

https://github.com/sunag/flow/blob/0c9b314050d42523cc7a9970113faf47d671b99b/jsm/core/Canvas.js#L329

Evidence

[sunag]

Thanks for detect this, I will test.

[egimata]

I found a possible, maybe not so correct, but working solution for this issue. I found out that "this.update" calls this.updateLines repeatedly. As it should and makes sense to update it. But not when the user is doing nothing.

So instead I changed a bit the code of start() and stop():

// ---- Rest of code -----//
// in Canvas:
// ---- Rest of code in canvas -----//
    this._onMoveEvent = ( e ) => {
    const event = e.touches ? e.touches[ 0 ] : e;
    const { zoom, rect } = this;

    this.clientX = event.clientX;
    this.clientY = event.clientY;

    const rectClientX = ( this.clientX - rect.left ) / zoom;
    const rectClientY = ( this.clientY - rect.top ) / zoom;

    this.relativeClientX = rectClientX - this.scrollLeft;
    this.relativeClientY = rectClientY - this.scrollTop;

    // Update Lines and Map here whenever the user uses "MoveEvent"
        this.updateLines();
        this.updateMap();
    };

    // Start the event listeners when the user clicks on some node
    this._onMouseDown = ( e ) => {
        this.start();
    }

    // Stop the event listeners and set `this.updatable === false` at `this.stop()`
    this._onMouseUp = ( e ) => {
        this.stop();
    }

    this._onUpdate = () => {
    this.update();    
    };

    this.start();

    // Create mousedown event listener only here so we dont include it in this.start() whenever the "this._onMousedown" is active
    document.addEventListener( 'mousedown', this._onMouseDown, true)

    this.relativeY = undefined;
    this.relativeX = undefined;
}

// start() Function
start() {

    this.updating = true;

    document.addEventListener( 'wheel', this._onMoveEvent, true );

    // Remove mousedown event listener here since we are using it to start this function on the upper event listener
    // document.addEventListener( 'mousedown', this._onMoveEvent, true );

    document.addEventListener( 'touchstart', this._onMoveEvent, true );

    document.addEventListener( 'mousemove', this._onMoveEvent, true );
    document.addEventListener( 'touchmove', this._onMoveEvent, true );

    // Add 'mouseup' event connected to this._onMouseUp so the updating stops when mouse is up
    document.addEventListener( 'mouseup', this._onMouseUp, true );

    document.addEventListener( 'dragover', this._onMoveEvent, true );

    // Remove this since we dont need to update again here.
    // requestAnimationFrame( this._onUpdate );

}

// stop() Function
stop() {
    // Update the latest dragged lines then remove updating
    this.update();

    // after update callback request has ended, set this.updating === false
    setTimeout(() => {
            this.updating = false;
            document.removeEventListener( 'wheel', this._onMoveEvent, true );

            document.removeEventListener( 'touchstart', this._onMoveEvent, true );

            document.removeEventListener( 'mousemove', this._onMoveEvent, true );
            document.removeEventListener( 'touchmove', this._onMoveEvent, true );

            document.removeEventListener( 'mouseup', this._onMouseUp, true );

            document.removeEventListener( 'dragover', this._onMoveEvent, true );
       }, 200)
}

You might want to add updateLines() also to the event listener of wheel when you zoom-in/out so the lines will update upon zooming.

const onMouseZoom = ( e ) => {
    e.preventDefault();
    e.stopImmediatePropagation();
    const delta = e.deltaY * .003;
    zoomTo( this.zoom - delta );

    // Add update lines here
    this.updateLines();
};

In this way you will use memory only when you are interacting with the nodes and connecting the ports.

I can create a pull request on this, but i think i did this in a short time and it may not be the best solution yet, but it works and maybe it will help when fixing it properly.

Note: I also removed frontCanvas and frontContext and removed globalCompositeOperation on both context and frontContext. For some reason it's working faster. Also you might want to comment the line context.fillRect( domRect.x, domRect.y, domRect.width, domRect.height ); on updateLines().

[sunag]

Hmm.. Are there still memory leaks or is it just unnecessary GPU processing?

[egimata]

Hmm.. Are there still memory leaks or is it just unnecessary GPU processing?

When I leave just the editor open there is no memory leak, neither GPU processing. GPU starts whenever I start to move the nodes or connecting the ports with lines. I'm guessing it all comes from drawing & updating the lines in canvas (well makes sense), but what confuses me is why it uses so much memory when it's just drawing some simple lines in a 2D canvas.

Update: Occasionally i experience some lags when there are a lot of nodes created