Optimize already encoded streams

[Rush]

I was doing some performance tests with passing already encoded streams through http-master (https://github.com/encharm/http-master) (which utilizes expressjs/compression). Weirdly enough there was a big performance hit when doing many requests that simply passed through expressjs/compresison. My initial suspicion was that requests were being re-encoded but no ... the complexity of your on, end and write wrappers was enough to visibly degrade performance. This pull request restores the original function handlers after knowing that stream is not gonna be encoded.

It completely solved my issue. I would appreciate a merge.

[dougwilson]

Sorry, this is not acceptable. Your change assumes that after we replaced those methods, another module did not also replace them.

If you can address that case, I will reconsider.

[dougwilson]

I quick test case you can run, is the following:

app.use(compression())
app.use(function (req, res, next) {
  var end = res.end;
  res.end = function () {
    console.log('clean up something')
    return end.apply(this, arguments)
  }
  next();
})
app.use(function (req, res, next) {
  res.setHeader('Content-Length', '3')
  res.end('hi!')
})

I just typed this out quickly, so please excuse any typos. With your changes, the "clean up something" should appear on the console (the content-length is there to help ensure we don't compress, which should trigger your added behavior).

[dougwilson]

P.S. I'm really sorry, I didn't mean to sound harsh, I just keep getting this exact PR in various repos every so often, which has the issue hopefully highlighted by the code above. I'm all for solving any performance problems.

Perhaps, if you cannot get something working that plays well with other modules, can you share the performance traces showing exactly where we are slowing down and perhaps address them?

[Rush]

@dougwilson thanks for your input, and no it was not too harsh.

It appears that those performance problems were happening only on 1.4.4 but not on master, judging from the code, possibly having been fixed by 2086d11.

Sorry for rushing to implement a fix without properly checking if master is already working well.

As I cannot see a statistically significant performance difference with this fix on top of master, I am closing this PR.

Apologies for wasting your time.

[dougwilson]

Ah, interesting. There were a bunch of performance improvements in the 1.5.0 release, so I suppose one of those fixed what you were seeing (which, is actually awesome to here, because I fixed those simply because they needed to be, rather than based on complaints!).

Also, you're not wasting my time :) I'm truly sorry if I came off harsh in the first response :(