Move static files middleware before sessions

[bmonty]

I was having a performance problem with reloading a site built off of meanjs. I solved the problem by moving the config for the static files middleware before the sessions middleware. Static files load significantly faster because there isn't session tracking being done for each of them.

[bmonty]

My original pull request didn't move the helmet config, so any static files didn't get the headers provided by helmet. I relooked this changes I made with the goal of having the headers look the same before and after (since this is what helmet affects). I was able to get the same headers and a performance improvement by moving both helmet and express.static before sessions. The stock session functionality (log in/out and restrict access to articles) works with the changes.

On my setup, page load times drop from ~4s to ~750ms. Looking at the network performance monitor in the Chrome dev tools, the time decrease comes from faster loads on static files.

Headers from request for bootstrap.css with latest meanjs:

HTTP/1.1 304 Not Modified
X-Frame-Options: SAMEORIGIN
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
X-Download-Options: noopen
Accept-Ranges: bytes
Date: Fri, 26 Dec 2014 12:27:26 GMT
Cache-Control: public, max-age=0
Last-Modified: Wed, 12 Nov 2014 17:03:16 GMT
ETag: W/"2176b-3526875964"
Connection: keep-alive

Headers with the change I originally proposed:

HTTP/1.1 304 Not Modified
Accept-Ranges: bytes
Date: Fri, 26 Dec 2014 12:53:10 GMT
Cache-Control: public, max-age=0
Last-Modified: Wed, 12 Nov 2014 17:03:16 GMT
ETag: W/"2176b-3526875964"
Connection: keep-alive

Headers after moving helmet and express.static (same as original):

HTTP/1.1 304 Not Modified
X-Frame-Options: SAMEORIGIN
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
X-Download-Options: noopen
Accept-Ranges: bytes
Date: Fri, 26 Dec 2014 12:36:00 GMT
Cache-Control: public, max-age=0
Last-Modified: Wed, 12 Nov 2014 17:03:16 GMT
ETag: W/"2176b-3526875964"
Connection: keep-alive

[ilanbiala]

@bmonty nice catch. LGTM.

[pursual]

This is broken again.

[mleanos]

@pursual Can you provide more details on what you're experiencing, and how this is broken?

[pursual]

The session is now initiated before the static routes, and every request for a static file pings mongo. Bottom of config/lib/express.js

If you ever wondered why meanjs was slow as hell out of the box, and failed to fully load 5/10 times, this is it. I actually missed the version where it was fixed since last time I used the framework.

[mleanos]

@pursual Yes.The session is initiated before the static routes. https://github.com/meanjs/mean/blob/master/config/lib/express.js#L232-L242

I can confirm that there is a significant performance increase when moving the Helmet & Static routes middleware before the Session.

Thanks @pursual for pointing this out. I think was re-introduced during the move from the older structure of 0.3 to 0.4.

@lirantal Can you confirm that this indeed does have an impact given our current structure. My performance testing here was very rudimentary.

@codydaig @ilanbiala Any special insight into this?

[pursual]

Pull request submitted, worked fine in my project.

[lirantal]

right, that PR is good, we should merge it.