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

Help Wanted, Possible Memory leak? #392

Closed
Starystars67 opened this issue Jul 25, 2018 · 7 comments
Closed

Help Wanted, Possible Memory leak? #392

Starystars67 opened this issue Jul 25, 2018 · 7 comments

Comments

@Starystars67
Copy link

Starystars67 commented Jul 25, 2018

Hello, okay so i know there have been other issues on the same subject but im at a loss with this and need some help please. Okay so why do i think EJS is the cause? here is some screen shots of heap snapshots in comparison between start up and after autocannon.
image
As you can see in the above image there is a sizable difference in file size given its a web server.
image

so after taking this and searching through the code I boiled it down to line 573 in ejs.js which is located in the lib folder of the ejs library.
Further scrolling through the leaked strings I found this which i think confirms the leak to be related to EJS:
image

I will add more as I get it, Again any help is very much appreciated and can someone please add the help-wanted tag.

Many Thanks.

EDIT: Code!! Because everyone loves code.

const memwatch   = require('memwatch-next');
var moment = require('moment');

// load the things we need
var express = require('express');
var app = express();

// set the view engine to ejs
app.set('view engine', 'ejs');

// use res.render to load up an ejs view file

// index page
app.get('/', function(req, res) {
	res.render('public/landingpage', {moment: moment});
});

app.listen(8080);
console.log('8080 is the magic port');

memwatch.on('leak', (info) => {
  console.log(info)
});
@m-majetic
Copy link

Greetings !
I have opened an issue as well, regarding a memory leak.
Did you resolve the issue or does it still persist ?

@Starystars67
Copy link
Author

Hello @m-majetic, unfortunately I have not managed to resolve the issue but it's nice to see someone else has encountered the issue. In a round about sort of way.

@m-majetic
Copy link

I see that other issues haven't been closed as well.
It makes me question, is everything is alright with the development of EJS ?

@Starystars67
Copy link
Author

Hmm makes you wonder doesn't it.

@mde
Copy link
Owner

mde commented Sep 24, 2018

I've taken a bit of time to look into this. From what I can tell from your screenshots, and a brief session with the Node debugger myself, it looks like those strings are the source strings for the constructed function that the Template objects use to execute and render templates here: https://github.com/mde/ejs/blob/master/lib/ejs.js#L626 This is a result of using the Function constructor, which is a built-in.

I took Express out of the equation completely, just loading EJS into a Node script and calling render with template text of empty string in a tight loop, using setImmediate to ensure GC can occur. Both with requested GC (global.gc()) and normal, Node-scheduled GC, I did see these strings getting GC'd. The behavior was the same with an inline use of Function constructor with a dummy function body.

I'm not sure if these strings are slow to GC, or what, but I have not been able to isolate anything specific in EJS that looks anything like a memory leak. I do see memory go up, but the behavior looks almost identical with the inline use of a Function constructor, or even with a complete no-op in that same loop.

Having said that, I will continue to dig into this and see if I can track down something more concrete. I only took a cursory look at this today, so I may very well be missing something else.

Thanks for your patience with this.

@m-majetic
Copy link

#401 (comment)

@mde
Copy link
Owner

mde commented Sep 25, 2018

Not an issue with EJS. For reference: expressjs/express#3751 (comment)

@mde mde closed this as completed Sep 25, 2018
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

3 participants