should req getters be memoized ? #1149

Closed
yields opened this Issue May 25, 2012 · 2 comments

Comments

Projects
None yet
3 participants
@yields
Contributor

yields commented May 25, 2012

yeah i know its not really an "issue", but i'm uncomfortable knowing that it can be memoized and it's not.

any thoughts on that?

i'm willing to pull it and do it myself if you want...

instead of parsing "Accepted" over and over just do.
https://github.com/visionmedia/express/blob/master/lib/request.js#L153

req.__defineGetter__('accepted', function(){
  var accept = this.get('Accept');
  return this.accepted = (accept
      ? utils.parseAccept(accept)
      : [])
});
@buschtoens

This comment has been minimized.

Show comment Hide comment
@buschtoens

buschtoens May 30, 2012

Contributor

You have to decide between performance and memory. You often need req getters only once. So I don't think it's necessary to store the result "just in case". This will consume more memory and memory access time then actually needed.

Contributor

buschtoens commented May 30, 2012

You have to decide between performance and memory. You often need req getters only once. So I don't think it's necessary to store the result "just in case". This will consume more memory and memory access time then actually needed.

@tj

This comment has been minimized.

Show comment Hide comment
@tj

tj Aug 1, 2012

Owner

most of them are meh, the important one to memoize is the url parsing since it happens so frequently, and it is. so until we measure things and find that any of these cause issues i'll close

Owner

tj commented Aug 1, 2012

most of them are meh, the important one to memoize is the url parsing since it happens so frequently, and it is. so until we measure things and find that any of these cause issues i'll close

@tj tj closed this Aug 1, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment