Skip to content

Remove in-memory caches #36

Closed
wants to merge 6 commits into from

3 participants

@dobesv
dobesv commented Nov 2, 2011

This patch removes the in-memory caches. It also includes my improvements to support gzip since otherwise the two changes would conflict. Let me know if you'd like a non-gzip-included version of this.

Originally I considered fixing the caching issues I saw, such as:

  1. The cache is always populated with the static file result even if caching is disabled
  2. Items are never removed from the cache, so this may use a lot of memory
  3. In serveDir it checks if (pathname in exports.store) but runs streamFiles(exports.indexStore[pathname].files) (i.e. checking one cache but reading the other)

But I realized that users requiring in-memory caching should do this as part of a smarter middleware component or inside a proxy in front of node rather than leaning on the static files component to deal with it internally.

That way the memory overhead of the cache can be better controlled, monitored, and utilized by all components of the server (or cluster).

The cache as-is will be a kind of liability due to its unbounded size, and creating a full-featured cache might be more appropriate as its own project where dynamically output content within the node server can also benefit from it.

Thanks!

Dobes

dobesv added some commits Nov 2, 2011
@dobesv dobesv Add static gzip file support, where a gzip file with a matching name …
…but adding .gz to the end can be served up to clients that support it. As part of that, changed the code so the Content-Type and Content-Length are sent even for an HTTP HEAD request.
1a88cad
@dobesv dobesv Updated the README to describe gzip configuration 77fcd6f
@dobesv dobesv Remove in-memory cache 594dd45
@dobesv dobesv Fix check for acceptEncoding header to work if there is no accept-enc…
…oding header
d491f59
@dobesv dobesv Fix check for acceptEncoding header to work if there is no accept-enc…
…oding header
83d4775
@dobesv dobesv Fix error with accept-encoding heaer check. Fix bug where Content-Le…
…gth was sent with a 304 not modified response.
cded9f3
@dubiousdavid

Why was this pull-request never commented on? Seems like a lot of good stuff in here.

@phstc
Collaborator
phstc commented Mar 30, 2013

It needs to be remerged.

This pull request cannot be automatically merged

@phstc phstc closed this Jul 1, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.