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

Memory leak connected to EJS and PUG view engines #3751

Closed
m-majetic opened this issue Sep 24, 2018 · 12 comments
Closed

Memory leak connected to EJS and PUG view engines #3751

m-majetic opened this issue Sep 24, 2018 · 12 comments

Comments

@m-majetic
Copy link

m-majetic commented Sep 24, 2018

I have tested two simple apps.
One using the ejs templating engine and the other using pug.
Both apps were tested with Siege. One route, and 5 - 20 concurrent users.
////////////////////////
EJS
////////////////////////
memleakservercode
memleakejscode
memleak
/////////////////////////////////
PUG
////////////////////////////////
pugmemleakserver
memleakpugfile
pugmemleakchrome2

In both tests I was using Express.js version 4.16.3, and latest versions of pug and ejs.
Edit -- wrong version of express stated...

@dougwilson
Copy link
Contributor

Express doesn't really do anything but pass data between the two view engines. We can definitely take a look. Can you provide the following?

  1. The source code in text format so we can copy and paste and run it instead of trying to type it out from a photo.
  2. The version of Node.js and the versions of pug/ejs involved.
  3. The steps to reproduce the memory leak.

@dougwilson
Copy link
Contributor

Also, just as an aside, can you try to test the following to verify if the memory leak is in ejs or express by testing this, which doesn't use Express?

var http = require('http')
var ejs = require('ejs')
var fs = require('fs')

var template = fs.readFileSync('views/home/home.ejs', 'utf8') // or where ever your file is located

function app (req, res) {
  res.setHeader('Content-Type', 'text/html')
  res.end(ejs.render(template))
}

http.createServer(app).listen(3000)

@wesleytodd
Copy link
Member

Dupe? #3552

I think we were pretty confident in that issue that there was a leak in ejs.

@m-majetic
Copy link
Author

  1. Source code
    //Server file (index.js) -- when using EJS, just replace the view engine to ejs
var express = require('express');

var app = express();
app.set('view engine', 'pug');

app.get('/', function(req, res){
  res.render('home/home');
});

app.listen(3000);

//Pug view file

doctype html
html
  head 
    title RndSite
  body
    h1 Welcome to my memory test app

//Ejs view file

<!DOCTYPE html>
<html>
  <head>
     <meta charset="utf-8">
     <title></title>
  </head>
  <body>
    <h1> Memory Test </h1>
  </body>
</html>
  1. Versions
    Node -- 8.12.0
    EJS -- 2.6.1
    Pug -- 2.0.3

  2. Steps
    Start the server with : "node --expose-gc --inspect=9222 index.js"
    Open the chrome inspector by typing "chrome://inspect" in the address bar.
    Start Allocation instrumentation on timeline.
    Open up Siege and run: siege -if urls.txt -c 5. // urls.txt file contains : "http://localhost:3000/"

@dougwilson
Copy link
Contributor

Thanks @m-majetic ! Can you please validate #3751 (comment) ?

@dougwilson
Copy link
Contributor

I think we were pretty confident in that issue that there was a leak in ejs.

Right, at least it doesn't seem to be in Express. Either Node.js HTTP server is causing it or (I suspect) the actual inspector itself is causing it as the leak only appears when the inspector is attached to the Node.js process; it doesn't seem to exist when there is no inspector.

@wesleytodd
Copy link
Member

When testing in that issue, I did a test like @dougwilson suggests. Only when I added the ejs part did I get a leak. #3552 (comment)

I would go ahead and just remove the http stuff all together and try just rendering a file with both engines in a loop. Then check the memory usage for that. I believe, IIRC, you will find your leak in there.

@m-majetic
Copy link
Author

The thing is, I am getting the same leak using pug as well...
There might be something with the inspector.

@dougwilson
I have tried it without express and I am getting the same thing.
I will now try to monitor the rss and heapTotal without the inspector

@dougwilson
Copy link
Contributor

Hi @m-majetic right, I get that it happens with both ejs and pug, but that still doesn't mean it is Express. It sounds like you tested with the basic app I provided that does not include Express at all. I think at this point it rules out Express as the contributing factor here. If you think otherwise on why specifically Express is causing the memory leak, please let us know and we can reopen to investigate further.

@m-majetic
Copy link
Author

With further investigation I found out that the problem is the node inspector.
Sorry for the false accusations.
After the debugger attaches it starts allocating memory to function strings for some reason.

proof

@almgwary

This comment has been minimized.

@dougwilson

This comment has been minimized.

@expressjs expressjs locked as resolved and limited conversation to collaborators Jan 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants